diff --git a/changelog/unreleased/41805 b/changelog/unreleased/41805 new file mode 100644 index 000000000000..656a086050dc --- /dev/null +++ b/changelog/unreleased/41805 @@ -0,0 +1,15 @@ +Security: Temporarily lock out local accounts after repeated failed logins + +We've added throttling of password authentication for local accounts. Core did +not limit the number of failed password attempts, so an account with a weak +password could be brute-forced online without any additional app installed. +After five failed attempts within 15 minutes an account is now locked for ten +minutes, and the login form explains when it can be tried again. The lockout +always expires on its own - no administrative unlock is needed and no account is +ever disabled permanently. A successful login clears the counter immediately. +Accounts of an external backend such as LDAP/AD are not affected, they are +throttled by their identity provider. The thresholds can be adjusted with the +new `account_lockout.*` config parameters. + +https://github.com/owncloud/core/pull/41805 +https://doc.owncloud.com/server/next/admin_manual/configuration/server/config_sample_php_parameters.html diff --git a/config/config.sample.php b/config/config.sample.php index 7427d763d8ab..bf151f43b88c 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -281,6 +281,34 @@ */ 'token_auth_enforced' => false, +/** + * Temporarily lock an account after too many failed password attempts. + * + * Only accounts of the built-in user backend are locked. Accounts provided by + * LDAP/AD or an OpenID Connect provider are left to the policy of that identity + * provider. + * + * The lockout always expires on its own, no administrative action is needed, and + * a successful login resets the counter immediately. + */ +'account_lockout.enabled' => true, + +/** + * Number of failed password attempts which triggers the lockout. + */ +'account_lockout.max_attempts' => 5, + +/** + * How long an account stays locked, in seconds. + */ +'account_lockout.duration' => 600, + +/** + * Seconds of inactivity after which the counter of failed attempts is forgotten, + * so that occasional typos spread over a long time never add up to a lockout. + */ +'account_lockout.attempt_window' => 900, + /** * Enforce a strict login check with the user backend * If enabled, a strict login check for password in the user backend will be enforced, diff --git a/core/Application.php b/core/Application.php index e791b1fcdc58..d9a6665ddb51 100644 --- a/core/Application.php +++ b/core/Application.php @@ -30,6 +30,7 @@ use OC\AppFramework\Utility\SimpleContainer; use OC\AppFramework\Utility\TimeFactory; +use OC\Authentication\AccountLockout\AccountLockout; use OC\Core\Controller\AppRegistryController; use OC\Core\Controller\AvatarController; use OC\Core\Controller\CloudController; @@ -144,7 +145,8 @@ public function __construct(array $urlParams= []) { $c->query('UserManager'), $c->query('ServerContainer')->query('OC\Authentication\Token\IProvider'), $c->query('TwoFactorAuthManager'), - $c->query('SecureRandom') + $c->query('SecureRandom'), + $c->query('ServerContainer')->query(AccountLockout::class) ); }); $container->registerService('CloudController', static function (SimpleContainer $c) { diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index b49df4529b8d..5e62f5964151 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -25,6 +25,7 @@ namespace OC\Core\Controller; +use OC\Authentication\AccountLockout\AccountLockedException; use OC\Authentication\TwoFactorAuth\Manager; use OC\User\Session; use OC_App; @@ -249,20 +250,28 @@ public function showLoginForm($user, $redirect_url, $remember_login) { */ public function tryLogin($user, $password, $redirect_url, $timezone = null, $remember_login = null) { $originalUser = $user; + $lockoutMessage = null; // TODO: Add all the insane error handling - $loginResult = $this->userSession->login($user, $password); - if ($loginResult !== true && $this->config->getSystemValue('strict_login_enforced', false) !== true) { - $users = $this->userManager->getByEmail($user); - // we only allow login by email if unique - if (\count($users) === 1) { - $user = $users[0]->getUID(); - $loginResult = $this->userSession->login($user, $password); + try { + $loginResult = $this->userSession->login($user, $password); + if ($loginResult !== true && $this->config->getSystemValue('strict_login_enforced', false) !== true) { + $users = $this->userManager->getByEmail($user); + // we only allow login by email if unique + if (\count($users) === 1) { + $user = $users[0]->getUID(); + $loginResult = $this->userSession->login($user, $password); + } } + } catch (AccountLockedException $e) { + // the lockout has to be explained on the login form - letting the + // exception through would render an error page instead + $loginResult = false; + $lockoutMessage = $e->getMessage(); } if ($loginResult !== true) { - $this->session->set('loginMessages', [ - ['invalidpassword'], [] - ]); + $this->session->set('loginMessages', $lockoutMessage === null + ? [['invalidpassword'], []] + : [[], [$lockoutMessage]]); $args = []; // Read current user and append if possible - we need to return the unmodified user otherwise we will leak the login name if ($user !== null) { diff --git a/core/Controller/OcsController.php b/core/Controller/OcsController.php index ff159f3ba540..da4e6de88d21 100644 --- a/core/Controller/OcsController.php +++ b/core/Controller/OcsController.php @@ -21,6 +21,7 @@ namespace OC\Core\Controller; +use OC\Authentication\AccountLockout\AccountLockout; use OC\OCS\Result; use OCP\IDBConnection; use OCP\IRequest; @@ -44,6 +45,9 @@ class OcsController extends \OCP\AppFramework\OCSController { /** @var IUserManager */ private $userManager; + /** @var AccountLockout */ + private $accountLockout; + /** * OccController constructor. * @@ -56,12 +60,14 @@ public function __construct( IRequest $request, IDBConnection $dbConnection, IUserSession $userSession, - IUserManager $userManager + IUserManager $userManager, + AccountLockout $accountLockout ) { parent::__construct($appName, $request); $this->dbConnection = $dbConnection; $this->userSession = $userSession; $this->userManager = $userManager; + $this->accountLockout = $accountLockout; } /** @@ -92,12 +98,21 @@ public function getConfig() { */ public function checkPerson($login, $password) { if ($login && $password) { + // this endpoint verifies a password, so it has to respect the lockout + // as well - otherwise it stays an unthrottled oracle for the same + // passwords. The response is the one of a wrong password on purpose, + // it must not tell an attacker anything about the account. + if ($this->accountLockout->getRemainingLockTime($login) > 0) { + return new Result(null, 102); + } $user = $this->userManager->checkPassword($login, $password); if ($user !== false) { + $this->accountLockout->clearFailures($login, $user->getUID()); $xml = []; $xml['person']['personid'] = $user->getUID(); return new Result($xml); } else { + $this->accountLockout->recordFailure($login); return new Result(null, 102); } } else { diff --git a/core/Controller/TokenController.php b/core/Controller/TokenController.php index da97d738cc20..abd0cc538c3e 100644 --- a/core/Controller/TokenController.php +++ b/core/Controller/TokenController.php @@ -22,6 +22,7 @@ namespace OC\Core\Controller; use OC\AppFramework\Http; +use OC\Authentication\AccountLockout\AccountLockout; use OC\Authentication\Token\DefaultTokenProvider; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; @@ -46,19 +47,24 @@ class TokenController extends Controller { /** @var ISecureRandom */ private $secureRandom; + /** @var AccountLockout */ + private $accountLockout; + /** * @param string $appName * @param IRequest $request * @param Manager $userManager * @param DefaultTokenProvider $tokenProvider * @param ISecureRandom $secureRandom + * @param AccountLockout $accountLockout */ - public function __construct($appName, IRequest $request, UserManager $userManager, IProvider $tokenProvider, TwoFactorAuthManager $twoFactorAuthManager, ISecureRandom $secureRandom) { + public function __construct($appName, IRequest $request, UserManager $userManager, IProvider $tokenProvider, TwoFactorAuthManager $twoFactorAuthManager, ISecureRandom $secureRandom, AccountLockout $accountLockout) { parent::__construct($appName, $request); $this->userManager = $userManager; $this->tokenProvider = $tokenProvider; $this->secureRandom = $secureRandom; $this->twoFactorAuthManager = $twoFactorAuthManager; + $this->accountLockout = $accountLockout; } /** @@ -79,12 +85,23 @@ public function generateToken($user, $password, $name = 'unknown client') { return $response; } $loginName = $user; + + // this endpoint verifies a password, so it has to respect the lockout as + // well - otherwise it stays an unthrottled oracle for the same passwords + if ($this->accountLockout->getRemainingLockTime($loginName) > 0) { + $response = new JSONResponse(); + $response->setStatus(Http::STATUS_UNAUTHORIZED); + return $response; + } + $user = $this->userManager->checkPassword($loginName, $password); if ($user === false) { + $this->accountLockout->recordFailure($loginName); $response = new JSONResponse(); $response->setStatus(Http::STATUS_UNAUTHORIZED); return $response; } + $this->accountLockout->clearFailures($loginName, $user->getUID()); if ($this->twoFactorAuthManager->isTwoFactorAuthenticated($user)) { $resp = new JSONResponse(); diff --git a/core/Migrations/Version20260904090000.php b/core/Migrations/Version20260904090000.php new file mode 100644 index 000000000000..9d0b49fa0631 --- /dev/null +++ b/core/Migrations/Version20260904090000.php @@ -0,0 +1,68 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ +namespace OC\Migrations; + +use Doctrine\DBAL\Schema\Schema; +use OCP\Migration\ISchemaMigration; + +/** + * Table holding the failed login attempt counters which drive the temporary + * lockout of local accounts. + */ +class Version20260904090000 implements ISchemaMigration { + public function changeSchema(Schema $schema, array $options) { + $prefix = $options['tablePrefix']; + if (!$schema->hasTable("{$prefix}account_lockouts")) { + $table = $schema->createTable("{$prefix}account_lockouts"); + $table->addColumn('id', 'bigint', [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 20, + ]); + // the lockout key: the resolved account uid, or - for a login name + // which does not resolve to an account - the normalised login name. + // Longer than users.uid on purpose, login names are attacker supplied. + $table->addColumn('uid', 'string', [ + 'notnull' => true, + 'length' => 128, + ]); + $table->addColumn('fail_count', 'integer', [ + 'notnull' => true, + 'length' => 4, + 'default' => 0, + ]); + // unix timestamp; null or in the past means not locked + $table->addColumn('locked_until', 'bigint', [ + 'notnull' => false, + 'length' => 20, + ]); + // unix timestamp of the most recent failure, drives the counter decay + $table->addColumn('last_fail_at', 'bigint', [ + 'notnull' => true, + 'length' => 20, + 'default' => 0, + ]); + $table->setPrimaryKey(['id']); + $table->addUniqueIndex(['uid'], 'acc_lockouts_uid_idx'); + $table->addIndex(['last_fail_at'], 'acc_lockouts_fail_at_idx'); + } + } +} diff --git a/core/Migrations/Version20260904090001.php b/core/Migrations/Version20260904090001.php new file mode 100644 index 000000000000..c075abd12edf --- /dev/null +++ b/core/Migrations/Version20260904090001.php @@ -0,0 +1,39 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ +namespace OC\Migrations; + +use OCP\Migration\IOutput; +use OCP\Migration\ISimpleMigration; + +/** + * Fresh installations get the job from Setup::installBackgroundJobs(), existing + * installations need it added here. + */ +class Version20260904090001 implements ISimpleMigration { + /** + * @param IOutput $out + */ + public function run(IOutput $out) { + // spelled exactly like in Setup::installBackgroundJobs(), the job list + // compares the class name verbatim when it deduplicates + \OC::$server->getJobList()->add('\OC\Authentication\AccountLockout\ExpireLockoutsJob'); + } +} diff --git a/lib/private/Authentication/AccountLockout/AccountLockedException.php b/lib/private/Authentication/AccountLockout/AccountLockedException.php new file mode 100644 index 000000000000..9c000700209d --- /dev/null +++ b/lib/private/Authentication/AccountLockout/AccountLockedException.php @@ -0,0 +1,35 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Authentication\AccountLockout; + +use OC\User\LoginException; + +/** + * Thrown when a login is refused because the account is temporarily locked + * after too many failed password attempts. + * + * It is a LoginException so that existing callers which already render a + * LoginException - most importantly the DAV backend, which turns it into a + * 401 - keep working unchanged. + */ +class AccountLockedException extends LoginException { +} diff --git a/lib/private/Authentication/AccountLockout/AccountLockout.php b/lib/private/Authentication/AccountLockout/AccountLockout.php new file mode 100644 index 000000000000..e28eda0efee4 --- /dev/null +++ b/lib/private/Authentication/AccountLockout/AccountLockout.php @@ -0,0 +1,265 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Authentication\AccountLockout; + +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IConfig; +use OCP\ILogger; +use OCP\IUserManager; + +/** + * Temporary lockout of local accounts after too many failed password attempts. + * + * The lockout always expires on its own - there is no administrative unlock and + * no permanent disabling of an account. + * + * Accounts served by an external backend (LDAP/AD, OIDC, ...) are not tracked: + * the identity provider enforces its own policy and counting the same attempt + * twice would punish twice. + * + * Login names which do not resolve to an account *are* tracked, and produce the + * very same response as a locked existing account. Without that, the lockout + * response would tell an attacker which user names exist. + */ +class AccountLockout { + public const CONFIG_ENABLED = 'account_lockout.enabled'; + public const CONFIG_MAX_ATTEMPTS = 'account_lockout.max_attempts'; + public const CONFIG_DURATION = 'account_lockout.duration'; + public const CONFIG_ATTEMPT_WINDOW = 'account_lockout.attempt_window'; + + public const DEFAULT_MAX_ATTEMPTS = 5; + public const DEFAULT_DURATION = 600; + public const DEFAULT_ATTEMPT_WINDOW = 900; + + /** + * The name reported by the built in user backend, lib/private/User/Database.php. + * Kept in exactly one place on purpose. + */ + private const LOCAL_BACKEND_NAME = 'Database'; + + /** Matches the length of `account_lockouts`.`uid` */ + private const MAX_KEY_LENGTH = 128; + + /** @var LockoutMapper */ + private $mapper; + /** @var IConfig */ + private $config; + /** @var IUserManager */ + private $userManager; + /** @var ITimeFactory */ + private $timeFactory; + /** @var ILogger */ + private $logger; + + /** + * One HTTP request may attempt the same credentials twice - a login by + * email is retried with the resolved user id, see Session::logClientIn(). + * That is one failure, not two. + * + * @var bool + */ + private $failureRecorded = false; + + public function __construct( + LockoutMapper $mapper, + IConfig $config, + IUserManager $userManager, + ITimeFactory $timeFactory, + ILogger $logger + ) { + $this->mapper = $mapper; + $this->config = $config; + $this->userManager = $userManager; + $this->timeFactory = $timeFactory; + $this->logger = $logger; + } + + /** + * Seconds left before $login may be tried again, 0 if it is not locked. + * + * Deliberately does not look the account up unless a lockout is in effect, + * so the common case adds a single indexed read and no timing difference + * between existing and unknown login names. + * + * @param string $login the login name as submitted + * @return int + */ + public function getRemainingLockTime($login): int { + if (!$this->isEnabled()) { + return 0; + } + + $row = $this->mapper->find($this->keyFor($login)); + if ($row === null || $row['locked_until'] === null) { + return 0; + } + + $remaining = $row['locked_until'] - $this->timeFactory->getTime(); + if ($remaining <= 0) { + return 0; + } + + // A login name which was unknown while the failures were counted may + // meanwhile have appeared in an external backend. Do not hold that + // account hostage - drop the counter instead. + if (!$this->isTracked($login)) { + $this->mapper->delete($this->keyFor($login)); + return 0; + } + + return $remaining; + } + + /** + * Count one failed password attempt and lock the account once the + * configured threshold is reached. + * + * @param string $login the login name as submitted + */ + public function recordFailure($login) { + if (!$this->isEnabled() || $this->failureRecorded) { + return; + } + if (!$this->isTracked($login)) { + return; + } + $this->failureRecorded = true; + + $key = $this->keyFor($login); + $now = $this->timeFactory->getTime(); + + // Each statement stands on its own, so parallel requests - including + // requests on other application servers - cannot lose a count. + $counted = $this->mapper->restartIfStale($key, $now, $now - $this->getAttemptWindow()); + if ($counted === 0) { + $counted = $this->mapper->increment($key, $now); + } + if ($counted === 0 && !$this->mapper->insertFirstFailure($key, $now)) { + // lost the insert race, the winner's row is there to be counted + $this->mapper->increment($key, $now); + } + + $maxAttempts = $this->getMaxAttempts(); + $row = $this->mapper->find($key); + if ($row === null || $row['fail_count'] < $maxAttempts || $row['locked_until'] !== null) { + return; + } + + if ($this->mapper->lock($key, $now + $this->getDuration(), $maxAttempts) > 0) { + $this->logger->warning( + \sprintf( + 'Temporarily locked out after %d failed login attempts: %s', + $row['fail_count'], + $key + ), + ['app' => 'core'] + ); + } + } + + /** + * Forget all failures of an account - called on every successful login. + * + * @param string $login the login name as submitted + * @param string|null $uid the user id it resolved to, if known + */ + public function clearFailures($login, $uid = null) { + if (!$this->isEnabled()) { + return; + } + + $keys = [$this->keyFor($login)]; + if ($uid !== null) { + $keys[] = $this->keyFor($uid); + } + foreach (\array_unique($keys) as $key) { + $this->mapper->delete($key); + } + } + + /** + * Housekeeping: forget counters which can no longer lock anybody out. + * + * @return int number of rows removed + */ + public function expireStale() { + $now = $this->timeFactory->getTime(); + return $this->mapper->deleteStale($now - $this->getAttemptWindow(), $now); + } + + /** + * Whether failures for this login name are counted at all. Unknown login + * names are, see the class comment; accounts of an external backend are not. + * + * @param string $login + * @return bool + */ + private function isTracked($login) { + $user = $this->userManager->get($login); + if ($user === null) { + return true; + } + return $user->getBackendClassName() === self::LOCAL_BACKEND_NAME; + } + + /** + * The row key for a login name. Core resolves accounts case insensitively, + * so the counter has to as well - otherwise `admin`, `Admin` and `ADMIN` + * would each get their own budget of attempts. Normalised in PHP rather + * than relying on the collation of the database in use. + * + * @param string $login + * @return string + */ + private function keyFor($login) { + $key = \mb_strtolower(\trim((string)$login), 'UTF-8'); + return \mb_substr($key, 0, self::MAX_KEY_LENGTH, 'UTF-8'); + } + + /** + * @return bool + */ + private function isEnabled() { + return (bool)$this->config->getSystemValue(self::CONFIG_ENABLED, true); + } + + /** + * @return int + */ + private function getMaxAttempts() { + return \max(1, (int)$this->config->getSystemValue(self::CONFIG_MAX_ATTEMPTS, self::DEFAULT_MAX_ATTEMPTS)); + } + + /** + * @return int + */ + private function getDuration() { + return \max(1, (int)$this->config->getSystemValue(self::CONFIG_DURATION, self::DEFAULT_DURATION)); + } + + /** + * @return int + */ + private function getAttemptWindow() { + return \max(1, (int)$this->config->getSystemValue(self::CONFIG_ATTEMPT_WINDOW, self::DEFAULT_ATTEMPT_WINDOW)); + } +} diff --git a/lib/private/Authentication/AccountLockout/ExpireLockoutsJob.php b/lib/private/Authentication/AccountLockout/ExpireLockoutsJob.php new file mode 100644 index 000000000000..d8ecdfeea8e5 --- /dev/null +++ b/lib/private/Authentication/AccountLockout/ExpireLockoutsJob.php @@ -0,0 +1,39 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Authentication\AccountLockout; + +use OC; +use OC\BackgroundJob\Job; + +/** + * Removes failed login counters which are too old to lock anybody out. + * + * Purely housekeeping - a lockout expires on its own whether this job runs or + * not, see AccountLockout. + */ +class ExpireLockoutsJob extends Job { + protected function run($argument) { + /* @var $accountLockout AccountLockout */ + $accountLockout = OC::$server->query(AccountLockout::class); + $accountLockout->expireStale(); + } +} diff --git a/lib/private/Authentication/AccountLockout/LockoutMapper.php b/lib/private/Authentication/AccountLockout/LockoutMapper.php new file mode 100644 index 000000000000..5ceb236709dc --- /dev/null +++ b/lib/private/Authentication/AccountLockout/LockoutMapper.php @@ -0,0 +1,210 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OC\Authentication\AccountLockout; + +use Doctrine\DBAL\Exception\TableNotFoundException; +use Doctrine\DBAL\Exception\UniqueConstraintViolationException; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; + +/** + * Database access for the failed login counters in `account_lockouts`. + * + * There is at most one row per lockout key. All write operations are single + * statements so that requests served by different application servers cannot + * race the threshold - see AccountLockout::recordFailure() for how they are + * combined. + * + * A pending upgrade is served with the login route enabled but the previous + * schema still in place, so every statement tolerates the table being absent + * and simply does not lock anybody out until the migration has run. + */ +class LockoutMapper { + public const TABLE = 'account_lockouts'; + + /** @var IDBConnection */ + private $db; + + public function __construct(IDBConnection $db) { + $this->db = $db; + } + + /** + * @param string $uid the normalised lockout key + * @return array|null ['fail_count' => int, 'locked_until' => int|null, 'last_fail_at' => int] + * or null if the key has no failures recorded + */ + public function find($uid) { + $qb = $this->db->getQueryBuilder(); + $qb->select('fail_count', 'locked_until', 'last_fail_at') + ->from(self::TABLE) + ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid))); + + try { + $result = $qb->execute(); + } catch (TableNotFoundException $e) { + return null; + } + $row = $result->fetchAssociative(); + $result->free(); + + if ($row === false) { + return null; + } + return [ + 'fail_count' => (int)$row['fail_count'], + 'locked_until' => $row['locked_until'] === null ? null : (int)$row['locked_until'], + 'last_fail_at' => (int)$row['last_fail_at'], + ]; + } + + /** + * Restart the counter at 1 if the existing row is stale - either the last + * failure is older than the attempt window, or a previous lockout has + * expired. Clears the expired lockout in the same statement. + * + * @param string $uid the normalised lockout key + * @param int $now unix timestamp + * @param int $windowStart failures older than this no longer count + * @return int number of rows updated, 0 if the row is absent or still current + */ + public function restartIfStale($uid, $now, $windowStart) { + $qb = $this->db->getQueryBuilder(); + $qb->update(self::TABLE) + ->set('fail_count', $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT)) + ->set('locked_until', 'null') + ->set('last_fail_at', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid))) + ->andWhere($qb->expr()->orX( + $qb->expr()->lt('last_fail_at', $qb->createNamedParameter($windowStart, IQueryBuilder::PARAM_INT)), + $qb->expr()->andX( + $qb->expr()->isNotNull('locked_until'), + $qb->expr()->lte('locked_until', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT)) + ) + )); + return $this->executeStatement($qb); + } + + /** + * Atomically add one to the counter of an existing row. + * + * @param string $uid the normalised lockout key + * @param int $now unix timestamp + * @return int number of rows updated, 0 if the row is absent + */ + public function increment($uid, $now) { + $qb = $this->db->getQueryBuilder(); + $qb->update(self::TABLE) + ->set('fail_count', $qb->createFunction('`fail_count` + 1')) + ->set('last_fail_at', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid))); + return $this->executeStatement($qb); + } + + /** + * Record the first failure for a key. + * + * @param string $uid the normalised lockout key + * @param int $now unix timestamp + * @return bool false if a concurrent request inserted the row first + */ + public function insertFirstFailure($uid, $now) { + $qb = $this->db->getQueryBuilder(); + $qb->insert(self::TABLE) + ->values([ + 'uid' => $qb->createNamedParameter($uid), + 'fail_count' => $qb->createNamedParameter(1, IQueryBuilder::PARAM_INT), + 'locked_until' => $qb->createNamedParameter(null), + 'last_fail_at' => $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT), + ]); + try { + $qb->execute(); + } catch (UniqueConstraintViolationException $e) { + return false; + } catch (TableNotFoundException $e) { + return true; + } + return true; + } + + /** + * Start a lockout, but only for a row which reached the threshold and is + * not locked already - so concurrent failures cannot extend a running + * lockout. + * + * @param string $uid the normalised lockout key + * @param int $lockedUntil unix timestamp the lockout expires at + * @param int $minFailCount the configured threshold + * @return int number of rows updated + */ + public function lock($uid, $lockedUntil, $minFailCount) { + $qb = $this->db->getQueryBuilder(); + $qb->update(self::TABLE) + ->set('locked_until', $qb->createNamedParameter($lockedUntil, IQueryBuilder::PARAM_INT)) + ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid))) + ->andWhere($qb->expr()->isNull('locked_until')) + ->andWhere($qb->expr()->gte('fail_count', $qb->createNamedParameter($minFailCount, IQueryBuilder::PARAM_INT))); + return $this->executeStatement($qb); + } + + /** + * Forget all failures of a key. + * + * @param string $uid the normalised lockout key + */ + public function delete($uid) { + $qb = $this->db->getQueryBuilder(); + $qb->delete(self::TABLE) + ->where($qb->expr()->eq('uid', $qb->createNamedParameter($uid))); + $this->executeStatement($qb); + } + + /** + * Housekeeping: drop rows which can no longer affect a login decision. + * + * @param int $olderThan rows whose last failure predates this are removed + * @param int $now unix timestamp; rows with a running lockout are kept + * @return int number of rows removed + */ + public function deleteStale($olderThan, $now) { + $qb = $this->db->getQueryBuilder(); + $qb->delete(self::TABLE) + ->where($qb->expr()->lt('last_fail_at', $qb->createNamedParameter($olderThan, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->orX( + $qb->expr()->isNull('locked_until'), + $qb->expr()->lte('locked_until', $qb->createNamedParameter($now, IQueryBuilder::PARAM_INT)) + )); + return $this->executeStatement($qb); + } + + /** + * @param IQueryBuilder $qb + * @return int number of rows affected, 0 if the table is not there yet + */ + private function executeStatement(IQueryBuilder $qb) { + try { + return $qb->execute(); + } catch (TableNotFoundException $e) { + return 0; + } + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index 3b4375524b4f..fb9a8ad619a6 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -44,6 +44,8 @@ use OC\AppFramework\Http\Request; use OC\AppFramework\Db\Db; use OC\AppFramework\Utility\TimeFactory; +use OC\Authentication\AccountLockout\AccountLockout; +use OC\Authentication\AccountLockout\LockoutMapper; use OC\Authentication\AccountModule\Manager as AccountModuleManager; use OC\Authentication\LoginPolicies\LoginPolicyManager; use OC\Authentication\LoginPolicies\GroupLoginPolicy; @@ -321,6 +323,18 @@ public function __construct($webRoot, \OC\Config $config) { return new TimeFactory(); }); $this->registerAlias('OCP\AppFramework\Utility\ITimeFactory', 'TimeFactory'); + $this->registerService(LockoutMapper::class, function (Server $c) { + return new LockoutMapper($c->getDatabaseConnection()); + }); + $this->registerService(AccountLockout::class, function (Server $c) { + return new AccountLockout( + $c->query(LockoutMapper::class), + $c->getConfig(), + $c->getUserManager(), + new TimeFactory(), + $c->getLogger() + ); + }); $this->registerService('UserSession', function (Server $c) { $manager = $c->getUserManager(); $session = new Memory(); @@ -347,7 +361,8 @@ public function __construct($webRoot, \OC\Config $config) { $c->getLogger(), $this, $userSyncService, - $c->getEventDispatcher() + $c->getEventDispatcher(), + $c->query(AccountLockout::class) ); $userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password) { \OC_Hook::emit('OC_User', 'pre_createUser', ['run' => true, 'uid' => $uid, 'password' => $password]); @@ -363,6 +378,8 @@ public function __construct($webRoot, \OC\Config $config) { $userSession->listen('\OC\User', 'postDelete', function ($user) { /** @var $user \OC\User\User */ \OC_Hook::emit('OC_User', 'post_deleteUser', ['uid' => $user->getUID()]); + // a recreated account must not inherit the failed attempts of its namesake + $this->query(AccountLockout::class)->clearFailures($user->getUID()); $this->emittingCall(function () { return true; }, ['before' => [], 'after' => ['uid' => $user->getUID()]], 'user', 'delete'); diff --git a/lib/private/Setup.php b/lib/private/Setup.php index 0d79276ab7aa..2af2c032e275 100644 --- a/lib/private/Setup.php +++ b/lib/private/Setup.php @@ -438,6 +438,7 @@ public function install(array $options): array { public static function installBackgroundJobs(): void { \OC::$server->getJobList()->add('\OC\Authentication\Token\DefaultTokenCleanupJob'); + \OC::$server->getJobList()->add('\OC\Authentication\AccountLockout\ExpireLockoutsJob'); } /** diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 951d85b9c01c..e077cd012fde 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -35,6 +35,8 @@ use Doctrine\DBAL\Exception\UniqueConstraintViolationException; use Exception; use OC; +use OC\Authentication\AccountLockout\AccountLockedException; +use OC\Authentication\AccountLockout\AccountLockout; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Exceptions\PasswordLoginForbiddenException; @@ -116,6 +118,9 @@ class Session implements IUserSession, Emitter { /** @var EventDispatcher */ protected $eventDispatcher; + /** @var AccountLockout */ + private $accountLockout; + /** * Session constructor. * @@ -128,6 +133,7 @@ class Session implements IUserSession, Emitter { * @param IServiceLoader $serviceLoader * @param SyncService $userSyncService * @param EventDispatcher $eventDispatcher + * @param AccountLockout $accountLockout */ public function __construct( IUserManager $manager, @@ -138,7 +144,8 @@ public function __construct( ILogger $logger, IServiceLoader $serviceLoader, SyncService $userSyncService, - EventDispatcher $eventDispatcher + EventDispatcher $eventDispatcher, + AccountLockout $accountLockout ) { $this->manager = $manager; $this->session = $session; @@ -149,6 +156,7 @@ public function __construct( $this->serviceLoader = $serviceLoader; $this->userSyncService = $userSyncService; $this->eventDispatcher = $eventDispatcher; + $this->accountLockout = $accountLockout; } /** @@ -516,6 +524,12 @@ public function tryBasicAuthLogin(IRequest $request) { } } catch (PasswordLoginForbiddenException $ex) { // Nothing to do + } catch (AccountLockedException $ex) { + // The callers of \OC::handleLogin() do not all handle a + // LoginException, so report the lockout the same way a wrong + // password is reported here. The endpoints which can carry the + // explanation - the login form and the DAV backend - do not use + // this method. } } return false; @@ -535,14 +549,46 @@ public function tryBasicAuthLogin(IRequest $request) { * compatibility. */ private function loginWithPassword($login, $password) { + // deliberately ahead of the credential check: a locked out account must + // not pay the cost of hashing the password, nor reach an external backend + $this->throwIfLockedOut($login); + $user = $this->manager->checkPassword($login, $password); if ($user === false) { + $this->accountLockout->recordFailure($login); $this->emitFailedLogin($login); return false; } + $this->accountLockout->clearFailures($login, $user->getUID()); return $this->loginInOwnCloud('password', $user, $password); } + /** + * @param string $login + * @throws AccountLockedException if too many passwords have been tried + */ + private function throwIfLockedOut($login) { + $remaining = $this->accountLockout->getRemainingLockTime($login); + if ($remaining <= 0) { + return; + } + + // injecting l10n does not work - there is a circular dependency between session and \OCP\L10N\IFactory + $l = \OC::$server->getL10N('lib'); + $minutes = (int)\ceil($remaining / 60); + $retryIn = $minutes > 1 + ? $l->n('%n minute', '%n minutes', $minutes) + : $l->n('%n second', '%n seconds', $remaining); + // the wording must not depend on whether the account exists - that would + // turn the lockout into a user enumeration oracle + $message = $l->t( + 'Too many failed login attempts. This account is temporarily locked. Please try again in %s.', + [$retryIn] + ); + $this->logger->info("login $login refused, account is temporarily locked", ['app' => __METHOD__]); + throw new AccountLockedException($message); + } + /** * Log an user in with a given token (id) * diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index 1a83f6c97f1a..2aaca6c73919 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -22,6 +22,7 @@ namespace Tests\Core\Controller; +use OC\Authentication\AccountLockout\AccountLockedException; use OC\Authentication\TwoFactorAuth\Manager; use OC\Core\Controller\LoginController; use OC\User\Session; @@ -586,6 +587,30 @@ public function testLoginWithInvalidCredentials($strictLoginCheck, $expectedGetB $this->assertEquals($expected, $this->loginController->tryLogin($user, $password, '/foo')); } + public function testLoginWithLockedOutAccount() { + $user = 'unknown'; + $loginPageUrl = 'some url'; + $message = 'Too many failed login attempts. This account is temporarily locked. Please try again in 10 minutes.'; + + $this->userSession->expects($this->once()) + ->method('login') + ->will($this->throwException(new AccountLockedException($message))); + // the lockout must be explained instead of being reported as a wrong password + $this->session->expects($this->once()) + ->method('set') + ->with('loginMessages', [[], [$message]]); + // no retry by email address - the account is locked, not misspelled + $this->userManager->expects($this->never()) + ->method('getByEmail'); + $this->urlGenerator->expects($this->once()) + ->method('linkToRoute') + ->with('core.login.showLoginForm', ['user' => $user]) + ->will($this->returnValue($loginPageUrl)); + + $expected = new RedirectResponse($loginPageUrl); + $this->assertEquals($expected, $this->loginController->tryLogin($user, 'secret', null)); + } + public function testLoginWithValidCredentials() { /** @var IUser | \PHPUnit\Framework\MockObject\MockObject $user */ $user = $this->createMock(IUser::class); diff --git a/tests/Core/Controller/OcsControllerTest.php b/tests/Core/Controller/OcsControllerTest.php index 9e774c0c740f..74dc5d66d68f 100644 --- a/tests/Core/Controller/OcsControllerTest.php +++ b/tests/Core/Controller/OcsControllerTest.php @@ -24,6 +24,7 @@ use Doctrine\DBAL\Result; use Doctrine\DBAL\Statement; use OC\AppFramework\Http\Request; +use OC\Authentication\AccountLockout\AccountLockout; use OC\Core\Controller\OcsController; use OCP\IDBConnection; use OCP\IRequest; @@ -51,6 +52,9 @@ class OcsControllerTest extends TestCase { /** @var IUserManager | MockObject */ private $userManager; + /** @var AccountLockout | MockObject */ + private $accountLockout; + /** @var OcsController | MockObject */ private $controller; @@ -60,12 +64,14 @@ protected function setUp(): void { $this->dbConn = $this->createMock(IDBConnection::class); $this->userSession = $this->createMock(IUserSession::class); $this->userManager = $this->createMock(IUserManager::class); + $this->accountLockout = $this->createMock(AccountLockout::class); $this->controller = new OcsController( 'core', $this->request, $this->dbConn, $this->userSession, - $this->userManager + $this->userManager, + $this->accountLockout ); } @@ -104,6 +110,48 @@ public function testCheckPerson($login, $password, $checkPasswordSuccess, $expec $this->assertEquals($expectedCode, $result->getStatusCode()); } + public function testCheckPersonWrongPasswordIsCounted() { + $this->userManager->expects($this->once()) + ->method('checkPassword') + ->willReturn(false); + $this->accountLockout->expects($this->once()) + ->method('recordFailure') + ->with('user'); + + $result = $this->controller->checkPerson('user', 'password'); + $this->assertEquals(102, $result->getStatusCode()); + } + + public function testCheckPersonLockedOutIsRefusedBeforeTheCredentialCheck() { + $this->accountLockout->expects($this->once()) + ->method('getRemainingLockTime') + ->with('user') + ->willReturn(300); + $this->userManager->expects($this->never()) + ->method('checkPassword'); + $this->accountLockout->expects($this->never()) + ->method('recordFailure'); + + // the very same result as a wrong password, so that the lockout does not + // become a user enumeration oracle + $result = $this->controller->checkPerson('user', 'password'); + $this->assertEquals(102, $result->getStatusCode()); + } + + public function testCheckPersonSuccessResetsTheCounter() { + $this->userManager->expects($this->once()) + ->method('checkPassword') + ->willReturn($this->getUserMock()); + $this->accountLockout->expects($this->once()) + ->method('clearFailures') + ->with('user', 'foo'); + $this->accountLockout->expects($this->never()) + ->method('recordFailure'); + + $result = $this->controller->checkPerson('user', 'password'); + $this->assertEquals(100, $result->getStatusCode()); + } + public function getAttributeDataProvider() { return [ ['app', null], diff --git a/tests/Core/Controller/TokenControllerTest.php b/tests/Core/Controller/TokenControllerTest.php index b36d96358d2e..0ea601b441b7 100644 --- a/tests/Core/Controller/TokenControllerTest.php +++ b/tests/Core/Controller/TokenControllerTest.php @@ -23,6 +23,7 @@ namespace Tests\Core\Controller; use OC\AppFramework\Http; +use OC\Authentication\AccountLockout\AccountLockout; use OC\Authentication\Token\IToken; use OC\Core\Controller\TokenController; use OCP\AppFramework\Http\JSONResponse; @@ -36,6 +37,8 @@ class TokenControllerTest extends TestCase { private $tokenProvider; private $twoFactorAuthManager; private $secureRandom; + /** @var AccountLockout | \PHPUnit\Framework\MockObject\MockObject */ + private $accountLockout; protected function setUp(): void { parent::setUp(); @@ -49,8 +52,9 @@ protected function setUp(): void { ->disableOriginalConstructor() ->getMock(); $this->secureRandom = $this->createMock('\OCP\Security\ISecureRandom'); + $this->accountLockout = $this->createMock(AccountLockout::class); - $this->tokenController = new TokenController('core', $this->request, $this->userManager, $this->tokenProvider, $this->twoFactorAuthManager, $this->secureRandom); + $this->tokenController = new TokenController('core', $this->request, $this->userManager, $this->tokenProvider, $this->twoFactorAuthManager, $this->secureRandom, $this->accountLockout); } public function testWithoutCredentials() { @@ -75,15 +79,46 @@ public function testWithInvalidCredentials() { $this->assertEquals($expected, $actual); } + public function testWithInvalidCredentialsTheFailureIsCounted() { + $this->userManager->expects($this->once()) + ->method('checkPassword') + ->with('john', 'passme') + ->will($this->returnValue(false)); + $this->accountLockout->expects($this->once()) + ->method('recordFailure') + ->with('john'); + + $this->tokenController->generateToken('john', 'passme'); + } + + public function testLockedOutIsRefusedBeforeTheCredentialCheck() { + $this->accountLockout->expects($this->once()) + ->method('getRemainingLockTime') + ->with('john') + ->willReturn(300); + $this->userManager->expects($this->never()) + ->method('checkPassword'); + $this->accountLockout->expects($this->never()) + ->method('recordFailure'); + $expected = new JSONResponse(); + $expected->setStatus(Http::STATUS_UNAUTHORIZED); + + $actual = $this->tokenController->generateToken('john', 'passme'); + + $this->assertEquals($expected, $actual); + } + public function testWithValidCredentials() { $user = $this->createMock('\OCP\IUser'); $this->userManager->expects($this->once()) ->method('checkPassword') ->with('john', '123456') ->will($this->returnValue($user)); - $user->expects($this->once()) - ->method('getUID') + $user->method('getUID') ->will($this->returnValue('john')); + $this->accountLockout->expects($this->once()) + ->method('clearFailures') + ->with('john', 'john'); $this->twoFactorAuthManager->expects($this->once()) ->method('isTwoFactorAuthenticated') ->with($user) diff --git a/tests/lib/Authentication/AccountLockout/AccountLockoutTest.php b/tests/lib/Authentication/AccountLockout/AccountLockoutTest.php new file mode 100644 index 000000000000..95aaf4937713 --- /dev/null +++ b/tests/lib/Authentication/AccountLockout/AccountLockoutTest.php @@ -0,0 +1,287 @@ + + * + * @copyright Copyright (c) 2026, ownCloud GmbH + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace Test\Authentication\AccountLockout; + +use OC\Authentication\AccountLockout\AccountLockout; +use OC\Authentication\AccountLockout\LockoutMapper; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IConfig; +use OCP\ILogger; +use OCP\IUser; +use OCP\IUserManager; +use Test\TestCase; + +/** + * Runs against the real database, so that the statements which have to be + * atomic are the ones being tested. + * + * @group DB + * @package Test\Authentication\AccountLockout + */ +class AccountLockoutTest extends TestCase { + /** @var LockoutMapper */ + private $mapper; + + /** @var IConfig | \PHPUnit\Framework\MockObject\MockObject */ + private $config; + + /** @var IUserManager | \PHPUnit\Framework\MockObject\MockObject */ + private $userManager; + + /** @var int */ + private $now = 1500000000; + + /** @var array system values overriding the defaults */ + private $systemValues = []; + + /** @var array login name (lower case) => backend name */ + private $accounts = [ + 'alice' => 'Database', + 'ldapuser' => 'LDAP', + ]; + + /** @var string[] keys to clean up */ + private $usedKeys = [ + 'alice', + 'admin', + 'ldapuser', + 'ldapuser2', + 'nosuchuser', + 'alice@example.com', + ]; + + protected function setUp(): void { + parent::setUp(); + + $this->mapper = new LockoutMapper(\OC::$server->getDatabaseConnection()); + $this->config = $this->createMock(IConfig::class); + $this->config->method('getSystemValue') + ->willReturnCallback(function ($key, $default = null) { + return \array_key_exists($key, $this->systemValues) ? $this->systemValues[$key] : $default; + }); + $this->userManager = $this->createMock(IUserManager::class); + $this->userManager->method('get') + ->willReturnCallback(function ($login) { + // core resolves accounts case insensitively + $key = \strtolower($login); + if (!isset($this->accounts[$key])) { + return null; + } + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn($key); + $user->method('getBackendClassName')->willReturn($this->accounts[$key]); + return $user; + }); + + $this->clearRows(); + } + + protected function tearDown(): void { + $this->clearRows(); + parent::tearDown(); + } + + private function clearRows() { + foreach ($this->usedKeys as $key) { + $this->mapper->delete($key); + } + } + + /** + * A fresh service instance stands for a fresh request - the per request + * deduplication of failures must not leak from one test to the next. + * + * @return AccountLockout + */ + private function newRequest() { + $timeFactory = $this->createMock(ITimeFactory::class); + $timeFactory->method('getTime')->willReturnCallback(function () { + return $this->now; + }); + + return new AccountLockout( + $this->mapper, + $this->config, + $this->userManager, + $timeFactory, + $this->createMock(ILogger::class) + ); + } + + /** + * @param string $login + * @param int $times + */ + private function badPassword($login, $times = 1) { + for ($i = 0; $i < $times; $i++) { + $this->newRequest()->recordFailure($login); + } + } + + public function testFourFailuresThenSuccessClearsTheCounter() { + $this->badPassword('alice', 4); + $this->assertSame(0, $this->newRequest()->getRemainingLockTime('alice')); + $this->assertSame(4, $this->mapper->find('alice')['fail_count']); + + $this->newRequest()->clearFailures('alice', 'alice'); + + $this->assertNull($this->mapper->find('alice')); + $this->assertSame(0, $this->newRequest()->getRemainingLockTime('alice')); + } + + public function testFifthFailureLocksTheAccount() { + $this->badPassword('alice', 5); + + // the correct password is refused as well, that is the point + $this->assertSame( + AccountLockout::DEFAULT_DURATION, + $this->newRequest()->getRemainingLockTime('alice') + ); + } + + public function testLockoutExpiresWithoutAdminAction() { + $this->badPassword('alice', 5); + $this->assertGreaterThan(0, $this->newRequest()->getRemainingLockTime('alice')); + + $this->now += AccountLockout::DEFAULT_DURATION; + + $this->assertSame(0, $this->newRequest()->getRemainingLockTime('alice')); + } + + public function testCounterDecaysAfterTheAttemptWindow() { + $this->badPassword('alice', 4); + + $this->now += AccountLockout::DEFAULT_ATTEMPT_WINDOW + 1; + + // the 5th failure overall, but the first one within the window + $this->badPassword('alice'); + + $this->assertSame(1, $this->mapper->find('alice')['fail_count']); + $this->assertSame(0, $this->newRequest()->getRemainingLockTime('alice')); + } + + public function testDisabledDoesNotEvenWrite() { + $this->systemValues[AccountLockout::CONFIG_ENABLED] = false; + + $this->badPassword('alice', 10); + + $this->assertNull($this->mapper->find('alice')); + $this->assertSame(0, $this->newRequest()->getRemainingLockTime('alice')); + } + + public function testConfiguredThresholdAndDurationAreUsed() { + $this->systemValues[AccountLockout::CONFIG_MAX_ATTEMPTS] = 2; + $this->systemValues[AccountLockout::CONFIG_DURATION] = 60; + + $this->badPassword('alice'); + $this->assertSame(0, $this->newRequest()->getRemainingLockTime('alice')); + + $this->badPassword('alice'); + $this->assertSame(60, $this->newRequest()->getRemainingLockTime('alice')); + } + + public function testExternalBackendIsNeverLocked() { + $this->badPassword('ldapuser', 10); + + $this->assertNull($this->mapper->find('ldapuser')); + $this->assertSame(0, $this->newRequest()->getRemainingLockTime('ldapuser')); + } + + public function testUnknownLoginIsIndistinguishableFromAnExistingAccount() { + $this->badPassword('nosuchuser', 5); + $this->badPassword('alice', 5); + + $this->assertSame( + $this->newRequest()->getRemainingLockTime('alice'), + $this->newRequest()->getRemainingLockTime('nosuchuser') + ); + } + + public function testLockoutOfAnUnknownLoginIsReleasedWhenTheAccountShowsUpExternally() { + $this->badPassword('ldapuser2', 5); + $this->assertGreaterThan(0, $this->newRequest()->getRemainingLockTime('ldapuser2')); + + // the account has been synced from the directory meanwhile + $this->accounts['ldapuser2'] = 'LDAP'; + + $this->assertSame(0, $this->newRequest()->getRemainingLockTime('ldapuser2')); + $this->assertNull($this->mapper->find('ldapuser2')); + } + + public function testCaseVariantsShareOneCounter() { + $this->accounts['admin'] = 'Database'; + + $this->badPassword('admin'); + $this->badPassword('Admin'); + $this->badPassword('ADMIN'); + $this->badPassword('aDmIn'); + $this->badPassword('admiN'); + + $this->assertSame(5, $this->mapper->find('admin')['fail_count']); + $this->assertGreaterThan(0, $this->newRequest()->getRemainingLockTime('ADMIN')); + } + + public function testLoginByEmailRetryCountsOnce() { + // one request, two attempts: the typed email address and the account it + // was resolved to - see Session::logClientIn() + $request = $this->newRequest(); + $request->recordFailure('alice@example.com'); + $request->recordFailure('alice'); + + $this->assertSame(1, $this->mapper->find('alice@example.com')['fail_count']); + $this->assertNull($this->mapper->find('alice')); + } + + public function testConcurrentFailuresDoNotOvershoot() { + // every failure is its own request, as it would be on a cluster + $this->badPassword('alice', 7); + + $row = $this->mapper->find('alice'); + $this->assertSame(7, $row['fail_count'], 'no count may be lost'); + $this->assertSame( + $this->now + AccountLockout::DEFAULT_DURATION, + $row['locked_until'], + 'the lockout starts once and is not extended by the failures which followed' + ); + } + + public function testExpireStaleKeepsRunningLockouts() { + // a lockout outliving the attempt window is the only way its row can be + // both stale and still in effect + $this->systemValues[AccountLockout::CONFIG_DURATION] = 3600; + + $this->badPassword('alice', 5); + $this->badPassword('nosuchuser', 1); + + $this->now += AccountLockout::DEFAULT_ATTEMPT_WINDOW + 1; + $this->newRequest()->expireStale(); + + // alice is still locked, so her row has to survive + $this->assertNotNull($this->mapper->find('alice')); + $this->assertNull($this->mapper->find('nosuchuser')); + + $this->now += 3600; + $this->newRequest()->expireStale(); + + $this->assertNull($this->mapper->find('alice')); + } +} diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index f0c0bde519a8..1328ea3f06f1 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -10,6 +10,8 @@ namespace Test\User; use OC\AppFramework\Http\Request; +use OC\Authentication\AccountLockout\AccountLockedException; +use OC\Authentication\AccountLockout\AccountLockout; use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Token\DefaultToken; @@ -61,6 +63,8 @@ class SessionTest extends TestCase { protected $userSyncService; /** @var EventDispatcher */ protected $eventDispatcher; + /** @var AccountLockout | \PHPUnit\Framework\MockObject\MockObject */ + protected $accountLockout; private $rootNode; private $userNode; @@ -78,6 +82,7 @@ protected function setUp(): void { $this->serviceLoader = $this->createMock(IServiceLoader::class); $this->userSyncService = $this->createMock(\OC\User\SyncService::class); $this->eventDispatcher = new EventDispatcher(); + $this->accountLockout = $this->createMock(AccountLockout::class); // need to overwrite the getUserFolder for the login tests $this->userNode = $this->createMock(Folder::class); @@ -157,7 +162,8 @@ public function testGetUser() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $user = $userSession->getUser(); $this->assertSame($expectedUser, $user); @@ -185,7 +191,7 @@ public function testIsLoggedIn($isLoggedIn) { /** @var \PHPUnit\Framework\MockObject\MockObject | Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods([ 'getUser' ]) @@ -223,7 +229,8 @@ public function testSetUser() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $userSession->setUser($user); } @@ -284,7 +291,7 @@ public function testLoginValidPasswordEnabled() { $eventDispatcher = $this->createMock(EventDispatcher::class); /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $eventDispatcher, $this->accountLockout]) ->setMethods([ 'prepareUserLogin' ]) @@ -352,7 +359,8 @@ public function testLoginValidPasswordDisabled() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $userSession->login('foo', 'bar'); } @@ -371,7 +379,8 @@ public function testLoginInvalidPassword() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $user = $this->createMock(IUser::class); @@ -412,7 +421,8 @@ public function testLoginNonExisting() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $session->expects($this->never()) @@ -432,6 +442,143 @@ public function testLoginNonExisting() { $userSession->login('foo', 'bar'); } + public function testLoginLockedOutIsRefusedBeforeTheCredentialCheck() { + /** @var ISession | \PHPUnit\Framework\MockObject\MockObject $session */ + $session = $this->createMock(Memory::class); + /** @var Manager | \PHPUnit\Framework\MockObject\MockObject $manager */ + $manager = $this->createMock(Manager::class); + $userSession = new Session( + $manager, + $session, + $this->timeFactory, + $this->tokenProvider, + $this->config, + $this->logger, + $this->serviceLoader, + $this->userSyncService, + $this->eventDispatcher, + $this->accountLockout + ); + + $this->tokenProvider->expects($this->once()) + ->method('getToken') + ->with('bar') + ->will($this->throwException(new InvalidTokenException())); + $this->accountLockout->expects($this->once()) + ->method('getRemainingLockTime') + ->with('foo') + ->willReturn(300); + + // the password must not even be hashed while the account is locked + $manager->expects($this->never()) + ->method('checkPassword'); + $this->accountLockout->expects($this->never()) + ->method('recordFailure'); + + $this->expectException(AccountLockedException::class); + $this->expectExceptionMessage('Too many failed login attempts. This account is temporarily locked. Please try again in 5 minutes.'); + + $userSession->login('foo', 'bar'); + } + + public function testLoginWrongPasswordIsCounted() { + /** @var ISession | \PHPUnit\Framework\MockObject\MockObject $session */ + $session = $this->createMock(Memory::class); + /** @var Manager | \PHPUnit\Framework\MockObject\MockObject $manager */ + $manager = $this->createMock(Manager::class); + $userSession = new Session( + $manager, + $session, + $this->timeFactory, + $this->tokenProvider, + $this->config, + $this->logger, + $this->serviceLoader, + $this->userSyncService, + $this->eventDispatcher, + $this->accountLockout + ); + + $this->tokenProvider->expects($this->once()) + ->method('getToken') + ->with('bar') + ->will($this->throwException(new InvalidTokenException())); + $manager->expects($this->once()) + ->method('checkPassword') + ->with('foo', 'bar') + ->willReturn(false); + $this->accountLockout->expects($this->once()) + ->method('recordFailure') + ->with('foo'); + $this->accountLockout->expects($this->never()) + ->method('clearFailures'); + + $this->assertFalse($userSession->login('foo', 'bar')); + } + + public function testLoginSuccessResetsTheCounter() { + /** @var ISession | \PHPUnit\Framework\MockObject\MockObject $session */ + $session = $this->createMock(Memory::class); + /** @var Manager | \PHPUnit\Framework\MockObject\MockObject $manager */ + $manager = $this->createMock(Manager::class); + /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ + $userSession = $this->getMockBuilder(Session::class) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) + ->setMethods(['prepareUserLogin']) + ->getMock(); + + $user = $this->createMock(IUser::class); + $user->method('isEnabled')->willReturn(true); + $user->method('getUID')->willReturn('foo'); + + $this->tokenProvider->expects($this->once()) + ->method('getToken') + ->with('bar') + ->will($this->throwException(new InvalidTokenException())); + $manager->expects($this->once()) + ->method('checkPassword') + ->with('Foo', 'bar') + ->willReturn($user); + + // both the login name as typed and the account it resolved to + $this->accountLockout->expects($this->once()) + ->method('clearFailures') + ->with('Foo', 'foo'); + $this->accountLockout->expects($this->never()) + ->method('recordFailure'); + + $this->assertTrue($userSession->login('Foo', 'bar')); + } + + public function testTryBasicAuthLoginReportsALockedAccountLikeAWrongPassword() { + /** @var ISession | \PHPUnit\Framework\MockObject\MockObject $session */ + $session = $this->createMock(Memory::class); + /** @var Manager | \PHPUnit\Framework\MockObject\MockObject $manager */ + $manager = $this->createMock(Manager::class); + /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ + $userSession = $this->getMockBuilder(Session::class) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) + ->setMethods(['logClientIn']) + ->getMock(); + $userSession->expects($this->once()) + ->method('logClientIn') + ->will($this->throwException(new AccountLockedException('locked'))); + + $csrf = $this->getMockBuilder(CsrfTokenManager::class) + ->disableOriginalConstructor() + ->getMock(); + $request = new Request( + ['server' => ['PHP_AUTH_USER' => 'foo', 'PHP_AUTH_PW' => 'bar']], + $this->createMock(ISecureRandom::class), + $this->createMock(IConfig::class), + $csrf + ); + + // not every caller of \OC::handleLogin() handles a LoginException, so the + // lockout must not escape this way + $this->assertFalse($userSession->tryBasicAuthLogin($request)); + } + /** * When using a device token, the loginname must match the one that was used * when generating the token on the browser. @@ -450,7 +597,8 @@ public function testLoginWithDifferentTokenLoginName() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $username = 'user123'; $token = new DefaultToken(); @@ -491,7 +639,7 @@ public function testLogClientInNoTokenPasswordWith2fa() { /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $eventDispatcher, $this->accountLockout]) ->setMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -531,7 +679,7 @@ public function testLogClientInUnexist($strictLoginCheck, $expectedGetByEmailCal /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -569,7 +717,7 @@ public function testLogClientInWithTokenPassword() { /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods(['isTokenPassword', 'login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -606,7 +754,7 @@ public function testLogClientInNoTokenPasswordNo2fa() { /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $eventDispatcher, $this->accountLockout]) ->setMethods(['login', 'isTwoFactorEnforced']) ->getMock(); @@ -669,7 +817,7 @@ public function testRememberLoginValidToken() { //override, otherwise tests will fail because of setcookie() ->setMethods(['setMagicInCookie']) //there are passed as parameters to the constructor - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->getMock(); $this->assertTrue($userSession->loginWithCookie('foo', $token)); @@ -717,7 +865,8 @@ public function testRememberLoginInvalidToken() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $calledLoginFailedEvent = []; @@ -780,7 +929,8 @@ public function testRememberLoginExpiredToken() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $calledLoginFailedEvent = []; @@ -836,7 +986,8 @@ public function testRememberLoginInvalidUser() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $granted = $userSession->loginWithCookie('foo', $token); @@ -865,7 +1016,7 @@ public function testActiveUserAfterSetSession() { $session->set('user_id', 'foo'); /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods([ 'validateSession' ]) @@ -896,7 +1047,8 @@ public function testCreateSessionToken() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $random = $this->createMock(ISecureRandom::class); @@ -953,7 +1105,8 @@ public function testCreateSessionTokenWithTokenPassword() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $random = $this->createMock(ISecureRandom::class); @@ -1012,7 +1165,8 @@ public function testCreateSessionTokenWithNonExistentUser() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); /** @var IRequest $request */ $request = $this->createMock(IRequest::class); @@ -1048,7 +1202,8 @@ public function testInvalidateSessionToken() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $this->assertNull($userSession->invalidateSessionToken()); } @@ -1070,7 +1225,7 @@ public function testTryTokenLoginWithDisabledUser() { /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) ->setMethods(['logout']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->getMock(); /** @var IRequest | \PHPUnit\Framework\MockObject\MockObject $request */ $request = $this->createMock(IRequest::class); @@ -1109,7 +1264,7 @@ public function testValidateSessionDisabledUser() { /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods(['logout']) ->getMock(); @@ -1162,7 +1317,7 @@ public function testValidateSessionNoPassword() { $tokenProvider = $this->createMock(IProvider::class); /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods(['logout']) ->getMock(); @@ -1214,7 +1369,8 @@ public function testUpdateSessionTokenPassword() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $password = '123456'; @@ -1253,7 +1409,8 @@ public function testUpdateSessionTokenPasswordNoSessionAvailable() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $session->expects($this->once()) @@ -1281,7 +1438,8 @@ public function testUpdateSessionTokenPasswordInvalidTokenException() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $password = '123456'; @@ -1316,7 +1474,8 @@ public function testClearRememberMeTokensForLoggedInUser() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $uid = 'mockedUser'; @@ -1371,7 +1530,8 @@ public function testClearRememberMeTokensForLoggedInUserWithToken() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $uid = 'mockedUser'; @@ -1439,7 +1599,8 @@ public function testCancelLogout() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $userSession->setUser($user); @@ -1482,7 +1643,7 @@ public function testLogout() { /** @var Session | \PHPUnit\Framework\MockObject\MockObject $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods(['getAuthModules', 'unsetMagicInCookie']) ->getMock(); $userSession->setUser($user); @@ -1528,7 +1689,8 @@ public function testApacheLogin() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); // Fail if not userid returned @@ -1565,7 +1727,8 @@ public function testFailedLoginWithApache() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); $apacheBackend = $this->createMock(IApacheBackend::class); @@ -1602,7 +1765,8 @@ public function testLoginWithApache() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); $apacheBackend = $this->createMock(IApacheBackend::class); @@ -1655,7 +1819,8 @@ public function testFailedLoginWithPassword() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); $failedLogin = new GenericEvent(null, ['user' => 'foo']); @@ -1697,7 +1862,8 @@ public function testLoginWithPassword() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); $beforeEvent = new GenericEvent( @@ -1739,7 +1905,8 @@ public function testLoginFailedWithToken() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); $iToken = $this->createMock(IToken::class); @@ -1784,7 +1951,8 @@ public function testLoginWithToken() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); $iToken = $this->createMock(IToken::class); @@ -1876,7 +2044,7 @@ public function testVerifyAuthHeaders($expectedReturn, array $modules, $loggedIn /** @var Session | \PHPUnit\Framework\MockObject\MockObject $session */ $session = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods(['getAuthModules', 'logout', 'isLoggedIn', 'getUser']) ->getMock(); $session->expects($this->any())->method('getAuthModules')->willReturn($modules); @@ -1926,7 +2094,7 @@ public function testTryAuthModuleLogin($expectedReturn, array $modules) { /** @var Session | \PHPUnit\Framework\MockObject\MockObject $session */ $session = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher]) + ->setConstructorArgs([$userManager, $session, $timeFactory, $tokenProvider, $this->config, $this->logger, $this->serviceLoader, $this->userSyncService, $this->eventDispatcher, $this->accountLockout]) ->setMethods(['getAuthModules', 'createSessionToken', 'loginUser', 'getUser']) ->getMock(); $session->expects($this->any())->method('getAuthModules')->willReturn($modules); @@ -1962,7 +2130,8 @@ public function testFailedLoginUser() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); $failedEvent = new GenericEvent(null, ['user' => null]); @@ -1994,7 +2163,8 @@ public function testLoginUser() { $this->logger, $this->serviceLoader, $this->userSyncService, - $this->eventDispatcher + $this->eventDispatcher, + $this->accountLockout ); $iUser = $this->createMock(IUser::class); @@ -2035,7 +2205,8 @@ public function testFailedLoginUserDisabled() { $this->logger, $this->serviceLoader, $this->userSyncService, - $eventDispatcher + $eventDispatcher, + $this->accountLockout ); $iUser = $this->createMock(IUser::class);