Merge pull request #16181 from owncloud/fix-15982

catch unallowed anonymous auth attempt and show specific error

6 files changed, 78 insertions, 14 deletions

diff --git a/apps/user_ldap/ajax/testConfiguration.php b/apps/user_ldap/ajax/testConfiguration.php
index 31f72a38e0b..f5fd5f23b87 100644
--- a/apps/user_ldap/ajax/testConfiguration.php
+++ b/apps/user_ldap/ajax/testConfiguration.php
@@ -34,16 +34,38 @@ $ldapWrapper = new OCA\user_ldap\lib\LDAP();
 $connection = new \OCA\user_ldap\lib\Connection($ldapWrapper, '', null);
 //needs to be true, otherwise it will also fail with an irritating message
 $_POST['ldap_configuration_active'] = 1;
-if($connection->setConfiguration($_POST)) {
-	//Configuration is okay
-	if($connection->bind()) {
-		OCP\JSON::success(array('message'
+
+try {
+	if ($connection->setConfiguration($_POST)) {
+		//Configuration is okay
+		if ($connection->bind()) {
+			/*
+			 * This shiny if block is an ugly hack to find out whether anonymous
+			 * bind is possible on AD or not. Because AD happily and constantly
+			 * replies with success to any anonymous bind request, we need to
+			 * fire up a broken operation. If AD does not allow anonymous bind,
+			 * it will end up with LDAP error code 1 which is turned into an
+			 * exception by the LDAP wrapper. We catch this. Other cases may
+			 * pass (like e.g. expected syntax error).
+			 */
+			try {
+				$ldapWrapper->read($connection->getConnectionResource(), 'neverwhere', 'objectClass=*', array('dn'));
+			} catch (\Exception $e) {
+				if($e->getCode() === 1) {
+					OCP\JSON::error(array('message' => $l->t('The configuration is invalid: anonymous bind is not allowed.')));
+					exit;
+				}
+			}
+			OCP\JSON::success(array('message'
 			=> $l->t('The configuration is valid and the connection could be established!')));
+		} else {
+			OCP\JSON::error(array('message'
+			=> $l->t('The configuration is valid, but the Bind failed. Please check the server settings and credentials.')));
+		}
 	} else {
 		OCP\JSON::error(array('message'
-			=> $l->t('The configuration is valid, but the Bind failed. Please check the server settings and credentials.')));
-	}
-} else {
-	OCP\JSON::error(array('message'
 		=> $l->t('The configuration is invalid. Please have a look at the logs for further details.')));
+	}
+} catch (\Exception $e) {
+	OCP\JSON::error(array('message' => $e->getMessage()));
 }
diff --git a/apps/user_ldap/js/wizard/wizardTabElementary.js b/apps/user_ldap/js/wizard/wizardTabElementary.js
index b8ab367dfd1..75664275a9c 100644
--- a/apps/user_ldap/js/wizard/wizardTabElementary.js
+++ b/apps/user_ldap/js/wizard/wizardTabElementary.js
@@ -165,6 +165,12 @@ OCA = OCA || {};
 		 * @inheritdoc
 		 */
 		overrideErrorMessage: function(message, key) {
+			var original = message;
+			message = this._super(message, key);
+			if(original !== message) {
+				// we pass the parents change
+				return message;
+			}
 			switch(key) {
 				case 'ldap_port':
 					if (message === 'Invalid credentials') {
@@ -267,7 +273,8 @@ OCA = OCA || {};
 						message = t('user_ldap', objectsFound + ' entries available within the provided Base DN');
 					}
 				} else {
-					message = t('user_ldap', 'An error occurred. Please check the Base DN, as well as connection settings and credentials.');
+					message = view.overrideErrorMessage(payload.data.message);
+					message = message || t('user_ldap', 'An error occurred. Please check the Base DN, as well as connection settings and credentials.');
 					if(payload.data.message) {
 						console.warn(payload.data.message);
 					}
diff --git a/apps/user_ldap/js/wizard/wizardTabGeneric.js b/apps/user_ldap/js/wizard/wizardTabGeneric.js
index 720628fa609..b755f3ca060 100644
--- a/apps/user_ldap/js/wizard/wizardTabGeneric.js
+++ b/apps/user_ldap/js/wizard/wizardTabGeneric.js
@@ -70,6 +70,17 @@ OCA = OCA || {};
 		 * @returns {string}
 		 */
 		overrideErrorMessage: function(message, key) {
+			if(message === 'LDAP authentication method rejected'
+				&& !this.configModel.configuration.ldap_dn)
+			{
+				message = t('user_ldap', 'Anonymous bind is not allowed. Please provide a User DN and Password.');
+			} else if (message === 'LDAP Operations error'
+				&& !this.configModel.configuration.ldap_dn
+				&& !this.configModel.configuration.ldap_agent_password)
+			{
+				message = t('user_ldap', 'LDAP Operations error. Anonymous bind might not be allowed.');
+			}
+
 			return message;
 		},
 
diff --git a/apps/user_ldap/js/wizard/wizardTabUserFilter.js b/apps/user_ldap/js/wizard/wizardTabUserFilter.js
index 992c1ccf379..4fe223ee075 100644
--- a/apps/user_ldap/js/wizard/wizardTabUserFilter.js
+++ b/apps/user_ldap/js/wizard/wizardTabUserFilter.js
@@ -122,6 +122,12 @@ OCA = OCA || {};
 		 * @inheritdoc
 		 */
 		overrideErrorMessage: function(message, key) {
+			var original = message;
+			message = this._super(message, key);
+			if(original !== message) {
+				// we pass the parents change
+				return message;
+			}
 			if(   key === 'ldap_userfilter_groups'
 			   && message === 'memberOf is not supported by the server'
 			) {
diff --git a/apps/user_ldap/lib/ldap.php b/apps/user_ldap/lib/ldap.php
index 74df3dd8ae7..4d45db2e155 100644
--- a/apps/user_ldap/lib/ldap.php
+++ b/apps/user_ldap/lib/ldap.php
@@ -287,6 +287,10 @@ class LDAP implements ILDAPWrapper {
 					//referrals, we switch them off, but then there is AD :)
 				} else if ($errorCode === -1) {
 					throw new ServerNotAvailableException('Lost connection to LDAP server.');
+				} else if ($errorCode === 48) {
+					throw new \Exception('LDAP authentication method rejected', $errorCode);
+				} else if ($errorCode === 1) {
+					throw new \Exception('LDAP Operations error', $errorCode);
 				} else {
 					\OCP\Util::writeLog('user_ldap',
 										'LDAP error '.$errorMsg.' (' .
diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php
index 824923eecbf..6c39f406e83 100644
--- a/apps/user_ldap/lib/wizard.php
+++ b/apps/user_ldap/lib/wizard.php
@@ -657,12 +657,26 @@ class Wizard extends LDAPUtility {
 			\OCP\Util::writeLog('user_ldap', 'Wiz: trying port '. $p . ', TLS '. $t, \OCP\Util::DEBUG);
 			//connectAndBind may throw Exception, it needs to be catched by the
 			//callee of this method
-			if($this->connectAndBind($p, $t) === true) {
-				$config = array('ldapPort' => $p,
-								'ldapTLS'  => intval($t)
-							);
+
+			// unallowed anonymous bind throws 48. But if it throws 48, we
+			// detected port and TLS, i.e. it is successful.
+			try {
+				$settingsFound = $this->connectAndBind($p, $t);
+			} catch (\Exception $e) {
+				if($e->getCode() === 48) {
+					$settingsFound = true;
+				} else {
+					throw $e;
+				}
+			}
+
+			if ($settingsFound === true) {
+				$config = array(
+					'ldapPort' => $p,
+					'ldapTLS' => intval($t)
+				);
 				$this->configuration->setConfiguration($config);
-				\OCP\Util::writeLog('user_ldap', 'Wiz: detected Port '. $p, \OCP\Util::DEBUG);
+				\OCP\Util::writeLog('user_ldap', 'Wiz: detected Port ' . $p, \OCP\Util::DEBUG);
 				$this->result->addChange('ldap_port', $p);
 				return $this->result;
 			}