Merge pull request #8532 from nextcloud/8499-stable13

[stable13]  Avoid fruitless login attempts
This commit is contained in:
Morris Jobke 2018-02-26 15:10:52 +01:00 committed by GitHub
commit 88ece3f5d7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 36 additions and 7 deletions

View File

@ -86,6 +86,8 @@ class Connection extends LDAPUtility {
protected $ignoreValidation = false; protected $ignoreValidation = false;
protected $bindResult = [];
/** /**
* Constructor * Constructor
* @param ILDAPWrapper $ldap * @param ILDAPWrapper $ldap
@ -113,7 +115,8 @@ class Connection extends LDAPUtility {
public function __destruct() { public function __destruct() {
if(!$this->dontDestruct && $this->ldap->isResource($this->ldapConnectionRes)) { if(!$this->dontDestruct && $this->ldap->isResource($this->ldapConnectionRes)) {
@$this->ldap->unbind($this->ldapConnectionRes); @$this->ldap->unbind($this->ldapConnectionRes);
}; }
$this->bindResult = [];
} }
/** /**
@ -202,6 +205,7 @@ class Connection extends LDAPUtility {
if(!is_null($this->ldapConnectionRes)) { if(!is_null($this->ldapConnectionRes)) {
@$this->ldap->unbind($this->ldapConnectionRes); @$this->ldap->unbind($this->ldapConnectionRes);
$this->ldapConnectionRes = null; $this->ldapConnectionRes = null;
$this->bindResult = [];
} }
} }
@ -560,6 +564,7 @@ class Connection extends LDAPUtility {
if($isBackupHost && ($error !== 0 || $isOverrideMainServer)) { if($isBackupHost && ($error !== 0 || $isOverrideMainServer)) {
$this->doConnect($this->configuration->ldapBackupHost, $this->doConnect($this->configuration->ldapBackupHost,
$this->configuration->ldapBackupPort); $this->configuration->ldapBackupPort);
$this->bindResult = [];
$bindStatus = $this->bind(); $bindStatus = $this->bind();
$error = $this->ldap->isResource($this->ldapConnectionRes) ? $error = $this->ldap->isResource($this->ldapConnectionRes) ?
$this->ldap->errno($this->ldapConnectionRes) : -1; $this->ldap->errno($this->ldapConnectionRes) : -1;
@ -612,13 +617,35 @@ class Connection extends LDAPUtility {
if(!$this->configuration->ldapConfigurationActive) { if(!$this->configuration->ldapConfigurationActive) {
return false; return false;
} }
$cr = $this->getConnectionResource(); $cr = $this->ldapConnectionRes;
if(!$this->ldap->isResource($cr)) { 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, $ldapLogin = @$this->ldap->bind($cr,
$this->configuration->ldapAgentName, $this->configuration->ldapAgentName,
$this->configuration->ldapAgentPassword); $this->configuration->ldapAgentPassword);
$this->bindResult = [
'dn' => $this->configuration->ldapAgentName,
'hash' => \OC::$server->getHasher()->hash($this->configPrefix . $this->configuration->ldapAgentPassword),
'result' => $ldapLogin,
];
if(!$ldapLogin) { if(!$ldapLogin) {
$errno = $this->ldap->errno($cr); $errno = $this->ldap->errno($cr);

View File

@ -174,7 +174,7 @@ class ConnectionTest extends \Test\TestCase {
->method('connect') ->method('connect')
->will($this->returnValue('ldapResource')); ->will($this->returnValue('ldapResource'));
$this->ldap->expects($this->exactly(2)) $this->ldap->expects($this->once())
->method('bind') ->method('bind')
->will($this->returnValue(false)); ->will($this->returnValue(false));

View File

@ -256,13 +256,15 @@ class LoginController extends Controller {
$users = $this->userManager->getByEmail($user); $users = $this->userManager->getByEmail($user);
// we only allow login by email if unique // we only allow login by email if unique
if (count($users) === 1) { if (count($users) === 1) {
$previousUser = $user;
$user = $users[0]->getUID(); $user = $users[0]->getUID();
if($user !== $previousUser) {
$loginResult = $this->userManager->checkPassword($user, $password); $loginResult = $this->userManager->checkPassword($user, $password);
} else { }
$this->logger->warning('Login failed: \''. $user .'\' (Remote IP: \''. $this->request->getRemoteAddress(). '\')', ['app' => 'core']);
} }
} }
if ($loginResult === false) { 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 // 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] : []; $args = !is_null($user) ? ['user' => $originalUser] : [];
if (!is_null($redirect_url)) { if (!is_null($redirect_url)) {