Merge pull request #5809 from owncloud/fix5753

LDAP Wizard: proper behaviour with anonymous bind, fixes #5753

5 files changed, 35 insertions, 53 deletions

diff --git a/apps/user_ldap/css/settings.css b/apps/user_ldap/css/settings.css
index 41578e91b69..be03419c2de 100644
--- a/apps/user_ldap/css/settings.css
+++ b/apps/user_ldap/css/settings.css
@@ -3,13 +3,9 @@
 	width: 60%;
 }
 
-.tablecell {
-	display: table-cell !important;
-	white-space: nowrap;
-}
-
 .tablerow {
 	display: table-row;
+	white-space: nowrap;
 }
 
 .tablerow input, .tablerow textarea {
@@ -78,6 +74,7 @@
 
 .wizSpinner {
 	height: 15px;
+	margin: 0.3em;
 }
 
 .ldapSettingControls {
@@ -100,7 +97,6 @@
 
 #ldap fieldset input, #ldap fieldset textarea {
  	width: 60%;
-	display: inline-block;
 }
 
 #ldap fieldset p input[type=checkbox] {
diff --git a/apps/user_ldap/js/settings.js b/apps/user_ldap/js/settings.js
index 3abec1e232e..dcaeb70b57f 100644
--- a/apps/user_ldap/js/settings.js
+++ b/apps/user_ldap/js/settings.js
@@ -192,19 +192,20 @@ var LdapWizard = {
 					'&ldap_serverconfig_chooser='+$('#ldap_serverconfig_chooser').val();
 
 			LdapWizard.showSpinner('#ldap_base');
+			$('#ldap_base').prop('disabled', 'disabled');
 			LdapWizard.ajax(param,
 				function(result) {
 					LdapWizard.applyChanges(result);
 					LdapWizard.hideSpinner('#ldap_base');
 					if($('#ldap_base').val()) {
-						$('#ldap_base').removeClass('invisible');
 						LdapWizard.hideInfoBox();
 					}
+					$('#ldap_base').prop('disabled', false);
 				},
 				function (result) {
 					LdapWizard.hideSpinner('#ldap_base');
-					$('#ldap_base').removeClass('invisible');
-					LdapWizard.showInfoBox('Please specify a port');
+					LdapWizard.showInfoBox('Please specify a Base DN');
+					$('#ldap_base').prop('disabled', false);
 				}
 			);
 		}
@@ -212,28 +213,28 @@ var LdapWizard = {
 
 	checkPort: function() {
 		host = $('#ldap_host').val();
-		user = $('#ldap_dn').val();
-		pass = $('#ldap_agent_password').val();
+		port = $('#ldap_port').val();
 
-		if(host && user && pass) {
+		if(host && !port) {
 			param = 'action=guessPortAndTLS'+
 					'&ldap_serverconfig_chooser='+$('#ldap_serverconfig_chooser').val();
 
 			LdapWizard.showSpinner('#ldap_port');
+			$('#ldap_port').prop('disabled', 'disabled');
 			LdapWizard.ajax(param,
 				function(result) {
 					LdapWizard.applyChanges(result);
 					LdapWizard.hideSpinner('#ldap_port');
 					if($('#ldap_port').val()) {
 						LdapWizard.checkBaseDN();
-						$('#ldap_port').removeClass('invisible');
+						$('#ldap_port').prop('disabled', false);
 						LdapWizard.hideInfoBox();
 					}
 				},
 				function (result) {
 					LdapWizard.hideSpinner('#ldap_port');
-					$('#ldap_port').removeClass('invisible');
-					LdapWizard.showInfoBox('Please specify the BaseDN');
+					$('#ldap_port').prop('disabled', false);
+					LdapWizard.showInfoBox('Please specify the Port');
 				}
 			);
 		}
@@ -461,12 +462,6 @@ var LdapWizard = {
 	},
 
 	init: function() {
-		if($('#ldap_port').val()) {
-			$('#ldap_port').removeClass('invisible');
-		}
-		if($('#ldap_base').val()) {
-			$('#ldap_base').removeClass('invisible');
-		}
 		LdapWizard.basicStatusCheck();
 	},
 
@@ -521,6 +516,8 @@ var LdapWizard = {
 	},
 
 	processChanges: function(triggerObj) {
+		LdapWizard.hideInfoBox();
+
 		if(triggerObj.id == 'ldap_host'
 		   || triggerObj.id == 'ldap_port'
 		   || triggerObj.id == 'ldap_dn'
diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php
index 9428a98e29c..0b2a6a540ff 100644
--- a/apps/user_ldap/lib/wizard.php
+++ b/apps/user_ldap/lib/wizard.php
@@ -63,8 +63,6 @@ class Wizard extends LDAPUtility {
 	public function countGroups() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   ))) {
 			return  false;
@@ -98,8 +96,6 @@ class Wizard extends LDAPUtility {
 	public function countUsers() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   'ldapUserFilter',
 										   ))) {
@@ -130,8 +126,6 @@ class Wizard extends LDAPUtility {
 	public function determineAttributes() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   'ldapUserFilter',
 										   ))) {
@@ -160,8 +154,6 @@ class Wizard extends LDAPUtility {
 	private function getUserAttributes() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   'ldapUserFilter',
 										   ))) {
@@ -214,8 +206,6 @@ class Wizard extends LDAPUtility {
 	private function determineGroups($dbkey, $confkey, $testMemberOf = true) {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   ))) {
 			return  false;
@@ -242,8 +232,6 @@ class Wizard extends LDAPUtility {
 	public function determineGroupMemberAssoc() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapGroupFilter',
 										   ))) {
 			return  false;
@@ -266,8 +254,6 @@ class Wizard extends LDAPUtility {
 	public function determineGroupObjectClasses() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   ))) {
 			return  false;
@@ -294,8 +280,6 @@ class Wizard extends LDAPUtility {
 	public function determineUserObjectClasses() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   ))) {
 			return  false;
@@ -322,8 +306,6 @@ class Wizard extends LDAPUtility {
 	public function getGroupFilter() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   ))) {
 			return false;
@@ -337,8 +319,6 @@ class Wizard extends LDAPUtility {
 	public function getUserListFilter() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   ))) {
 			return false;
@@ -355,13 +335,12 @@ class Wizard extends LDAPUtility {
 	public function getUserLoginFilter() {
 		if(!$this->checkRequirements(array('ldapHost',
 										   'ldapPort',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapBase',
 										   'ldapUserFilter',
 										   ))) {
 			return false;
 		}
+
 		$filter = $this->composeLdapFilter(self::LFILTER_LOGIN);
 		if(!$filter) {
 			throw new \Exception('Cannot create filter');
@@ -377,8 +356,6 @@ class Wizard extends LDAPUtility {
 	 */
 	public function guessPortAndTLS() {
 		if(!$this->checkRequirements(array('ldapHost',
-										   'ldapAgentName',
-										   'ldapAgentPassword'
 										   ))) {
 			return false;
 		}
@@ -418,8 +395,6 @@ class Wizard extends LDAPUtility {
 	 */
 	public function guessBaseDN() {
 		if(!$this->checkRequirements(array('ldapHost',
-										   'ldapAgentName',
-										   'ldapAgentPassword',
 										   'ldapPort',
 										   ))) {
 			return false;
@@ -799,7 +774,22 @@ class Wizard extends LDAPUtility {
 		throw new \Exception($error);
 	}
 
+	/**
+	 * @brief checks whether a valid combination of agent and password has been
+	 * provided (either two values or nothing for anonymous connect)
+	 * @return boolean, true if everything is fine, false otherwise
+	 *
+	 */
+	private function checkAgentRequirements() {
+		$agent = $this->configuration->ldapAgentName;
+		$pwd = $this->configuration->ldapAgentPassword;
+
+		return ( (!empty($agent) && !empty($pwd))
+		       || (empty($agent) &&  empty($pwd)));
+	}
+
 	private function checkRequirements($reqs) {
+		$this->checkAgentRequirements();
 		foreach($reqs as $option) {
 			$value = $this->configuration->$option;
 			if(empty($value)) {
@@ -1005,9 +995,9 @@ class Wizard extends LDAPUtility {
 		//In case the port is already provided, we will check this first
 		if($port > 0) {
 			$hostInfo = parse_url($host);
-			if(is_array($hostInfo)
+			if(!(is_array($hostInfo)
 				&& isset($hostInfo['scheme'])
-				&& stripos($hostInfo['scheme'], 'ldaps') === false) {
+				&& stripos($hostInfo['scheme'], 'ldaps') !== false)) {
 				$portSettings[] = array('port' => $port, 'tls' => true);
 			}
 			$portSettings[] =array('port' => $port, 'tls' => false);
diff --git a/apps/user_ldap/templates/part.wizard-server.php b/apps/user_ldap/templates/part.wizard-server.php
index 01dd8d0fcb2..fde7c36cdd1 100644
--- a/apps/user_ldap/templates/part.wizard-server.php
+++ b/apps/user_ldap/templates/part.wizard-server.php
@@ -32,7 +32,7 @@
 							/>
 						<span>
 							<input type="number" id="ldap_port" name="ldap_port"
-								class="invisible lwautosave"
+								class="lwautosave"
 								placeholder="<?php p($l->t('Port'));?>" />
 						</span>
 					</div>
@@ -56,7 +56,7 @@
 
 			<div class="tablerow">
 				<textarea id="ldap_base" name="ldap_base"
-					class="tablecell invisible lwautosave"
+					class="tablecell lwautosave"
 					placeholder="<?php p($l->t('One Base DN per line'));?>"
 					title="<?php p($l->t('You can specify Base DN for users and groups in the Advanced tab'));?>">
 				</textarea>
diff --git a/apps/user_ldap/templates/settings.php b/apps/user_ldap/templates/settings.php
index 22aab0186f7..feb5ac6385d 100644
--- a/apps/user_ldap/templates/settings.php
+++ b/apps/user_ldap/templates/settings.php
@@ -20,8 +20,7 @@
 			<h3><?php p($l->t('Connection Settings'));?></h3>
 			<div>
 				<p><label for="ldap_configuration_active"><?php p($l->t('Configuration Active'));?></label><input type="checkbox" id="ldap_configuration_active" name="ldap_configuration_active" value="1" data-default="<?php p($_['ldap_configuration_active_default']); ?>"  title="<?php p($l->t('When unchecked, this configuration will be skipped.'));?>" /></p>
-				<p><label for="ldap_login_filter"><?php p($l->t('User Login Filter'));?></label>
-				<input type="text" id="ldap_login_filter" name="ldap_login_filter"
+				<p><label for="ldap_login_filter"><?php p($l->t('User Login Filter'));?></label><input type="text" id="ldap_login_filter" name="ldap_login_filter"
 					data-default="<?php p($_['ldap_login_filter_default']); ?>"
 					title="<?php p($l->t('Defines the filter to apply, when login is attempted. %%uid replaces the username in the login action. Example: "uid=%%uid"'));?>" /></p>
 				<p><label for="ldap_backup_host"><?php p($l->t('Backup (Replica) Host'));?></label><input type="text" id="ldap_backup_host" name="ldap_backup_host" data-default="<?php p($_['ldap_backup_host_default']); ?>" title="<?php p($l->t('Give an optional backup host. It must be a replica of the main LDAP/AD server.'));?>"></p>