[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.

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
@@ -23,10 +23,6 @@ abstract class CourseBackend
 	 */
 	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.)
 	 */
 	const MAX_ADDIDIONAL_LOCATIONS = 5;
@@ -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
@@ -30,23 +30,6 @@
 					<div class="list-group-item">
 						<div class="row">
 							<div class="col-md-3">
-								<label>{{lang_serverUrl}}</label>
-							</div>
-							<div class="col-md-7">
-								<input required class="form-control" id="input-url-{{id}}" name="url" type="text"
-										 value="{{url}}">
-							</div>
-							<div class="col-md-2">
-								<a class="btn btn-default" id="help-url" title="{{lang_urlTooltip}}">
-									<span class="glyphicon glyphicon-question-sign"></span>
-								</a>
-							</div>
-						</div>
-					</div>
-
-					<div class="list-group-item">
-						<div class="row">
-							<div class="col-md-3">
 								<label>{{lang_serverType}}</label>
 							</div>
 							<div class="col-md-7">