From b9e53097577499b519f9fcdd053421cb1507bab3 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 7 May 2015 21:09:10 +0200 Subject: [PATCH 1/3] catch unallowed anonymous auth attempt and show specific error --- apps/user_ldap/ajax/testConfiguration.php | 21 ++++++++++++------- .../js/wizard/wizardTabElementary.js | 9 +++++++- apps/user_ldap/js/wizard/wizardTabGeneric.js | 5 +++++ .../js/wizard/wizardTabUserFilter.js | 6 ++++++ apps/user_ldap/lib/ldap.php | 2 ++ 5 files changed, 34 insertions(+), 9 deletions(-) diff --git a/apps/user_ldap/ajax/testConfiguration.php b/apps/user_ldap/ajax/testConfiguration.php index 31f72a38e0..289957764a 100644 --- a/apps/user_ldap/ajax/testConfiguration.php +++ b/apps/user_ldap/ajax/testConfiguration.php @@ -34,16 +34,21 @@ $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()) { + 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 b8ab367dfd..75664275a9 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 720628fa60..c272df7e3c 100644 --- a/apps/user_ldap/js/wizard/wizardTabGeneric.js +++ b/apps/user_ldap/js/wizard/wizardTabGeneric.js @@ -70,6 +70,11 @@ 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.'); + } return message; }, diff --git a/apps/user_ldap/js/wizard/wizardTabUserFilter.js b/apps/user_ldap/js/wizard/wizardTabUserFilter.js index 992c1ccf37..4fe223ee07 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 74df3dd8ae..48852a3a49 100644 --- a/apps/user_ldap/lib/ldap.php +++ b/apps/user_ldap/lib/ldap.php @@ -287,6 +287,8 @@ 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'); } else { \OCP\Util::writeLog('user_ldap', 'LDAP error '.$errorMsg.' (' . From 5a563936579110bfa7d333ae8f32121cbc36cc7d Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Fri, 8 May 2015 17:15:29 +0200 Subject: [PATCH 2/3] throw exception on LDAP error 1, which we usually do not see and is pretty generic. AD uses is for uses not enlisted in the RFC, like on issues with anonymous binds. we also try to guess this case and show a hint. --- apps/user_ldap/ajax/testConfiguration.php | 17 +++++++++++++++++ apps/user_ldap/js/wizard/wizardTabGeneric.js | 6 ++++++ apps/user_ldap/lib/ldap.php | 2 ++ 3 files changed, 25 insertions(+) diff --git a/apps/user_ldap/ajax/testConfiguration.php b/apps/user_ldap/ajax/testConfiguration.php index 289957764a..f5fd5f23b8 100644 --- a/apps/user_ldap/ajax/testConfiguration.php +++ b/apps/user_ldap/ajax/testConfiguration.php @@ -39,6 +39,23 @@ 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 { diff --git a/apps/user_ldap/js/wizard/wizardTabGeneric.js b/apps/user_ldap/js/wizard/wizardTabGeneric.js index c272df7e3c..b755f3ca06 100644 --- a/apps/user_ldap/js/wizard/wizardTabGeneric.js +++ b/apps/user_ldap/js/wizard/wizardTabGeneric.js @@ -74,7 +74,13 @@ OCA = OCA || {}; && !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/lib/ldap.php b/apps/user_ldap/lib/ldap.php index 48852a3a49..8d2c493a4d 100644 --- a/apps/user_ldap/lib/ldap.php +++ b/apps/user_ldap/lib/ldap.php @@ -289,6 +289,8 @@ class LDAP implements ILDAPWrapper { throw new ServerNotAvailableException('Lost connection to LDAP server.'); } else if ($errorCode === 48) { throw new \Exception('LDAP authentication method rejected'); + } else if ($errorCode === 1) { + throw new \Exception('LDAP Operations error', $errorCode); } else { \OCP\Util::writeLog('user_ldap', 'LDAP error '.$errorMsg.' (' . From cdb068933414657d2e9316cd030279d3529bbd9b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 12 May 2015 14:45:32 +0200 Subject: [PATCH 3/3] handle unallowed auth exception on port detection --- apps/user_ldap/lib/ldap.php | 2 +- apps/user_ldap/lib/wizard.php | 24 +++++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/apps/user_ldap/lib/ldap.php b/apps/user_ldap/lib/ldap.php index 8d2c493a4d..4d45db2e15 100644 --- a/apps/user_ldap/lib/ldap.php +++ b/apps/user_ldap/lib/ldap.php @@ -288,7 +288,7 @@ class LDAP implements ILDAPWrapper { } else if ($errorCode === -1) { throw new ServerNotAvailableException('Lost connection to LDAP server.'); } else if ($errorCode === 48) { - throw new \Exception('LDAP authentication method rejected'); + throw new \Exception('LDAP authentication method rejected', $errorCode); } else if ($errorCode === 1) { throw new \Exception('LDAP Operations error', $errorCode); } else { diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 824923eecb..6c39f406e8 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; }