Skip to content

Commit 88ece3f

Browse files
authored
Merge pull request #8532 from nextcloud/8499-stable13
[stable13] Avoid fruitless login attempts
2 parents bf2f744 + fb2ebbd commit 88ece3f

3 files changed

Lines changed: 36 additions & 7 deletions

File tree

apps/user_ldap/lib/Connection.php

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ class Connection extends LDAPUtility {
8686

8787
protected $ignoreValidation = false;
8888

89+
protected $bindResult = [];
90+
8991
/**
9092
* Constructor
9193
* @param ILDAPWrapper $ldap
@@ -113,7 +115,8 @@ public function __construct(ILDAPWrapper $ldap, $configPrefix = '', $configID =
113115
public function __destruct() {
114116
if(!$this->dontDestruct && $this->ldap->isResource($this->ldapConnectionRes)) {
115117
@$this->ldap->unbind($this->ldapConnectionRes);
116-
};
118+
}
119+
$this->bindResult = [];
117120
}
118121

119122
/**
@@ -202,6 +205,7 @@ public function resetConnectionResource() {
202205
if(!is_null($this->ldapConnectionRes)) {
203206
@$this->ldap->unbind($this->ldapConnectionRes);
204207
$this->ldapConnectionRes = null;
208+
$this->bindResult = [];
205209
}
206210
}
207211

@@ -560,6 +564,7 @@ private function establishConnection() {
560564
if($isBackupHost && ($error !== 0 || $isOverrideMainServer)) {
561565
$this->doConnect($this->configuration->ldapBackupHost,
562566
$this->configuration->ldapBackupPort);
567+
$this->bindResult = [];
563568
$bindStatus = $this->bind();
564569
$error = $this->ldap->isResource($this->ldapConnectionRes) ?
565570
$this->ldap->errno($this->ldapConnectionRes) : -1;
@@ -612,13 +617,35 @@ public function bind() {
612617
if(!$this->configuration->ldapConfigurationActive) {
613618
return false;
614619
}
615-
$cr = $this->getConnectionResource();
620+
$cr = $this->ldapConnectionRes;
616621
if(!$this->ldap->isResource($cr)) {
617-
return false;
622+
$cr = $this->getConnectionResource();
618623
}
624+
625+
if(
626+
count($this->bindResult) !== 0
627+
&& $this->bindResult['dn'] === $this->configuration->ldapAgentName
628+
&& \OC::$server->getHasher()->verify(
629+
$this->configPrefix . $this->configuration->ldapAgentPassword,
630+
$this->bindResult['hash']
631+
)
632+
) {
633+
// don't attempt to bind again with the same data as before
634+
// bind might have been invoked via getConnectionResource(),
635+
// but we need results specifically for e.g. user login
636+
return $this->bindResult['result'];
637+
}
638+
619639
$ldapLogin = @$this->ldap->bind($cr,
620640
$this->configuration->ldapAgentName,
621641
$this->configuration->ldapAgentPassword);
642+
643+
$this->bindResult = [
644+
'dn' => $this->configuration->ldapAgentName,
645+
'hash' => \OC::$server->getHasher()->hash($this->configPrefix . $this->configuration->ldapAgentPassword),
646+
'result' => $ldapLogin,
647+
];
648+
622649
if(!$ldapLogin) {
623650
$errno = $this->ldap->errno($cr);
624651

apps/user_ldap/tests/ConnectionTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public function testBindWithInvalidCredentials() {
174174
->method('connect')
175175
->will($this->returnValue('ldapResource'));
176176

177-
$this->ldap->expects($this->exactly(2))
177+
$this->ldap->expects($this->once())
178178
->method('bind')
179179
->will($this->returnValue(false));
180180

core/Controller/LoginController.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,13 +256,15 @@ public function tryLogin($user, $password, $redirect_url, $remember_login = fals
256256
$users = $this->userManager->getByEmail($user);
257257
// we only allow login by email if unique
258258
if (count($users) === 1) {
259+
$previousUser = $user;
259260
$user = $users[0]->getUID();
260-
$loginResult = $this->userManager->checkPassword($user, $password);
261-
} else {
262-
$this->logger->warning('Login failed: \''. $user .'\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', ['app' => 'core']);
261+
if($user !== $previousUser) {
262+
$loginResult = $this->userManager->checkPassword($user, $password);
263+
}
263264
}
264265
}
265266
if ($loginResult === false) {
267+
$this->logger->warning('Login failed: \''. $user .'\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', ['app' => 'core']);
266268
// Read current user and append if possible - we need to return the unmodified user otherwise we will leak the login name
267269
$args = !is_null($user) ? ['user' => $originalUser] : [];
268270
if (!is_null($redirect_url)) {

0 commit comments

Comments
 (0)