Merge pull request #47686 from nextcloud/fix/move-email-logic-local-user-backend

fix: Move login via email logic to local backend
pull/50702/head^2
Côme Chilliet 2 months ago committed by GitHub
commit 2ef04bfb5d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 1
      lib/composer/composer/autoload_classmap.php
  2. 1
      lib/composer/composer/autoload_static.php
  3. 2
      lib/private/Authentication/Login/Chain.php
  4. 53
      lib/private/Authentication/Login/EmailLoginCommand.php
  5. 263
      lib/private/User/Database.php
  6. 148
      tests/lib/Authentication/Login/EmailLoginCommandTest.php

@ -1078,7 +1078,6 @@ return array(
'OC\\Authentication\\Login\\ClearLostPasswordTokensCommand' => $baseDir . '/lib/private/Authentication/Login/ClearLostPasswordTokensCommand.php',
'OC\\Authentication\\Login\\CompleteLoginCommand' => $baseDir . '/lib/private/Authentication/Login/CompleteLoginCommand.php',
'OC\\Authentication\\Login\\CreateSessionTokenCommand' => $baseDir . '/lib/private/Authentication/Login/CreateSessionTokenCommand.php',
'OC\\Authentication\\Login\\EmailLoginCommand' => $baseDir . '/lib/private/Authentication/Login/EmailLoginCommand.php',
'OC\\Authentication\\Login\\FinishRememberedLoginCommand' => $baseDir . '/lib/private/Authentication/Login/FinishRememberedLoginCommand.php',
'OC\\Authentication\\Login\\FlowV2EphemeralSessionsCommand' => $baseDir . '/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php',
'OC\\Authentication\\Login\\LoggedInCheckCommand' => $baseDir . '/lib/private/Authentication/Login/LoggedInCheckCommand.php',

@ -1127,7 +1127,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OC\\Authentication\\Login\\ClearLostPasswordTokensCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/ClearLostPasswordTokensCommand.php',
'OC\\Authentication\\Login\\CompleteLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/CompleteLoginCommand.php',
'OC\\Authentication\\Login\\CreateSessionTokenCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/CreateSessionTokenCommand.php',
'OC\\Authentication\\Login\\EmailLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/EmailLoginCommand.php',
'OC\\Authentication\\Login\\FinishRememberedLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/FinishRememberedLoginCommand.php',
'OC\\Authentication\\Login\\FlowV2EphemeralSessionsCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php',
'OC\\Authentication\\Login\\LoggedInCheckCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/LoggedInCheckCommand.php',

@ -13,7 +13,6 @@ class Chain {
private PreLoginHookCommand $preLoginHookCommand,
private UserDisabledCheckCommand $userDisabledCheckCommand,
private UidLoginCommand $uidLoginCommand,
private EmailLoginCommand $emailLoginCommand,
private LoggedInCheckCommand $loggedInCheckCommand,
private CompleteLoginCommand $completeLoginCommand,
private CreateSessionTokenCommand $createSessionTokenCommand,
@ -31,7 +30,6 @@ class Chain {
$chain
->setNext($this->userDisabledCheckCommand)
->setNext($this->uidLoginCommand)
->setNext($this->emailLoginCommand)
->setNext($this->loggedInCheckCommand)
->setNext($this->completeLoginCommand)
->setNext($this->flowV2EphemeralSessionsCommand)

@ -1,53 +0,0 @@
<?php
declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\Authentication\Login;
use OCP\IUserManager;
class EmailLoginCommand extends ALoginCommand {
/** @var IUserManager */
private $userManager;
public function __construct(IUserManager $userManager) {
$this->userManager = $userManager;
}
public function process(LoginData $loginData): LoginResult {
if ($loginData->getUser() === false) {
if (!filter_var($loginData->getUsername(), FILTER_VALIDATE_EMAIL)) {
return $this->processNextOrFinishSuccessfully($loginData);
}
$users = $this->userManager->getByEmail($loginData->getUsername());
// we only allow login by email if unique
if (count($users) === 1) {
// FIXME: This is a workaround to still stick to configured LDAP login filters
// this can be removed once the email login is properly implemented in the local user backend
// as described in https://github.com/nextcloud/server/issues/5221
if ($users[0]->getBackendClassName() === 'LDAP') {
return $this->processNextOrFinishSuccessfully($loginData);
}
$username = $users[0]->getUID();
if ($username !== $loginData->getUsername()) {
$user = $this->userManager->checkPassword(
$username,
$loginData->getPassword()
);
if ($user !== false) {
$loginData->setUser($user);
$loginData->setUsername($username);
}
}
}
}
return $this->processNextOrFinishSuccessfully($loginData);
}
}

@ -12,7 +12,9 @@ use InvalidArgumentException;
use OCP\AppFramework\Db\TTransactional;
use OCP\Cache\CappedMemoryCache;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IUserManager;
use OCP\Security\Events\ValidatePasswordPolicyEvent;
use OCP\Security\IHasher;
use OCP\User\Backend\ABackend;
@ -41,17 +43,12 @@ class Database extends ABackend implements
ISearchKnownUsersBackend,
IGetRealUIDBackend,
IPasswordHashBackend {
/** @var CappedMemoryCache */
private $cache;
/** @var IEventDispatcher */
private $eventDispatcher;
/** @var IDBConnection */
private $dbConn;
/** @var string */
private $table;
private CappedMemoryCache $cache;
private IConfig $config;
private ?IDBConnection $dbConnection;
private IEventDispatcher $eventDispatcher;
private string $table;
use TTransactional;
@ -65,15 +62,18 @@ class Database extends ABackend implements
$this->cache = new CappedMemoryCache();
$this->table = $table;
$this->eventDispatcher = $eventDispatcher ?? \OCP\Server::get(IEventDispatcher::class);
$this->config = \OCP\Server::get(IConfig::class);
$this->dbConnection = null;
}
/**
* FIXME: This function should not be required!
*/
private function fixDI() {
if ($this->dbConn === null) {
$this->dbConn = \OC::$server->getDatabaseConnection();
private function getDbConnection() {
if ($this->dbConnection === null) {
$this->dbConnection = \OCP\Server::get(IDBConnection::class);
}
return $this->dbConnection;
}
/**
@ -87,52 +87,54 @@ class Database extends ABackend implements
* itself, not in its subclasses.
*/
public function createUser(string $uid, string $password): bool {
$this->fixDI();
if (!$this->userExists($uid)) {
$this->eventDispatcher->dispatchTyped(new ValidatePasswordPolicyEvent($password));
return $this->atomic(function () use ($uid, $password) {
$qb = $this->dbConn->getQueryBuilder();
$qb->insert($this->table)
->values([
'uid' => $qb->createNamedParameter($uid),
'password' => $qb->createNamedParameter(\OCP\Server::get(IHasher::class)->hash($password)),
'uid_lower' => $qb->createNamedParameter(mb_strtolower($uid)),
]);
$result = $qb->executeStatement();
// Clear cache
unset($this->cache[$uid]);
// Repopulate the cache
$this->loadUser($uid);
return (bool)$result;
}, $this->dbConn);
if ($this->userExists($uid)) {
return false;
}
return false;
$this->eventDispatcher->dispatchTyped(new ValidatePasswordPolicyEvent($password));
$dbConn = $this->getDbConnection();
return $this->atomic(function () use ($uid, $password, $dbConn) {
$qb = $dbConn->getQueryBuilder();
$qb->insert($this->table)
->values([
'uid' => $qb->createNamedParameter($uid),
'password' => $qb->createNamedParameter(\OCP\Server::get(IHasher::class)->hash($password)),
'uid_lower' => $qb->createNamedParameter(mb_strtolower($uid)),
]);
$result = $qb->executeStatement();
// Clear cache
unset($this->cache[$uid]);
// Repopulate the cache
$this->loadUser($uid);
return (bool)$result;
}, $dbConn);
}
/**
* delete a user
* Deletes a user
*
* @param string $uid The username of the user to delete
* @return bool
*
* Deletes a user
*/
public function deleteUser($uid) {
$this->fixDI();
// Delete user-group-relation
$query = $this->dbConn->getQueryBuilder();
$dbConn = $this->getDbConnection();
$query = $dbConn->getQueryBuilder();
$query->delete($this->table)
->where($query->expr()->eq('uid_lower', $query->createNamedParameter(mb_strtolower($uid))));
$result = $query->executeStatement();
if (isset($this->cache[$uid])) {
// If the user logged in through email there is a second cache entry, also unset that.
$email = $this->cache[$uid]['email'] ?? null;
if ($email !== null) {
unset($this->cache[$email]);
}
// Unset the cache entry
unset($this->cache[$uid]);
}
@ -140,7 +142,8 @@ class Database extends ABackend implements
}
private function updatePassword(string $uid, string $passwordHash): bool {
$query = $this->dbConn->getQueryBuilder();
$dbConn = $this->getDbConnection();
$query = $dbConn->getQueryBuilder();
$query->update($this->table)
->set('password', $query->createNamedParameter($passwordHash))
->where($query->expr()->eq('uid_lower', $query->createNamedParameter(mb_strtolower($uid))));
@ -159,35 +162,34 @@ class Database extends ABackend implements
* Change the password of a user
*/
public function setPassword(string $uid, string $password): bool {
$this->fixDI();
if ($this->userExists($uid)) {
$this->eventDispatcher->dispatchTyped(new ValidatePasswordPolicyEvent($password));
if (!$this->userExists($uid)) {
return false;
}
$hasher = \OCP\Server::get(IHasher::class);
$hashedPassword = $hasher->hash($password);
$this->eventDispatcher->dispatchTyped(new ValidatePasswordPolicyEvent($password));
$return = $this->updatePassword($uid, $hashedPassword);
$hasher = \OCP\Server::get(IHasher::class);
$hashedPassword = $hasher->hash($password);
if ($return) {
$this->cache[$uid]['password'] = $hashedPassword;
}
$return = $this->updatePassword($uid, $hashedPassword);
return $return;
if ($return) {
$this->cache[$uid]['password'] = $hashedPassword;
}
return false;
return $return;
}
public function getPasswordHash(string $userId): ?string {
$this->fixDI();
if (!$this->userExists($userId)) {
return null;
}
if (!empty($this->cache[$userId]['password'])) {
return $this->cache[$userId]['password'];
}
$qb = $this->dbConn->getQueryBuilder();
$dbConn = $this->getDbConnection();
$qb = $dbConn->getQueryBuilder();
$qb->select('password')
->from($this->table)
->where($qb->expr()->eq('uid_lower', $qb->createNamedParameter(mb_strtolower($userId))));
@ -196,6 +198,7 @@ class Database extends ABackend implements
if ($hash === false) {
return null;
}
$this->cache[$userId]['password'] = $hash;
return $hash;
}
@ -204,11 +207,12 @@ class Database extends ABackend implements
if (!\OCP\Server::get(IHasher::class)->validate($passwordHash)) {
throw new InvalidArgumentException();
}
$this->fixDI();
$result = $this->updatePassword($userId, $passwordHash);
if (!$result) {
return false;
}
$this->cache[$userId]['password'] = $passwordHash;
return true;
}
@ -229,21 +233,20 @@ class Database extends ABackend implements
throw new \InvalidArgumentException('Invalid displayname');
}
$this->fixDI();
if ($this->userExists($uid)) {
$query = $this->dbConn->getQueryBuilder();
$query->update($this->table)
->set('displayname', $query->createNamedParameter($displayName))
->where($query->expr()->eq('uid_lower', $query->createNamedParameter(mb_strtolower($uid))));
$query->executeStatement();
if (!$this->userExists($uid)) {
return false;
}
$this->cache[$uid]['displayname'] = $displayName;
$dbConn = $this->getDbConnection();
$query = $dbConn->getQueryBuilder();
$query->update($this->table)
->set('displayname', $query->createNamedParameter($displayName))
->where($query->expr()->eq('uid_lower', $query->createNamedParameter(mb_strtolower($uid))));
$query->executeStatement();
return true;
}
$this->cache[$uid]['displayname'] = $displayName;
return false;
return true;
}
/**
@ -269,9 +272,8 @@ class Database extends ABackend implements
public function getDisplayNames($search = '', $limit = null, $offset = null) {
$limit = $this->fixLimit($limit);
$this->fixDI();
$query = $this->dbConn->getQueryBuilder();
$dbConn = $this->getDbConnection();
$query = $dbConn->getQueryBuilder();
$query->select('uid', 'displayname')
->from($this->table, 'u')
@ -281,9 +283,9 @@ class Database extends ABackend implements
$query->expr()->eq('configkey', $query->expr()->literal('email')))
)
// sqlite doesn't like re-using a single named parameter here
->where($query->expr()->iLike('uid', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')))
->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')))
->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $this->dbConn->escapeLikeParameter($search) . '%')))
->where($query->expr()->iLike('uid', $query->createPositionalParameter('%' . $dbConn->escapeLikeParameter($search) . '%')))
->orWhere($query->expr()->iLike('displayname', $query->createPositionalParameter('%' . $dbConn->escapeLikeParameter($search) . '%')))
->orWhere($query->expr()->iLike('configvalue', $query->createPositionalParameter('%' . $dbConn->escapeLikeParameter($search) . '%')))
->orderBy($query->func()->lower('displayname'), 'ASC')
->addOrderBy('uid_lower', 'ASC')
->setMaxResults($limit)
@ -309,9 +311,8 @@ class Database extends ABackend implements
public function searchKnownUsersByDisplayName(string $searcher, string $pattern, ?int $limit = null, ?int $offset = null): array {
$limit = $this->fixLimit($limit);
$this->fixDI();
$query = $this->dbConn->getQueryBuilder();
$dbConn = $this->getDbConnection();
$query = $dbConn->getQueryBuilder();
$query->select('u.uid', 'u.displayname')
->from($this->table, 'u')
@ -321,8 +322,8 @@ class Database extends ABackend implements
))
->where($query->expr()->eq('k.known_to', $query->createNamedParameter($searcher)))
->andWhere($query->expr()->orX(
$query->expr()->iLike('u.uid', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($pattern) . '%')),
$query->expr()->iLike('u.displayname', $query->createNamedParameter('%' . $this->dbConn->escapeLikeParameter($pattern) . '%'))
$query->expr()->iLike('u.uid', $query->createNamedParameter('%' . $dbConn->escapeLikeParameter($pattern) . '%')),
$query->expr()->iLike('u.displayname', $query->createNamedParameter('%' . $dbConn->escapeLikeParameter($pattern) . '%'))
))
->orderBy('u.displayname', 'ASC')
->addOrderBy('u.uid_lower', 'ASC')
@ -341,7 +342,7 @@ class Database extends ABackend implements
/**
* Check if the password is correct
*
* @param string $loginName The loginname
* @param string $loginName The login name
* @param string $password The password
* @return string
*
@ -368,46 +369,64 @@ class Database extends ABackend implements
/**
* Load an user in the cache
*
* @param string $uid the username
* @param string $loginName the username or email
* @return boolean true if user was found, false otherwise
*/
private function loadUser($uid) {
$this->fixDI();
private function loadUser(string $loginName, bool $tryEmail = true): bool {
if (isset($this->cache[$loginName])) {
return $this->cache[$loginName] !== false;
}
$uid = (string)$uid;
if (!isset($this->cache[$uid])) {
//guests $uid could be NULL or ''
if ($uid === '') {
$this->cache[$uid] = false;
return true;
}
//guests $uid could be NULL or ''
if ($loginName === '') {
$this->cache[$loginName] = false;
return false;
}
$dbConn = $this->getDbConnection();
$qb = $dbConn->getQueryBuilder();
$qb->select('uid', 'displayname', 'password')
->from($this->table)
->where(
$qb->expr()->eq(
'uid_lower', $qb->createNamedParameter(mb_strtolower($loginName))
)
);
$result = $qb->executeQuery();
$row = $result->fetch();
$result->closeCursor();
// "uid" is primary key, so there can only be a single result
if ($row !== false) {
$this->cache[$loginName] = [
'uid' => (string)$row['uid'],
'displayname' => (string)$row['displayname'],
'password' => (string)$row['password'],
];
return true;
}
$qb = $this->dbConn->getQueryBuilder();
$qb->select('uid', 'displayname', 'password')
->from($this->table)
->where(
$qb->expr()->eq(
'uid_lower', $qb->createNamedParameter(mb_strtolower($uid))
)
);
$result = $qb->executeQuery();
$row = $result->fetch();
$result->closeCursor();
// "uid" is primary key, so there can only be a single result
if ($row !== false) {
$this->cache[$uid] = [
'uid' => (string)$row['uid'],
'displayname' => (string)$row['displayname'],
'password' => (string)$row['password'],
];
} else {
$this->cache[$uid] = false;
return false;
// Not found by UID so we try also for email, load uid for email.
if ($tryEmail) {
/** @var string|null $uid Psalm does not get the type correct here */
[$uid] = [...$this->config->getUsersForUserValue('settings', 'email', mb_strtolower($loginName)), null];
// If found, try loading it
if ($uid !== null && $uid !== $loginName) {
$result = $this->loadUser($uid, false);
if ($result) {
// Also add cache result for the email
$this->cache[$loginName] = $this->cache[$uid];
// Set a reference to the uid cache entry for also delete email entry on user delete
$this->cache[$uid]['email'] = $loginName;
return true;
}
}
}
return true;
// Not found by uid nor email, so cache as not existing
$this->cache[$loginName] = false;
return false;
}
/**
@ -436,8 +455,7 @@ class Database extends ABackend implements
* @return boolean
*/
public function userExists($uid) {
$this->loadUser($uid);
return $this->cache[$uid] !== false;
return $this->loadUser($uid);
}
/**
@ -448,7 +466,7 @@ class Database extends ABackend implements
*/
public function getHome(string $uid) {
if ($this->userExists($uid)) {
return \OC::$server->getConfig()->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $uid;
return $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data') . '/' . $uid;
}
return false;
@ -465,9 +483,8 @@ class Database extends ABackend implements
* counts the users in the database
*/
public function countUsers(int $limit = 0): int|false {
$this->fixDI();
$query = $this->dbConn->getQueryBuilder();
$dbConn = $this->getDbConnection();
$query = $dbConn->getQueryBuilder();
$query->select($query->func()->count('uid'))
->from($this->table);
$result = $query->executeQuery();
@ -503,7 +520,7 @@ class Database extends ABackend implements
throw new \Exception('key uid is expected to be set in $param');
}
$backends = \OC::$server->getUserManager()->getBackends();
$backends = \OCP\Server::get(IUserManager::class)->getBackends();
foreach ($backends as $backend) {
if ($backend instanceof Database) {
/** @var \OC\User\Database $backend */

@ -1,148 +0,0 @@
<?php
/**
* SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
declare(strict_types=1);
namespace Test\Authentication\Login;
use OC\Authentication\Login\EmailLoginCommand;
use OCP\IUser;
use OCP\IUserManager;
use PHPUnit\Framework\MockObject\MockObject;
class EmailLoginCommandTest extends ALoginCommandTest {
/** @var IUserManager|MockObject */
private $userManager;
protected function setUp(): void {
parent::setUp();
$this->userManager = $this->createMock(IUserManager::class);
$this->cmd = new EmailLoginCommand(
$this->userManager
);
}
public function testProcessAlreadyLoggedIn(): void {
$data = $this->getLoggedInLoginData();
$result = $this->cmd->process($data);
$this->assertTrue($result->isSuccess());
}
public function testProcessNotAnEmailLogin(): void {
$data = $this->getFailedLoginData();
$this->userManager->expects($this->never())
->method('getByEmail')
->with($this->username)
->willReturn([]);
$result = $this->cmd->process($data);
$this->assertTrue($result->isSuccess());
}
public function testProcessDuplicateEmailLogin(): void {
$data = $this->getFailedLoginData();
$data->setUsername('user@example.com');
$this->userManager->expects($this->once())
->method('getByEmail')
->with('user@example.com')
->willReturn([
$this->createMock(IUser::class),
$this->createMock(IUser::class),
]);
$result = $this->cmd->process($data);
$this->assertTrue($result->isSuccess());
}
public function testProcessUidIsEmail(): void {
$email = 'user@domain.com';
$data = $this->getFailedLoginData();
$data->setUsername($email);
$emailUser = $this->createMock(IUser::class);
$emailUser->expects($this->any())
->method('getUID')
->willReturn($email);
$this->userManager->expects($this->once())
->method('getByEmail')
->with($email)
->willReturn([
$emailUser,
]);
$this->userManager->expects($this->never())
->method('checkPassword');
$result = $this->cmd->process($data);
$this->assertTrue($result->isSuccess());
$this->assertFalse($data->getUser());
$this->assertEquals($email, $data->getUsername());
}
public function testProcessWrongPassword(): void {
$email = 'user@domain.com';
$data = $this->getFailedLoginData();
$data->setUsername($email);
$emailUser = $this->createMock(IUser::class);
$emailUser->expects($this->any())
->method('getUID')
->willReturn('user2');
$this->userManager->expects($this->once())
->method('getByEmail')
->with($email)
->willReturn([
$emailUser,
]);
$this->userManager->expects($this->once())
->method('checkPassword')
->with(
'user2',
$this->password
)
->willReturn(false);
$result = $this->cmd->process($data);
$this->assertTrue($result->isSuccess());
$this->assertFalse($data->getUser());
$this->assertEquals($email, $data->getUsername());
}
public function testProcess(): void {
$email = 'user@domain.com';
$data = $this->getFailedLoginData();
$data->setUsername($email);
$emailUser = $this->createMock(IUser::class);
$emailUser->expects($this->any())
->method('getUID')
->willReturn('user2');
$this->userManager->expects($this->once())
->method('getByEmail')
->with($email)
->willReturn([
$emailUser,
]);
$this->userManager->expects($this->once())
->method('checkPassword')
->with(
'user2',
$this->password
)
->willReturn($emailUser);
$result = $this->cmd->process($data);
$this->assertTrue($result->isSuccess());
$this->assertEquals($emailUser, $data->getUser());
$this->assertEquals('user2', $data->getUsername());
}
}
Loading…
Cancel
Save