From f015d38a89eedef02c713a8623ad2b03c2b3e728 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 22 Feb 2018 12:45:28 +0100 Subject: [PATCH 1/2] track the state of the bind result Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Connection.php | 33 ++++++++++++++++++++++--- apps/user_ldap/tests/ConnectionTest.php | 2 +- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index c73a35e6bf..5105c12a22 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -86,6 +86,8 @@ class Connection extends LDAPUtility { protected $ignoreValidation = false; + protected $bindResult = []; + /** * Constructor * @param ILDAPWrapper $ldap @@ -113,7 +115,8 @@ class Connection extends LDAPUtility { public function __destruct() { if(!$this->dontDestruct && $this->ldap->isResource($this->ldapConnectionRes)) { @$this->ldap->unbind($this->ldapConnectionRes); - }; + } + $this->bindResult = []; } /** @@ -202,6 +205,7 @@ class Connection extends LDAPUtility { if(!is_null($this->ldapConnectionRes)) { @$this->ldap->unbind($this->ldapConnectionRes); $this->ldapConnectionRes = null; + $this->bindResult = []; } } @@ -560,6 +564,7 @@ class Connection extends LDAPUtility { if($isBackupHost && ($error !== 0 || $isOverrideMainServer)) { $this->doConnect($this->configuration->ldapBackupHost, $this->configuration->ldapBackupPort); + $this->bindResult = []; $bindStatus = $this->bind(); $error = $this->ldap->isResource($this->ldapConnectionRes) ? $this->ldap->errno($this->ldapConnectionRes) : -1; @@ -612,13 +617,35 @@ class Connection extends LDAPUtility { if(!$this->configuration->ldapConfigurationActive) { return false; } - $cr = $this->getConnectionResource(); + $cr = $this->ldapConnectionRes; if(!$this->ldap->isResource($cr)) { - return false; + $cr = $this->getConnectionResource(); } + + if( + count($this->bindResult) !== 0 + && $this->bindResult['dn'] === $this->configuration->ldapAgentName + && \OC::$server->getHasher()->verify( + $this->configPrefix . $this->configuration->ldapAgentPassword, + $this->bindResult['hash'] + ) + ) { + // don't attempt to bind again with the same data as before + // bind might have been invoked via getConnectionResource(), + // but we need results specifically for e.g. user login + return $this->bindResult['result']; + } + $ldapLogin = @$this->ldap->bind($cr, $this->configuration->ldapAgentName, $this->configuration->ldapAgentPassword); + + $this->bindResult = [ + 'dn' => $this->configuration->ldapAgentName, + 'hash' => \OC::$server->getHasher()->hash($this->configPrefix . $this->configuration->ldapAgentPassword), + 'result' => $ldapLogin, + ]; + if(!$ldapLogin) { $errno = $this->ldap->errno($cr); diff --git a/apps/user_ldap/tests/ConnectionTest.php b/apps/user_ldap/tests/ConnectionTest.php index c0f91d25d3..cead84b05b 100644 --- a/apps/user_ldap/tests/ConnectionTest.php +++ b/apps/user_ldap/tests/ConnectionTest.php @@ -174,7 +174,7 @@ class ConnectionTest extends \Test\TestCase { ->method('connect') ->will($this->returnValue('ldapResource')); - $this->ldap->expects($this->exactly(2)) + $this->ldap->expects($this->once()) ->method('bind') ->will($this->returnValue(false)); From fb2ebbd23266a54b2430046913bda88996df40d4 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 22 Feb 2018 12:46:06 +0100 Subject: [PATCH 2/2] don't try login with the same name that just failed Signed-off-by: Arthur Schiwon --- core/Controller/LoginController.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index e53095a7de..9b9c5bcd4e 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -256,13 +256,15 @@ class LoginController extends Controller { $users = $this->userManager->getByEmail($user); // we only allow login by email if unique if (count($users) === 1) { + $previousUser = $user; $user = $users[0]->getUID(); - $loginResult = $this->userManager->checkPassword($user, $password); - } else { - $this->logger->warning('Login failed: \''. $user .'\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', ['app' => 'core']); + if($user !== $previousUser) { + $loginResult = $this->userManager->checkPassword($user, $password); + } } } if ($loginResult === false) { + $this->logger->warning('Login failed: \''. $user .'\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', ['app' => 'core']); // Read current user and append if possible - we need to return the unmodified user otherwise we will leak the login name $args = !is_null($user) ? ['user' => $originalUser] : []; if (!is_null($redirect_url)) {