diff --git a/apps/user_ldap/lib/Connection.php b/apps/user_ldap/lib/Connection.php index e6a01a17d2..6140aa297b 100644 --- a/apps/user_ldap/lib/Connection.php +++ b/apps/user_ldap/lib/Connection.php @@ -511,6 +511,8 @@ class Connection extends LDAPUtility { /** * Connects and Binds to LDAP + * + * @throws ServerNotAvailableException */ private function establishConnection() { if(!$this->configuration->ldapConfigurationActive) { @@ -557,18 +559,12 @@ class Connection extends LDAPUtility { || $this->getFromCache('overrideMainServer')); $isBackupHost = (trim($this->configuration->ldapBackupHost) !== ""); $bindStatus = false; - $error = -1; try { if (!$isOverrideMainServer) { $this->doConnect($this->configuration->ldapHost, $this->configuration->ldapPort); - $bindStatus = $this->bind(); - $error = $this->ldap->isResource($this->ldapConnectionRes) ? - $this->ldap->errno($this->ldapConnectionRes) : -1; - } - if($bindStatus === true) { - return $bindStatus; } + return $this->bind(); } catch (ServerNotAvailableException $e) { if(!$isBackupHost) { throw $e; @@ -576,7 +572,7 @@ class Connection extends LDAPUtility { } //if LDAP server is not reachable, try the Backup (Replica!) Server - if($isBackupHost && ($error !== 0 || $isOverrideMainServer)) { + if($isBackupHost || $isOverrideMainServer) { $this->doConnect($this->configuration->ldapBackupHost, $this->configuration->ldapBackupPort); $this->bindResult = []; diff --git a/apps/user_ldap/tests/ConnectionTest.php b/apps/user_ldap/tests/ConnectionTest.php index cead84b05b..c6c1fe1192 100644 --- a/apps/user_ldap/tests/ConnectionTest.php +++ b/apps/user_ldap/tests/ConnectionTest.php @@ -38,7 +38,7 @@ use OCA\User_LDAP\ILDAPWrapper; * @package OCA\User_LDAP\Tests */ class ConnectionTest extends \Test\TestCase { - /** @var \OCA\User_LDAP\ILDAPWrapper */ + /** @var \OCA\User_LDAP\ILDAPWrapper|\PHPUnit_Framework_MockObject_MockObject */ protected $ldap; /** @var Connection */ @@ -110,7 +110,7 @@ class ConnectionTest extends \Test\TestCase { ->method('setOption') ->will($this->returnValue(true)); - $this->ldap->expects($this->exactly(3)) + $this->ldap->expects($this->exactly(2)) ->method('connect') ->will($this->returnValue('ldapResource')); @@ -119,7 +119,7 @@ class ConnectionTest extends \Test\TestCase { ->will($this->returnValue(0)); // Not called often enough? Then, the fallback to the backup server is broken. - $this->connection->expects($this->exactly(4)) + $this->connection->expects($this->exactly(3)) ->method('getFromCache') ->with('overrideMainServer') ->will($this->onConsecutiveCalls(false, false, true, true)); @@ -145,6 +145,53 @@ class ConnectionTest extends \Test\TestCase { $this->connection->init(); } + public function testDontUseBackupServerOnFailedAuth() { + $mainHost = 'ldap://nixda.ldap'; + $backupHost = 'ldap://fallback.ldap'; + $config = [ + 'ldapConfigurationActive' => true, + 'ldapHost' => $mainHost, + 'ldapPort' => 389, + 'ldapBackupHost' => $backupHost, + 'ldapBackupPort' => 389, + 'ldapAgentName' => 'uid=agent', + 'ldapAgentPassword' => 'SuchASecret' + ]; + + $this->connection->setIgnoreValidation(true); + $this->connection->setConfiguration($config); + + $this->ldap->expects($this->any()) + ->method('isResource') + ->will($this->returnValue(true)); + + $this->ldap->expects($this->any()) + ->method('setOption') + ->will($this->returnValue(true)); + + $this->ldap->expects($this->once()) + ->method('connect') + ->will($this->returnValue('ldapResource')); + + $this->ldap->expects($this->any()) + ->method('errno') + ->will($this->returnValue(49)); + + $this->connection->expects($this->any()) + ->method('getFromCache') + ->with('overrideMainServer') + ->willReturn(false); + + $this->connection->expects($this->never()) + ->method('writeToCache'); + + $this->ldap->expects($this->exactly(1)) + ->method('bind') + ->willReturn(false); + + $this->connection->init(); + } + public function testBindWithInvalidCredentials() { // background: Bind with invalid credentials should return false // and not throw a ServerNotAvailableException.