From c781a551ae84127ef05eaa36909dca44e49e1200 Mon Sep 17 00:00:00 2001
From: Simon Rettberg
Date: Wed, 19 Apr 2017 23:43:24 +0200
Subject: [locationinfo] Better backend-specific property handling, get rid of
URL
- The backend URL still had special treatment for legacy reasons, when it
would be perfectly fine to make it just another generic property the
backend has to define.
- Allow for the backend to declare a default value for properties.
- Base class will now check and sanitize the setCredentials() input.
---
modules-available/locationinfo/api.inc.php | 7 +-
.../locationinfo/inc/coursebackend.inc.php | 47 +++++++++++---
.../coursebackend/coursebackend_davinci.inc.php | 12 ++--
.../inc/coursebackend/coursebackend_dummy.inc.php | 17 +++--
.../coursebackend/coursebackend_hisinone.inc.php | 75 +++++++++++++---------
modules-available/locationinfo/page.inc.php | 42 ++++++------
.../locationinfo/templates/server-settings.html | 17 -----
7 files changed, 120 insertions(+), 97 deletions(-)
diff --git a/modules-available/locationinfo/api.inc.php b/modules-available/locationinfo/api.inc.php
index a6e2fda3..412b20bf 100644
--- a/modules-available/locationinfo/api.inc.php
+++ b/modules-available/locationinfo/api.inc.php
@@ -238,7 +238,7 @@ function formatOpeningtime($openingtime)
*/
function getConfig($locationID)
{
- $dbresult = Database::queryFirst("SELECT l.locationname, li.config, li.serverroomid, s.servertype, s.serverurl FROM `location_info` AS li
+ $dbresult = Database::queryFirst("SELECT l.locationname, li.config, li.serverroomid, s.servertype FROM `location_info` AS li
RIGHT JOIN `location` AS l ON l.locationid=li.locationid
LEFT JOIN `setting_location_info` AS s ON s.serverid=li.serverid
WHERE l.locationid=:locationID", array('locationID' => $locationID));
@@ -364,7 +364,7 @@ function getCalendar($idList)
if (!empty($idList)) {
// Build SQL query for multiple ids.
$qs = '?' . str_repeat(',?', count($idList) - 1);
- $query = "SELECT l.locationid, l.serverid, l.serverroomid, s.serverurl, s.servertype, s.credentials
+ $query = "SELECT l.locationid, l.serverid, l.serverroomid, s.servertype, s.credentials
FROM `location_info` AS l
INNER JOIN setting_location_info AS s ON (s.serverid = l.serverid)
WHERE l.hidden = 0 AND l.locationid IN ($qs)
@@ -376,7 +376,6 @@ function getCalendar($idList)
if (!isset($serverList[$dbresult['serverid']])) {
$serverList[$dbresult['serverid']] = array(
'credentials' => json_decode($dbresult['credentials'], true),
- 'url' => $dbresult['serverurl'],
'type' => $dbresult['servertype'],
'idlist' => array()
);
@@ -395,7 +394,7 @@ function getCalendar($idList)
array('lid' => $server['locationid']));
continue;
}
- $credentialsOk = $serverInstance->setCredentials($server['credentials'], $server['url'], $serverid);
+ $credentialsOk = $serverInstance->setCredentials($serverid, $server['credentials']);
if ($credentialsOk) {
$calendarFromBackend = $serverInstance->fetchSchedule($server['idlist']);
diff --git a/modules-available/locationinfo/inc/coursebackend.inc.php b/modules-available/locationinfo/inc/coursebackend.inc.php
index 7dc50549..0d84b0fb 100644
--- a/modules-available/locationinfo/inc/coursebackend.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend.inc.php
@@ -22,10 +22,6 @@ abstract class CourseBackend
* @var int as internal serverId
*/
protected $serverId;
- /**
- * @var string url of the service
- */
- protected $location;
/**
* @const int max number of additional locations to fetch (for backends that benefit from request coalesc.)
*/
@@ -36,7 +32,6 @@ abstract class CourseBackend
*/
public final function __construct()
{
- $this->location = "";
$this->error = false;
}
@@ -97,7 +92,7 @@ abstract class CourseBackend
/**
- * @returns array with parameter name as key and and an array with type, help text and mask as value
+ * @returns \BackendProperty[] list of properties that need to be set
*/
public abstract function getCredentials();
@@ -110,12 +105,10 @@ abstract class CourseBackend
* uses json to setCredentials, the json must follow the form given in
* getCredentials
*
- * @param array $data with the credentials
- * @param string $url address of the server
- * @param int $serverId ID of the server
+ * @param array $data assoc array with data required by backend
* @returns bool if the credentials were in the correct format
*/
- public abstract function setCredentials($data, $url, $serverId);
+ public abstract function setCredentialsInternal($data);
/**
* @return int desired caching time of results, in seconds. 0 = no caching
@@ -215,6 +208,25 @@ abstract class CourseBackend
return $returnValue;
}
+ public final function setCredentials($serverId, $data)
+ {
+ foreach ($this->getCredentials() as $prop) {
+ if (!isset($data[$prop->property])) {
+ $data[$prop->property] = $prop->default;
+ }
+ if (in_array($prop->type, ['string', 'bool', 'int'])) {
+ settype($data[$prop->property], $prop->type);
+ } else {
+ settype($data[$prop->property], 'string');
+ }
+ }
+ if ($this->setCredentialsInternal($data)) {
+ $this->serverId = $serverId;
+ return true;
+ }
+ return false;
+ }
+
/**
* @return false if there was no error string with error message if there was one
*/
@@ -297,3 +309,18 @@ abstract class CourseBackend
}
}
+
+/**
+ * Class BackendProperty describes a property a backend requires to define its functionality
+ */
+class BackendProperty {
+ public $property;
+ public $type;
+ public $default;
+ public function __construct($property, $type, $default = '')
+ {
+ $this->property = $property;
+ $this->type = $type;
+ $this->default = $default;
+ }
+}
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
index db75c19d..77928b39 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_davinci.inc.php
@@ -3,14 +3,16 @@
class CourseBackend_Davinci extends CourseBackend
{
- public function setCredentials($data, $location, $serverId)
+ private $location;
+
+ public function setCredentialsInternal($data)
{
- if (empty($location)) {
+ if (empty($data['baseUrl'])) {
$this->error = "No url is given";
return false;
}
+ $location = preg_replace('#/+(davinciis\.dll)?\W*$#i', '', $data['baseUrl']);
$this->location = $location . "/DAVINCIIS.dll?";
- $this->serverId = $serverId;
//Davinci doesn't have credentials
return true;
}
@@ -30,7 +32,9 @@ class CourseBackend_Davinci extends CourseBackend
public function getCredentials()
{
- return array();
+ return [
+ new BackendProperty('baseUrl', 'string')
+ ];
}
public function getDisplayName()
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
index 5bceac3e..b8329196 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_dummy.inc.php
@@ -13,7 +13,7 @@ class CourseBackend_Dummy extends CourseBackend
* @param int $serverId ID of the server
* @returns bool if the credentials were in the correct format
*/
- public function setCredentials($json, $location, $serverId)
+ public function setCredentialsInternal($json)
{
$x = $json;
$this->pw = $x['password'];
@@ -47,15 +47,14 @@ class CourseBackend_Dummy extends CourseBackend
public function getCredentials()
{
$options = ["opt1", "opt2", "opt3", "opt4", "opt5", "opt6", "opt7", "opt8"];
- $credentials = [
- "username" => "string",
- "password" => "password",
- "integer" => "int",
- "option" => $options,
- "CheckTheBox" => "bool",
- "CB2 t" => "bool"
+ return [
+ new BackendProperty('username', 'string', 'default-user'),
+ new BackendProperty('password', 'password'),
+ new BackendProperty('integer', 'int', 7),
+ new BackendProperty('option', $options),
+ new BackendProperty('CheckTheBox', 'bool'),
+ new BackendProperty('CB2t', 'bool', true)
];
- return $credentials;
}
/**
diff --git a/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php b/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
index 9e7c9db0..b01146a8 100644
--- a/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
+++ b/modules-available/locationinfo/inc/coursebackend/coursebackend_hisinone.inc.php
@@ -2,36 +2,59 @@
class CourseBackend_HisInOne extends CourseBackend
{
- private $username;
- private $password;
- private $open;
+ private $username = '';
+ private $password = '';
+ private $open = true;
+ private $location;
+ private $verifyHostname = true;
+ private $verifyCert = true;
- public function setCredentials($data, $url, $serverId)
+ public function setCredentialsInternal($data)
{
- if (array_key_exists('password', $data) && array_key_exists('username', $data) && array_key_exists('role', $data) && isset($data['open'])) {
- $this->error = false;
- $this->password = $data['password'];
- $this->username = $data['username'] . "\t" . $data['role'];
- $this->open = $data['open'];
- if ($url == "") {
- $this->error = "No url is given";
- return false;
- }
- if ($this->open) {
- $this->location = $url . "/qisserver/services2/OpenCourseService";
- } else {
- $this->location = $url . "/qisserver/services2/CourseService";
+ if (!$data['open']) {
+ // If not using OpenCourseService, require credentials
+ foreach (['username', 'password'] as $field) {
+ if (empty($data[$field])) {
+ $this->error = 'setCredentials: Missing field ' . $field;
+ return false;
+ }
}
- $this->serverId = $serverId;
- } else {
- $this->error = "wrong credentials";
+ }
+ if (empty($data['baseUrl'])) {
+ $this->error = "No url is given";
return false;
}
+ $this->error = false;
+ $this->username = $data['username'] . "\t" . $data['role'];
+ $this->password = $data['password'];
+ $this->open = $data['open'];
+ $url = preg_replace('#(/+qisserver(/+services\d+(/+OpenCourseService)?)?)?\W*$#i', '', $data['baseUrl']);
+ if ($this->open) {
+ $this->location = $url . "/qisserver/services2/OpenCourseService";
+ } else {
+ $this->location = $url . "/qisserver/services2/CourseService";
+ }
+ $this->verifyHostname = $data['verifyHostname'];
+ $this->verifyCert = $data['verifyCert'];
+
return true;
}
+ public function getCredentials()
+ {
+ return [
+ new BackendProperty('baseUrl', 'string'),
+ new BackendProperty('username', 'string'),
+ new BackendProperty('role', 'string'),
+ new BackendProperty('password', 'password'),
+ new BackendProperty('open', 'bool', true),
+ new BackendProperty('verifyCert', 'bool', true),
+ new BackendProperty('verifyHostname', 'bool', true)
+ ];
+ }
+
public function checkConnection()
{
if (empty($this->location)) {
@@ -159,8 +182,8 @@ class CourseBackend_HisInOne extends CourseBackend
$options = array(
CURLOPT_RETURNTRANSFER => true,
CURLOPT_FOLLOWLOCATION => true,
- CURLOPT_SSL_VERIFYHOST => false,
- CURLOPT_SSL_VERIFYPEER => false,
+ CURLOPT_SSL_VERIFYHOST => $this->verifyHostname,
+ CURLOPT_SSL_VERIFYPEER => $this->verifyCert,
CURLOPT_URL => $this->location,
CURLOPT_POSTFIELDS => $request,
CURLOPT_HTTPHEADER => $header,
@@ -197,14 +220,6 @@ class CourseBackend_HisInOne extends CourseBackend
return "HisInOne";
}
-
- public function getCredentials()
- {
- $credentials = ["username" => "string", "role" => "string", "password" => "password", "open" => "bool"];
- return $credentials;
- }
-
-
public function fetchSchedulesInternal($requestedRoomIds)
{
if (empty($requestedRoomIds)) {
diff --git a/modules-available/locationinfo/page.inc.php b/modules-available/locationinfo/page.inc.php
index 31702dff..263d07d0 100644
--- a/modules-available/locationinfo/page.inc.php
+++ b/modules-available/locationinfo/page.inc.php
@@ -106,7 +106,6 @@ class Page_LocationInfo extends Page
{
$serverid = Request::post('id', -1, 'int');
$servername = Request::post('name', 'unnamed', 'string');
- $serverurl = Request::post('url', '', 'string');
$servertype = Request::post('type', '', 'string');
$backend = CourseBackend::getInstance($servertype);
@@ -119,24 +118,23 @@ class Page_LocationInfo extends Page
$credentialsJson = array();
$counter = 0;
- foreach ($tmptypeArray as $key => $value) {
- $credentialsJson[$key] = Request::post($counter);
+ foreach ($tmptypeArray as $cred) {
+ $credentialsJson[$cred->property] = Request::post($counter);
$counter++;
}
$params = array(
'name' => $servername,
- 'url' => $serverurl,
'type' => $servertype,
'credentials' => json_encode($credentialsJson)
);
if ($serverid === 0) {
- Database::exec('INSERT INTO `setting_location_info` (servername, serverurl, servertype, credentials)
- VALUES (:name, :url, :type, :credentials)', $params);
+ Database::exec('INSERT INTO `setting_location_info` (servername, servertype, credentials)
+ VALUES (:name, :type, :credentials)', $params);
$this->checkConnection(Database::lastInsertId());
} else {
$params['id'] = $serverid;
Database::exec('UPDATE `setting_location_info`
- SET servername = :name, serverurl = :url, servertype = :type, credentials = :credentials
+ SET servername = :name, servertype = :type, credentials = :credentials
WHERE serverid = :id', $params);
$this->checkConnection($serverid);
}
@@ -280,7 +278,7 @@ class Page_LocationInfo extends Page
Util::traceError('checkConnection called with no server id');
}
- $dbresult = Database::queryFirst("SELECT servertype, credentials, serverurl
+ $dbresult = Database::queryFirst("SELECT servertype, credentials
FROM `setting_location_info`
WHERE serverid = :serverid", array('serverid' => $serverid));
@@ -289,7 +287,7 @@ class Page_LocationInfo extends Page
LocationInfo::setServerError($serverid, 'Unknown backend type: ' . $dbresult['servertype']);
return;
}
- $credentialsOk = $serverInstance->setCredentials(json_decode($dbresult['credentials'], true), $dbresult['serverurl'], $serverid);
+ $credentialsOk = $serverInstance->setCredentials($serverid, json_decode($dbresult['credentials'], true));
if ($credentialsOk) {
$connectionOk = $serverInstance->checkConnection();
@@ -439,7 +437,7 @@ class Page_LocationInfo extends Page
*/
private function ajaxServerSettings($id)
{
- $dbresult = Database::queryFirst('SELECT servername, serverurl, servertype, credentials
+ $dbresult = Database::queryFirst('SELECT servername, servertype, credentials
FROM `setting_location_info` WHERE serverid = :id', array('id' => $id));
// Credentials stuff.
@@ -463,35 +461,34 @@ class Page_LocationInfo extends Page
$backend['credentials'] = array();
$counter = 0;
- foreach ($credentials as $key => $value) {
+ foreach ($credentials as $cred) {
$credential['uid'] = $counter;
- $credential['name'] = Dictionary::translateFile($s, $key);
- $credential['type'] = $value;
- $credential['title'] = Dictionary::translateFile($s, $key . "_title");
+ $credential['name'] = Dictionary::translateFile($s, $cred->property);
+ $credential['type'] = $cred->type;
+ $credential['title'] = Dictionary::translateFile($s, $cred->property . "_title");
if (Property::getPasswordFieldType() === 'text') {
$credential['mask'] = false;
} else {
- if ($value == "password") {
+ if ($cred->type === "password") {
$credential['mask'] = true;
}
}
if ($backend['typ'] == $dbresult['servertype']) {
- foreach ($dbcredentials as $k => $v) {
- if ($k == $key) {
- $credential['value'] = $v;
- break;
- }
+ if (isset($dbcredentials[$cred->property])) {
+ $credential['value'] = $dbcredentials[$cred->property];
+ } else {
+ $credential['value'] = $cred->default;
}
}
$selection = array();
- if (is_array($value)) {
+ if (is_array($cred->type)) {
$selfirst = true;
- foreach ($value as $opt) {
+ foreach ($cred->type as $opt) {
$option['option'] = $opt;
if (isset($credential['value'])) {
if ($opt == $credential['value']) {
@@ -523,7 +520,6 @@ class Page_LocationInfo extends Page
echo Render::parse('server-settings', array('id' => $id,
'name' => $dbresult['servername'],
- 'url' => $dbresult['serverurl'],
'servertype' => $dbresult['servertype'],
'backendList' => array_values($serverBackends)));
}
diff --git a/modules-available/locationinfo/templates/server-settings.html b/modules-available/locationinfo/templates/server-settings.html
index c135543f..1257ab52 100644
--- a/modules-available/locationinfo/templates/server-settings.html
+++ b/modules-available/locationinfo/templates/server-settings.html
@@ -27,23 +27,6 @@
-
-
-
-
-
-
-
-
-
-
-