From cf206e5b1d321d2358e0d00b3485b2a5ff0e7d7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <323649642+oc-tmueller@users.noreply.github.com> Date: Fri, 4 Sep 2026 11:36:58 +0200 Subject: [PATCH] feat: temporarily lock out local accounts after failed logins (OC10-153) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Core did not throttle password authentication at all (CWE-307), so a local account with a weak password could be brute-forced online at request rate unless an optional app was installed. Every password verifying entry point was an unlimited oracle. Adds a DB backed lockout for accounts of the built-in `Database` backend: five failed attempts within 15 minutes lock the account for ten minutes. The lockout always expires on its own - no administrative unlock exists and no account is ever disabled - and a successful login clears the counter immediately. All four values are configurable via `account_lockout.*`. Gated at the three entry points which verify a password against the user backend, ahead of the credential check so a locked account does not even pay for the hash: - Session::loginWithPassword(), covering the web login form, WebDAV and OCS basic auth - TokenController::generateToken() (app password creation) - OcsController::checkPerson() Token, auth module and remember-me logins are untouched: they revalidate an existing credential rather than a submitted password, so throttling them would only lock out already authenticated clients. Accounts of an external backend are not tracked - LDAP/AD counts the same attempt itself via badPwdCount and OIDC lockout belongs to the IdP, so counting twice would punish twice. Login names which do not resolve to any account are tracked under the same key space and produce exactly the same response as a locked existing account, otherwise the lockout would answer whether a user name exists. The unlocked read path performs no account lookup at all, so there is no timing difference either. Notable details: - Keys are lower cased in PHP, so `admin`, `Admin` and `ADMIN` share one budget of attempts regardless of the collation of the database. - Every counter update is a single statement, so requests served by different application servers cannot lose a count, and the lockout is started by a conditional UPDATE so concurrent failures cannot extend a running one. - One request may verify the same credentials twice (login by email is retried with the resolved uid); that is counted once. - The login route stays available while an upgrade is pending, so every statement tolerates the table being absent and simply does not lock anybody out until the migration has run. - Deleting an account drops its counter, so a recreated namesake does not inherit it. - tryBasicAuthLogin() reports a lockout like a wrong password because not every caller of \OC::handleLogin() handles a LoginException; the login form and the DAV backend, which can carry the explanation, do not use that method. Co-Authored-By: Claude Opus 5 Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --- changelog/unreleased/41805 | 15 + config/config.sample.php | 28 ++ core/Application.php | 4 +- core/Controller/LoginController.php | 29 +- core/Controller/OcsController.php | 17 +- core/Controller/TokenController.php | 19 +- core/Migrations/Version20260904090000.php | 68 +++++ core/Migrations/Version20260904090001.php | 39 +++ .../AccountLockout/AccountLockedException.php | 35 +++ .../AccountLockout/AccountLockout.php | 265 ++++++++++++++++ .../AccountLockout/ExpireLockoutsJob.php | 39 +++ .../AccountLockout/LockoutMapper.php | 210 +++++++++++++ lib/private/Server.php | 19 +- lib/private/Setup.php | 1 + lib/private/User/Session.php | 48 ++- tests/Core/Controller/LoginControllerTest.php | 25 ++ tests/Core/Controller/OcsControllerTest.php | 50 ++- tests/Core/Controller/TokenControllerTest.php | 41 ++- .../AccountLockout/AccountLockoutTest.php | 287 ++++++++++++++++++ tests/lib/User/SessionTest.php | 257 +++++++++++++--- 20 files changed, 1434 insertions(+), 62 deletions(-) create mode 100644 changelog/unreleased/41805 create mode 100644 core/Migrations/Version20260904090000.php create mode 100644 core/Migrations/Version20260904090001.php create mode 100644 lib/private/Authentication/AccountLockout/AccountLockedException.php create mode 100644 lib/private/Authentication/AccountLockout/AccountLockout.php create mode 100644 lib/private/Authentication/AccountLockout/ExpireLockoutsJob.php create mode 100644 lib/private/Authentication/AccountLockout/LockoutMapper.php create mode 100644 tests/lib/Authentication/AccountLockout/AccountLockoutTest.php 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);