From 2994348c4e249f709b73047f9d5abbb523f93952 Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Fri, 11 Sep 2026 08:49:23 -0300 Subject: [PATCH 1/4] perf: move remember login tokens out of oc_preferences Signed-off-by: Cristian Scheid --- core/Controller/LoginController.php | 8 +- .../Version36000Date20260908184209.php | 67 +++++++++ lib/composer/composer/autoload_classmap.php | 3 + lib/composer/composer/autoload_static.php | 3 + .../RememberLogin/RememberLoginToken.php | 38 +++++ .../RememberLoginTokenMapper.php | 84 +++++++++++ lib/private/Server.php | 4 + .../BackgroundJobs/CleanupLoginTokens.php | 6 + .../Listeners/BeforeUserDeletedListener.php | 4 + lib/private/User/Session.php | 67 +++++++-- tests/Core/Controller/LoginControllerTest.php | 16 +- tests/lib/User/SessionTest.php | 142 +++++++++++------- 12 files changed, 370 insertions(+), 72 deletions(-) create mode 100644 core/Migrations/Version36000Date20260908184209.php create mode 100644 lib/private/Authentication/RememberLogin/RememberLoginToken.php create mode 100644 lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 5c11c4bba83dc..31135a2c3b045 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -13,6 +13,7 @@ use OC\AppFramework\Http\Request; use OC\Authentication\Login\Chain; use OC\Authentication\Login\LoginData; +use OC\Authentication\RememberLogin\RememberLoginTokenMapper; use OC\Authentication\WebAuthn\Manager as WebAuthnManager; use OC\User\Session; use OC_App; @@ -68,6 +69,7 @@ public function __construct( private IManager $manager, private IL10N $l10n, private IAppManager $appManager, + private RememberLoginTokenMapper $rememberLoginTokenMapper, ) { parent::__construct($appName, $request); } @@ -82,7 +84,11 @@ public function logout() { $loginToken = $this->request->getCookie('nc_token'); $uid = $this->userSession->getUser()?->getUID(); if ($loginToken !== null && $uid !== null) { - $this->config->deleteUserValue($uid, 'login_token', $loginToken); + $affectedRows = $this->rememberLoginTokenMapper->deleteByToken($loginToken); + if ($affectedRows < 1) { + // TODO: remove this after migration to 'remember_login_tokens' table is finished + $this->config->deleteUserValue($uid, 'login_token', $loginToken); + } } $this->userSession->logout(); diff --git a/core/Migrations/Version36000Date20260908184209.php b/core/Migrations/Version36000Date20260908184209.php new file mode 100644 index 0000000000000..2c22b11f8d979 --- /dev/null +++ b/core/Migrations/Version36000Date20260908184209.php @@ -0,0 +1,67 @@ +hasTable('remember_login_tokens')) { + $table = $schema->createTable('remember_login_tokens'); + $table->addColumn('id', Types::BIGINT, [ + 'autoincrement' => true, + 'notnull' => true, + 'length' => 20, + 'unsigned' => true, + ]); + $table->addColumn('uid', Types::STRING, [ + 'notnull' => true, + 'length' => 64, + ]); + $table->addColumn('token', Types::STRING, [ + 'notnull' => true, + 'length' => 200, + ]); + $table->addColumn('created', Types::BIGINT, [ + 'notnull' => true, + 'length' => 20, + 'unsigned' => true, + ]); + $table->setPrimaryKey(['id']); + $table->addUniqueIndex(['token'], 'remember_login_tokens_token'); + $table->addIndex(['uid'], 'remember_login_tokens_uid'); + + return $schema; + } + + return null; + } +} diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 4467383ba6a9b..7e9a615975a64 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1324,6 +1324,8 @@ 'OC\\Authentication\\Login\\UserDisabledCheckCommand' => $baseDir . '/lib/private/Authentication/Login/UserDisabledCheckCommand.php', 'OC\\Authentication\\Login\\WebAuthnChain' => $baseDir . '/lib/private/Authentication/Login/WebAuthnChain.php', 'OC\\Authentication\\Notifications\\Notifier' => $baseDir . '/lib/private/Authentication/Notifications/Notifier.php', + 'OC\\Authentication\\RememberLogin\\RememberLoginToken' => $baseDir . '/lib/private/Authentication/RememberLogin/RememberLoginToken.php', + 'OC\\Authentication\\RememberLogin\\RememberLoginTokenMapper' => $baseDir . '/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php', 'OC\\Authentication\\Token\\INamedToken' => $baseDir . '/lib/private/Authentication/Token/INamedToken.php', 'OC\\Authentication\\Token\\IProvider' => $baseDir . '/lib/private/Authentication/Token/IProvider.php', 'OC\\Authentication\\Token\\IToken' => $baseDir . '/lib/private/Authentication/Token/IToken.php', @@ -1733,6 +1735,7 @@ 'OC\\Core\\Migrations\\Version34000Date20260518163022' => $baseDir . '/core/Migrations/Version34000Date20260518163022.php', 'OC\\Core\\Migrations\\Version34000Date20260521110333' => $baseDir . '/core/Migrations/Version34000Date20260521110333.php', 'OC\\Core\\Migrations\\Version35000Date20260527162338' => $baseDir . '/core/Migrations/Version35000Date20260527162338.php', + 'OC\\Core\\Migrations\\Version36000Date20260908184209' => $baseDir . '/core/Migrations/Version36000Date20260908184209.php', 'OC\\Core\\Notification\\CoreNotifier' => $baseDir . '/core/Notification/CoreNotifier.php', 'OC\\Core\\ResponseDefinitions' => $baseDir . '/core/ResponseDefinitions.php', 'OC\\Core\\Service\\CronService' => $baseDir . '/core/Service/CronService.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 27b13860aec74..be0af6c217b72 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1365,6 +1365,8 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Authentication\\Login\\UserDisabledCheckCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/UserDisabledCheckCommand.php', 'OC\\Authentication\\Login\\WebAuthnChain' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/WebAuthnChain.php', 'OC\\Authentication\\Notifications\\Notifier' => __DIR__ . '/../../..' . '/lib/private/Authentication/Notifications/Notifier.php', + 'OC\\Authentication\\RememberLogin\\RememberLoginToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/RememberLogin/RememberLoginToken.php', + 'OC\\Authentication\\RememberLogin\\RememberLoginTokenMapper' => __DIR__ . '/../../..' . '/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php', 'OC\\Authentication\\Token\\INamedToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/INamedToken.php', 'OC\\Authentication\\Token\\IProvider' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IProvider.php', 'OC\\Authentication\\Token\\IToken' => __DIR__ . '/../../..' . '/lib/private/Authentication/Token/IToken.php', @@ -1774,6 +1776,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Core\\Migrations\\Version34000Date20260518163022' => __DIR__ . '/../../..' . '/core/Migrations/Version34000Date20260518163022.php', 'OC\\Core\\Migrations\\Version34000Date20260521110333' => __DIR__ . '/../../..' . '/core/Migrations/Version34000Date20260521110333.php', 'OC\\Core\\Migrations\\Version35000Date20260527162338' => __DIR__ . '/../../..' . '/core/Migrations/Version35000Date20260527162338.php', + 'OC\\Core\\Migrations\\Version36000Date20260908184209' => __DIR__ . '/../../..' . '/core/Migrations/Version36000Date20260908184209.php', 'OC\\Core\\Notification\\CoreNotifier' => __DIR__ . '/../../..' . '/core/Notification/CoreNotifier.php', 'OC\\Core\\ResponseDefinitions' => __DIR__ . '/../../..' . '/core/ResponseDefinitions.php', 'OC\\Core\\Service\\CronService' => __DIR__ . '/../../..' . '/core/Service/CronService.php', diff --git a/lib/private/Authentication/RememberLogin/RememberLoginToken.php b/lib/private/Authentication/RememberLogin/RememberLoginToken.php new file mode 100644 index 0000000000000..9f50c1cad45c6 --- /dev/null +++ b/lib/private/Authentication/RememberLogin/RememberLoginToken.php @@ -0,0 +1,38 @@ +addType('uid', Types::STRING); + $this->addType('token', Types::STRING); + $this->addType('created', Types::INTEGER); + } +} diff --git a/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php b/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php new file mode 100644 index 0000000000000..ec268f39458c9 --- /dev/null +++ b/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php @@ -0,0 +1,84 @@ + + */ +class RememberLoginTokenMapper extends QBMapper { + public function __construct( + IDBConnection $db, + private IConfig $config, + ) { + parent::__construct($db, 'remember_login_tokens', RememberLoginToken::class); + } + + #[Override] + public function insert(Entity $entity): Entity { + /** @var RememberLoginToken $entity */ + $entity->setToken($this->hashToken($entity->getToken())); + + return parent::insert($entity); + } + + /** + * @throws DoesNotExistException + */ + public function findByToken(string $token): RememberLoginToken { + $query = $this->db->getQueryBuilder(); + $query->select('*') + ->from($this->getTableName()) + ->where($query->expr()->eq('token', $query->createNamedParameter($this->hashToken($token)))); + + return $this->findEntity($query); + } + + public function deleteByToken(string $token): int { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->eq('token', $query->createNamedParameter($this->hashToken($token)))); + + return $query->executeStatement(); + } + + /** + * Removes every remembered login token for given user + */ + public function deleteByUid(string $uid): int { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->eq('uid', $query->createNamedParameter($uid))); + + return $query->executeStatement(); + } + + /** + * Removes every remembered login token older than the given timestamp + */ + public function deleteOlderThan(int $timestamp): int { + $query = $this->db->getQueryBuilder(); + $query->delete($this->getTableName()) + ->where($query->expr()->lt('created', $query->createNamedParameter($timestamp, IQueryBuilder::PARAM_INT))); + + return $query->executeStatement(); + } + + private function hashToken(string $token): string { + return hash('sha512', $token . $this->config->getSystemValueString('secret')); + } +} diff --git a/lib/private/Server.php b/lib/private/Server.php index ab4f6c93cb68a..7916a1387cf69 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -22,6 +22,7 @@ use OC\Authentication\Listeners\LoginFailedListener; use OC\Authentication\Listeners\UserLoggedInListener; use OC\Authentication\LoginCredentials\Store; +use OC\Authentication\RememberLogin\RememberLoginTokenMapper; use OC\Authentication\Token\IProvider; use OC\Authentication\TwoFactorAuth\Registry; use OC\Avatar\AvatarManager; @@ -435,8 +436,10 @@ public function __construct( // might however be called when Nextcloud is not yet setup. if (\OCP\Server::get(SystemConfig::class)->getValue('installed', false)) { $provider = $c->get(IProvider::class); + $rememberLoginTokenMapper = $c->get(RememberLoginTokenMapper::class); } else { $provider = null; + $rememberLoginTokenMapper = null; } $userSession = new Session( @@ -449,6 +452,7 @@ public function __construct( $c->get(ILockdownManager::class), $c->get(LoggerInterface::class), $c->get(IEventDispatcher::class), + $rememberLoginTokenMapper, ); /** @deprecated 21.0.0 use BeforeUserCreatedEvent event with the IEventDispatcher instead */ $userSession->listen('\OC\User', 'preCreateUser', function ($uid, $password): void { diff --git a/lib/private/User/BackgroundJobs/CleanupLoginTokens.php b/lib/private/User/BackgroundJobs/CleanupLoginTokens.php index 4adbfb3dad5c7..8a0f4bb55530e 100644 --- a/lib/private/User/BackgroundJobs/CleanupLoginTokens.php +++ b/lib/private/User/BackgroundJobs/CleanupLoginTokens.php @@ -9,6 +9,7 @@ namespace OC\User\BackgroundJobs; +use OC\Authentication\RememberLogin\RememberLoginTokenMapper; use OCP\AppFramework\Utility\ITimeFactory; use OCP\BackgroundJob\TimedJob; use OCP\IConfig; @@ -18,6 +19,7 @@ class CleanupLoginTokens extends TimedJob { public function __construct( ITimeFactory $time, private readonly IDBConnection $connection, + private readonly RememberLoginTokenMapper $rememberLoginTokenMapper, private readonly IConfig $config, ) { parent::__construct($time); @@ -28,6 +30,10 @@ public function __construct( #[\Override] protected function run($argument): void { $rememberMeMaxAge = $this->config->getSystemValueInt('remember_login_cookie_lifetime', 60 * 60 * 24 * 15); + + $this->rememberLoginTokenMapper->deleteOlderThan(time() - $rememberMeMaxAge); + + // TODO: remove this after migration to 'remember_login_tokens' table is finished $qb = $this->connection->getQueryBuilder(); $qb ->delete('preferences') diff --git a/lib/private/User/Listeners/BeforeUserDeletedListener.php b/lib/private/User/Listeners/BeforeUserDeletedListener.php index 163ec2bb2cdc6..3d0cc255b9e89 100644 --- a/lib/private/User/Listeners/BeforeUserDeletedListener.php +++ b/lib/private/User/Listeners/BeforeUserDeletedListener.php @@ -9,6 +9,7 @@ namespace OC\User\Listeners; +use OC\Authentication\RememberLogin\RememberLoginTokenMapper; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\Files\NotFoundException; @@ -25,6 +26,7 @@ public function __construct( private LoggerInterface $logger, private IAvatarManager $avatarManager, private ICredentialsManager $credentialsManager, + private RememberLoginTokenMapper $rememberLoginTokenMapper, ) { } @@ -50,5 +52,7 @@ public function handle(Event $event): void { } // Delete storages credentials on user deletion $this->credentialsManager->erase($user->getUID()); + // Delete remember login tokens on user deletion + $this->rememberLoginTokenMapper->deleteByUid($user->getUID()); } } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 578f8e75b9e25..b64a77334bdd9 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -12,6 +12,8 @@ use OC\Authentication\Events\LoginFailed; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Exceptions\PasswordLoginForbiddenException; +use OC\Authentication\RememberLogin\RememberLoginToken; +use OC\Authentication\RememberLogin\RememberLoginTokenMapper; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; use OC\Authentication\Token\PublicKeyToken; @@ -22,6 +24,7 @@ use OC\Security\CSRF\CsrfTokenManager; use OC_User; use OCA\DAV\Connector\Sabre\Auth; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\TTransactional; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Authentication\Exceptions\ExpiredTokenException; @@ -82,6 +85,7 @@ public function __construct( private ILockdownManager $lockdownManager, private LoggerInterface $logger, private IEventDispatcher $dispatcher, + private ?RememberLoginTokenMapper $rememberLoginTokenMapper, ) { } @@ -893,11 +897,22 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { return false; } - // get stored tokens - $tokens = $this->config->getUserKeys($uid, 'login_token'); - // test cookies token against stored tokens - if (!in_array($currentToken, $tokens, true)) { - $this->logger->info('Tried to log in but could not verify token', [ + try { + // get stored token + $rememberLoginToken = $this->rememberLoginTokenMapper->findByToken($currentToken); + } catch (DoesNotExistException $ex) { + // TODO: remove this after migration to 'remember_login_tokens' table is finished + $rememberLoginToken = $this->migrateLegacyRememberLoginToken($uid, $currentToken); + if ($rememberLoginToken === null) { + $this->logger->info('Tried to log in but could not verify token', [ + 'app' => 'core', + 'user' => $uid, + ]); + return false; + } + } + if ($rememberLoginToken->getUid() !== $uid) { + $this->logger->warning('Tried to login using remember-me token token from a different user', [ 'app' => 'core', 'user' => $uid, ]); @@ -924,9 +939,8 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { } // replace successfully used token with a new one - $this->config->deleteUserValue($uid, 'login_token', $currentToken); - $newToken = $this->random->generate(32); - $this->config->setUserValue($uid, 'login_token', $newToken, (string)$this->timeFactory->getTime()); + $this->rememberLoginTokenMapper->deleteByToken($currentToken); + $newToken = $this->createRememberLoginToken($uid); $this->logger->debug('Remember-me token replaced', [ 'app' => 'core', 'user' => $uid, @@ -977,11 +991,44 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { * @param IUser $user */ public function createRememberMeToken(IUser $user) { - $token = $this->random->generate(32); - $this->config->setUserValue($user->getUID(), 'login_token', $token, (string)$this->timeFactory->getTime()); + $token = $this->createRememberLoginToken($user->getUID()); $this->setMagicInCookie($user->getUID(), $token); } + /** + * Generates a new remember login token, stores it for the given user and returns the plain token + */ + private function createRememberLoginToken(string $uid): string { + $token = $this->random->generate(32); + $rememberLoginToken = new RememberLoginToken(); + $rememberLoginToken->setUid($uid); + $rememberLoginToken->setToken($token); + $rememberLoginToken->setCreated($this->timeFactory->getTime()); + $this->rememberLoginTokenMapper->insert($rememberLoginToken); + + return $token; + } + + /** + * TODO: remove this after migration to 'remember_login_tokens' table is finished + */ + private function migrateLegacyRememberLoginToken(string $uid, string $token): ?RememberLoginToken { + $legacyTokens = $this->config->getUserKeys($uid, 'login_token'); + if (!in_array($token, $legacyTokens, true)) { + return null; + } + + $createdAt = (int)$this->config->getUserValue($uid, 'login_token', $token); + $this->config->deleteUserValue($uid, 'login_token', $token); + + $rememberLoginToken = new RememberLoginToken(); + $rememberLoginToken->setUid($uid); + $rememberLoginToken->setToken($token); + $rememberLoginToken->setCreated($createdAt); + + return $this->rememberLoginTokenMapper->insert($rememberLoginToken); + } + /** * logout the user from the session */ diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index 3c8fbd849b936..e7a6528247a77 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -13,6 +13,7 @@ use OC\Authentication\Login\Chain as LoginChain; use OC\Authentication\Login\LoginData; use OC\Authentication\Login\LoginResult; +use OC\Authentication\RememberLogin\RememberLoginTokenMapper; use OC\Authentication\TwoFactorAuth\Manager; use OC\Core\Controller\LoginController; use OC\User\Session; @@ -81,6 +82,9 @@ class LoginControllerTest extends TestCase { /** @var IAppManager|MockObject */ private $appManager; + /** @var RememberLoginTokenMapper|MockObject */ + private $rememberLoginTokenMapper; + #[\Override] protected function setUp(): void { parent::setUp(); @@ -98,6 +102,7 @@ protected function setUp(): void { $this->notificationManager = $this->createMock(IManager::class); $this->l = $this->createMock(IL10N::class); $this->appManager = $this->createMock(IAppManager::class); + $this->rememberLoginTokenMapper = $this->createMock(RememberLoginTokenMapper::class); $this->l->expects($this->any()) ->method('t') @@ -131,6 +136,7 @@ protected function setUp(): void { $this->notificationManager, $this->l, $this->appManager, + $this->rememberLoginTokenMapper, ); } @@ -147,9 +153,9 @@ public function testLogoutWithoutToken(): void { ->expects($this->once()) ->method('isUserAgent') ->willReturn(false); - $this->config + $this->rememberLoginTokenMapper ->expects($this->never()) - ->method('deleteUserValue'); + ->method('deleteByToken'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') @@ -206,10 +212,10 @@ public function testLogoutWithToken(): void { ->expects($this->once()) ->method('getUser') ->willReturn($user); - $this->config + $this->rememberLoginTokenMapper ->expects($this->once()) - ->method('deleteUserValue') - ->with('JohnDoe', 'login_token', 'MyLoginToken'); + ->method('deleteByToken') + ->with('MyLoginToken'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index f466569eb9b3c..bc7c78695fd74 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -13,6 +13,8 @@ use OC\Authentication\Exceptions\InvalidTokenException; use OC\Authentication\Exceptions\PasswordlessTokenException; use OC\Authentication\Exceptions\PasswordLoginForbiddenException; +use OC\Authentication\RememberLogin\RememberLoginToken; +use OC\Authentication\RememberLogin\RememberLoginTokenMapper; use OC\Authentication\Token\IProvider; use OC\Authentication\Token\IToken; use OC\Authentication\Token\PublicKeyToken; @@ -23,6 +25,7 @@ use OC\User\Session; use OC\User\User; use OCA\DAV\Connector\Sabre\Auth; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; use OCP\ICacheFactory; @@ -68,6 +71,8 @@ class SessionTest extends \Test\TestCase { private $logger; /** @var IEventDispatcher|MockObject */ private $dispatcher; + /** @var RememberLoginTokenMapper|MockObject */ + private $rememberLoginTokenMapper; #[\Override] protected function setUp(): void { @@ -86,6 +91,7 @@ protected function setUp(): void { $this->lockdownManager = $this->createMock(ILockdownManager::class); $this->logger = $this->createMock(LoggerInterface::class); $this->dispatcher = $this->createMock(IEventDispatcher::class); + $this->rememberLoginTokenMapper = $this->createMock(RememberLoginTokenMapper::class); $this->userSession = $this->getMockBuilder(Session::class) ->setConstructorArgs([ $this->manager, @@ -96,7 +102,8 @@ protected function setUp(): void { $this->random, $this->lockdownManager, $this->logger, - $this->dispatcher + $this->dispatcher, + $this->rememberLoginTokenMapper, ]) ->onlyMethods([ 'setMagicInCookie', @@ -120,7 +127,7 @@ public function testIsLoggedIn($isLoggedIn): void { $manager = $this->createMock(Manager::class); $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods([ 'getUser' ]) @@ -147,7 +154,7 @@ public function testSetUser(): void { ->method('getUID') ->willReturn('foo'); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $userSession->setUser($user); } @@ -204,7 +211,7 @@ public function testLoginValidPasswordEnabled(): void { ->willReturn($user); $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods([ 'prepareUserLogin' ]) @@ -267,7 +274,7 @@ public function testLoginValidPasswordDisabled(): void { $this->dispatcher->expects($this->never()) ->method('dispatch'); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $userSession->login('foo', 'bar'); } @@ -286,7 +293,7 @@ public function testLoginInvalidPassword(): void { ]) ->getMock(); $backend = $this->createMock(\Test\Util\User\Dummy::class); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $user = $this->createMock(IUser::class); @@ -329,7 +336,7 @@ public function testPasswordlessLoginNoLastCheckUpdate(): void { $this->createMock(LoggerInterface::class), ]) ->getMock(); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('foo'); @@ -373,7 +380,7 @@ public function testLoginLastCheckUpdate(): void { $this->createMock(LoggerInterface::class), ]) ->getMock(); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $user = $this->createMock(IUser::class); $user->method('getUID')->willReturn('foo'); @@ -406,7 +413,7 @@ public function testLoginLastCheckUpdate(): void { public function testLoginNonExisting(): void { $session = $this->createMock(Memory::class); $manager = $this->createMock(Manager::class); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $session->expects($this->never()) ->method('set'); @@ -434,7 +441,7 @@ public function testLogClientInNoTokenPasswordWith2fa(): void { /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -479,7 +486,7 @@ public function testLogClientInUnexist(): void { /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -505,7 +512,7 @@ public function testLogClientInWithTokenPassword(): void { /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -547,7 +554,7 @@ public function testLogClientInNoTokenPasswordNo2fa(): void { /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods(['login', 'isTwoFactorEnforced']) ->getMock(); @@ -750,7 +757,7 @@ public function testRememberLoginValidToken(): void { $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->onlyMethods(['setMagicInCookie', 'setLoginName']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->getMock(); $user = $this->createMock(IUser::class); @@ -764,20 +771,30 @@ public function testRememberLoginValidToken(): void { ->method('get') ->with('foo') ->willReturn($user); - $this->config->expects($this->once()) - ->method('getUserKeys') - ->with('foo', 'login_token') - ->willReturn([$token]); - $this->config->expects($this->once()) - ->method('deleteUserValue') - ->with('foo', 'login_token', $token); + + $storedRememberLoginToken = new RememberLoginToken(); + $storedRememberLoginToken->setUid('foo'); + $storedRememberLoginToken->setToken($token); + $storedRememberLoginToken->setCreated(9000); + + $this->rememberLoginTokenMapper->expects($this->once()) + ->method('findByToken') + ->with($token) + ->willReturn($storedRememberLoginToken); + $this->rememberLoginTokenMapper->expects($this->once()) + ->method('deleteByToken') + ->with($token); $this->random->expects($this->once()) ->method('generate') ->with(32) ->willReturn('abcdefg123456'); - $this->config->expects($this->once()) - ->method('setUserValue') - ->with('foo', 'login_token', 'abcdefg123456', 10000); + $this->rememberLoginTokenMapper->expects($this->once()) + ->method('insert') + ->with($this->callback(function (RememberLoginToken $newRememberLoginToken): bool { + return $newRememberLoginToken->getUid() === 'foo' + && $newRememberLoginToken->getToken() === 'abcdefg123456' + && $newRememberLoginToken->getCreated() === 10000; + })); $tokenObject = $this->createMock(IToken::class); $tokenObject->expects($this->once()) @@ -846,7 +863,7 @@ public function testRememberLoginInvalidSessionToken(): void { $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->onlyMethods(['setMagicInCookie']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->getMock(); $user = $this->createMock(IUser::class); @@ -860,15 +877,21 @@ public function testRememberLoginInvalidSessionToken(): void { ->method('get') ->with('foo') ->willReturn($user); - $this->config->expects($this->once()) - ->method('getUserKeys') - ->with('foo', 'login_token') - ->willReturn([$token]); - $this->config->expects($this->once()) - ->method('deleteUserValue') - ->with('foo', 'login_token', $token); - $this->config->expects($this->once()) - ->method('setUserValue'); // TODO: mock new random value + + $storedRememberLoginToken = new RememberLoginToken(); + $storedRememberLoginToken->setUid('foo'); + $storedRememberLoginToken->setToken($token); + $storedRememberLoginToken->setCreated(9000); + + $this->rememberLoginTokenMapper->expects($this->once()) + ->method('findByToken') + ->with($token) + ->willReturn($storedRememberLoginToken); + $this->rememberLoginTokenMapper->expects($this->once()) + ->method('deleteByToken') + ->with($token); + $this->rememberLoginTokenMapper->expects($this->once()) + ->method('insert'); $session->expects($this->once()) ->method('getId') @@ -920,7 +943,7 @@ public function testRememberLoginInvalidToken(): void { $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->onlyMethods(['setMagicInCookie']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->getMock(); $user = $this->createMock(IUser::class); @@ -933,13 +956,16 @@ public function testRememberLoginInvalidToken(): void { ->method('get') ->with('foo') ->willReturn($user); + $this->rememberLoginTokenMapper->expects($this->once()) + ->method('findByToken') + ->with($token) + ->willThrowException(new DoesNotExistException('')); $this->config->expects($this->once()) ->method('getUserKeys') ->with('foo', 'login_token') - ->willReturn(['anothertoken']); - $this->config->expects($this->never()) - ->method('deleteUserValue') - ->with('foo', 'login_token', $token); + ->willReturn([]); + $this->rememberLoginTokenMapper->expects($this->never()) + ->method('deleteByToken'); $this->tokenProvider->expects($this->never()) ->method('renewSessionToken'); @@ -973,7 +999,7 @@ public function testRememberLoginInvalidUser(): void { $userSession = $this->getMockBuilder(Session::class) //override, otherwise tests will fail because of setcookie() ->onlyMethods(['setMagicInCookie']) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->getMock(); $token = 'goodToken'; $oldSessionId = 'sess321'; @@ -984,10 +1010,8 @@ public function testRememberLoginInvalidUser(): void { ->method('get') ->with('foo') ->willReturn(null); - $this->config->expects($this->never()) - ->method('getUserKeys') - ->with('foo', 'login_token') - ->willReturn(['anothertoken']); + $this->rememberLoginTokenMapper->expects($this->never()) + ->method('findByToken'); $this->tokenProvider->expects($this->never()) ->method('renewSessionToken'); @@ -1021,7 +1045,7 @@ public function testActiveUserAfterSetSession(): void { $session = new Memory(); $session->set('user_id', 'foo'); $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods([ 'validateSession' ]) @@ -1041,7 +1065,7 @@ public function testCreateSessionToken(): void { $manager = $this->createMock(Manager::class); $session = $this->createMock(ISession::class); $user = $this->createMock(IUser::class); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $requestId = $this->createMock(IRequestId::class); $config = $this->createMock(IConfig::class); @@ -1082,7 +1106,7 @@ public function testCreateRememberedSessionToken(): void { $manager = $this->createMock(Manager::class); $session = $this->createMock(ISession::class); $user = $this->createMock(IUser::class); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $requestId = $this->createMock(IRequestId::class); $config = $this->createMock(IConfig::class); @@ -1126,7 +1150,7 @@ public function testCreateSessionTokenWithTokenPassword(): void { $session = $this->createMock(ISession::class); $token = $this->createMock(IToken::class); $user = $this->createMock(IUser::class); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $requestId = $this->createMock(IRequestId::class); $config = $this->createMock(IConfig::class); @@ -1173,7 +1197,7 @@ public function testCreateSessionTokenWithNonExistentUser(): void { ->disableOriginalConstructor() ->getMock(); $session = $this->createMock(ISession::class); - $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher); + $userSession = new Session($manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper); $request = $this->createMock(IRequest::class); $uid = 'user123'; @@ -1199,10 +1223,14 @@ public function testCreateRememberMeToken(): void { ->method('generate') ->with(32) ->willReturn('LongRandomToken'); - $this->config + $this->rememberLoginTokenMapper ->expects($this->once()) - ->method('setUserValue') - ->with('UserUid', 'login_token', 'LongRandomToken', 10000); + ->method('insert') + ->with($this->callback(function (RememberLoginToken $rememberLoginToken): bool { + return $rememberLoginToken->getUid() === 'UserUid' + && $rememberLoginToken->getToken() === 'LongRandomToken' + && $rememberLoginToken->getCreated() === 10000; + })); $this->userSession ->expects($this->once()) ->method('setMagicInCookie') @@ -1246,7 +1274,8 @@ public function testTryBasicAuthLoginValid(): void { $this->random, $this->lockdownManager, $this->logger, - $this->dispatcher + $this->dispatcher, + $this->rememberLoginTokenMapper, ]) ->onlyMethods([ 'logClientIn', @@ -1297,7 +1326,8 @@ public function testTryBasicAuthLoginNoLogin(): void { $this->random, $this->lockdownManager, $this->logger, - $this->dispatcher + $this->dispatcher, + $this->rememberLoginTokenMapper, ]) ->onlyMethods([ 'logClientIn', @@ -1326,7 +1356,7 @@ public function testLogClientInThrottlerUsername(): void { /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -1373,7 +1403,7 @@ public function testLogClientInThrottlerEmail(): void { /** @var Session $userSession */ $userSession = $this->getMockBuilder(Session::class) - ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher]) + ->setConstructorArgs([$manager, $session, $this->timeFactory, $this->tokenProvider, $this->config, $this->random, $this->lockdownManager, $this->logger, $this->dispatcher, $this->rememberLoginTokenMapper]) ->onlyMethods(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); From b1ca9ad3bea1c10b0df377c611922b162283c17b Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Mon, 14 Sep 2026 14:48:13 -0300 Subject: [PATCH 2/4] fix: bump version.php to trigger DB upgrades on dev instances Signed-off-by: Cristian Scheid --- version.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.php b/version.php index 31e5cb2bcda4c..ca54c28709bc6 100644 --- a/version.php +++ b/version.php @@ -11,7 +11,7 @@ // between betas, final and RCs. This is _not_ the public version number. Reset minor/patch level // when updating major/minor version number. -$OC_Version = [36, 0, 0, 0]; +$OC_Version = [36, 0, 0, 1]; // The human-readable string $OC_VersionString = '36.0.0 dev'; From 41711499571ade5aa7f885b475310ab2fd20aeec Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Mon, 14 Sep 2026 14:50:26 -0300 Subject: [PATCH 3/4] refactor: port RememberLoginToken to new entity system and use snowflake ids Signed-off-by: Cristian Scheid --- .../Version36000Date20260908184209.php | 8 +-- .../RememberLogin/RememberLoginToken.php | 43 +++++------- .../RememberLoginTokenMapper.php | 66 +++++++++++-------- lib/private/User/Session.php | 17 ++--- tests/lib/User/SessionTest.php | 31 +++------ 5 files changed, 71 insertions(+), 94 deletions(-) diff --git a/core/Migrations/Version36000Date20260908184209.php b/core/Migrations/Version36000Date20260908184209.php index 2c22b11f8d979..45f0be72a8856 100644 --- a/core/Migrations/Version36000Date20260908184209.php +++ b/core/Migrations/Version36000Date20260908184209.php @@ -21,7 +21,7 @@ #[CreateTable( table: 'remember_login_tokens', - columns: ['uid', 'token', 'created'], + columns: ['uid', 'token'], description: 'New table to store remember login tokens, replacing the login_token entries kept in oc_preferences', )] #[AddIndex(table: 'remember_login_tokens', type: IndexType::PRIMARY)] @@ -37,7 +37,6 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt if (!$schema->hasTable('remember_login_tokens')) { $table = $schema->createTable('remember_login_tokens'); $table->addColumn('id', Types::BIGINT, [ - 'autoincrement' => true, 'notnull' => true, 'length' => 20, 'unsigned' => true, @@ -50,11 +49,6 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt 'notnull' => true, 'length' => 200, ]); - $table->addColumn('created', Types::BIGINT, [ - 'notnull' => true, - 'length' => 20, - 'unsigned' => true, - ]); $table->setPrimaryKey(['id']); $table->addUniqueIndex(['token'], 'remember_login_tokens_token'); $table->addIndex(['uid'], 'remember_login_tokens_uid'); diff --git a/lib/private/Authentication/RememberLogin/RememberLoginToken.php b/lib/private/Authentication/RememberLogin/RememberLoginToken.php index 9f50c1cad45c6..1a92b3ba57db8 100644 --- a/lib/private/Authentication/RememberLogin/RememberLoginToken.php +++ b/lib/private/Authentication/RememberLogin/RememberLoginToken.php @@ -9,30 +9,21 @@ namespace OC\Authentication\RememberLogin; -use OCP\AppFramework\Db\Entity; -use OCP\DB\Types; - -/** - * @method void setUid(string $uid) - * @method string getUid() - * @method void setToken(string $token) - * @method string getToken() - * @method void setCreated(int $created) - * @method int getCreated() - */ -class RememberLoginToken extends Entity { - /** @var string */ - protected $uid; - - /** @var string */ - protected $token; - - /** @var int */ - protected $created; - - public function __construct() { - $this->addType('uid', Types::STRING); - $this->addType('token', Types::STRING); - $this->addType('created', Types::INTEGER); - } +use OCP\AppFramework\ORM\Attribute\Column; +use OCP\AppFramework\ORM\Attribute\Entity; +use OCP\AppFramework\ORM\Attribute\Id; +use OCP\DB\Schema\ColumnType; +use OCP\Snowflake\ISnowflakeGenerator; + +#[Entity(name: 'remember_login_tokens')] +final class RememberLoginToken { + #[Id(generatorClass: ISnowflakeGenerator::class)] + #[Column(name: 'id', type: ColumnType::Bigint)] + public ?string $id = null; + + #[Column(name: 'uid', type: ColumnType::String, length: 64)] + public string $uid; + + #[Column(name: 'token', type: ColumnType::String, length: 200)] + public string $token; } diff --git a/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php b/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php index ec268f39458c9..5b738a236543d 100644 --- a/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php +++ b/lib/private/Authentication/RememberLogin/RememberLoginTokenMapper.php @@ -9,29 +9,34 @@ namespace OC\Authentication\RememberLogin; +use OC\AppFramework\ORM\EntityManager; use OCP\AppFramework\Db\DoesNotExistException; -use OCP\AppFramework\Db\Entity; -use OCP\AppFramework\Db\QBMapper; -use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\AppFramework\ORM\Repository; use OCP\IConfig; use OCP\IDBConnection; +use OCP\Snowflake\ISnowflakeGenerator; use Override; /** - * @template-extends QBMapper + * @template-extends Repository */ -class RememberLoginTokenMapper extends QBMapper { +class RememberLoginTokenMapper extends Repository { + public const string entityClass = RememberLoginToken::class; + public function __construct( - IDBConnection $db, - private IConfig $config, + IDBConnection $connection, + EntityManager $entityManager, + private readonly ISnowflakeGenerator $snowflakeGenerator, + private readonly IConfig $config, ) { - parent::__construct($db, 'remember_login_tokens', RememberLoginToken::class); + /** @psalm-suppress InternalMethod */ + parent::__construct($connection, $entityManager); } #[Override] - public function insert(Entity $entity): Entity { + public function insert(object $entity): object { /** @var RememberLoginToken $entity */ - $entity->setToken($this->hashToken($entity->getToken())); + $entity->token = $this->hashToken($entity->token); return parent::insert($entity); } @@ -40,42 +45,45 @@ public function insert(Entity $entity): Entity { * @throws DoesNotExistException */ public function findByToken(string $token): RememberLoginToken { - $query = $this->db->getQueryBuilder(); - $query->select('*') - ->from($this->getTableName()) - ->where($query->expr()->eq('token', $query->createNamedParameter($this->hashToken($token)))); - - return $this->findEntity($query); + return $this->findOneBy(['token' => $this->hashToken($token)]); } public function deleteByToken(string $token): int { - $query = $this->db->getQueryBuilder(); - $query->delete($this->getTableName()) - ->where($query->expr()->eq('token', $query->createNamedParameter($this->hashToken($token)))); - - return $query->executeStatement(); + return $this->deleteBy(['token' => $this->hashToken($token)]); } /** * Removes every remembered login token for given user */ public function deleteByUid(string $uid): int { - $query = $this->db->getQueryBuilder(); - $query->delete($this->getTableName()) - ->where($query->expr()->eq('uid', $query->createNamedParameter($uid))); + return $this->deleteBy(['uid' => $uid]); + } - return $query->executeStatement(); + /** + * Updates old token with the new one and generates a new snowflake ID, + * refreshing the creation timestamp encoded in it + * + * @return int Number of updated rows + */ + public function rotateToken(string $oldToken, string $newToken): int { + $qb = $this->connection->getQueryBuilder(); + $qb->update($this->getTableName()) + ->set('id', $qb->createNamedParameter($this->snowflakeGenerator->nextId())) + ->set('token', $qb->createNamedParameter($this->hashToken($newToken))) + ->where($qb->expr()->eq('token', $qb->createNamedParameter($this->hashToken($oldToken)))); + + return $qb->executeStatement(); } /** * Removes every remembered login token older than the given timestamp */ public function deleteOlderThan(int $timestamp): int { - $query = $this->db->getQueryBuilder(); - $query->delete($this->getTableName()) - ->where($query->expr()->lt('created', $query->createNamedParameter($timestamp, IQueryBuilder::PARAM_INT))); + $qb = $this->connection->getQueryBuilder(); + $qb->delete($this->getTableName()) + ->where($qb->expr()->lt('id', $qb->createNamedParameter($this->snowflakeGenerator->minForTimeId($timestamp)))); - return $query->executeStatement(); + return $qb->executeStatement(); } private function hashToken(string $token): string { diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index b64a77334bdd9..6576ba7c3a651 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -911,7 +911,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { return false; } } - if ($rememberLoginToken->getUid() !== $uid) { + if ($rememberLoginToken->uid !== $uid) { $this->logger->warning('Tried to login using remember-me token token from a different user', [ 'app' => 'core', 'user' => $uid, @@ -939,8 +939,8 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { } // replace successfully used token with a new one - $this->rememberLoginTokenMapper->deleteByToken($currentToken); - $newToken = $this->createRememberLoginToken($uid); + $newToken = $this->random->generate(32); + $this->rememberLoginTokenMapper->rotateToken($currentToken, $newToken); $this->logger->debug('Remember-me token replaced', [ 'app' => 'core', 'user' => $uid, @@ -1001,9 +1001,8 @@ public function createRememberMeToken(IUser $user) { private function createRememberLoginToken(string $uid): string { $token = $this->random->generate(32); $rememberLoginToken = new RememberLoginToken(); - $rememberLoginToken->setUid($uid); - $rememberLoginToken->setToken($token); - $rememberLoginToken->setCreated($this->timeFactory->getTime()); + $rememberLoginToken->uid = $uid; + $rememberLoginToken->token = $token; $this->rememberLoginTokenMapper->insert($rememberLoginToken); return $token; @@ -1018,13 +1017,11 @@ private function migrateLegacyRememberLoginToken(string $uid, string $token): ?R return null; } - $createdAt = (int)$this->config->getUserValue($uid, 'login_token', $token); $this->config->deleteUserValue($uid, 'login_token', $token); $rememberLoginToken = new RememberLoginToken(); - $rememberLoginToken->setUid($uid); - $rememberLoginToken->setToken($token); - $rememberLoginToken->setCreated($createdAt); + $rememberLoginToken->uid = $uid; + $rememberLoginToken->token = $token; return $this->rememberLoginTokenMapper->insert($rememberLoginToken); } diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php index bc7c78695fd74..9e20d15d35a1d 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -773,28 +773,20 @@ public function testRememberLoginValidToken(): void { ->willReturn($user); $storedRememberLoginToken = new RememberLoginToken(); - $storedRememberLoginToken->setUid('foo'); - $storedRememberLoginToken->setToken($token); - $storedRememberLoginToken->setCreated(9000); + $storedRememberLoginToken->uid = 'foo'; + $storedRememberLoginToken->token = $token; $this->rememberLoginTokenMapper->expects($this->once()) ->method('findByToken') ->with($token) ->willReturn($storedRememberLoginToken); - $this->rememberLoginTokenMapper->expects($this->once()) - ->method('deleteByToken') - ->with($token); $this->random->expects($this->once()) ->method('generate') ->with(32) ->willReturn('abcdefg123456'); $this->rememberLoginTokenMapper->expects($this->once()) - ->method('insert') - ->with($this->callback(function (RememberLoginToken $newRememberLoginToken): bool { - return $newRememberLoginToken->getUid() === 'foo' - && $newRememberLoginToken->getToken() === 'abcdefg123456' - && $newRememberLoginToken->getCreated() === 10000; - })); + ->method('rotateToken') + ->with($token, 'abcdefg123456'); $tokenObject = $this->createMock(IToken::class); $tokenObject->expects($this->once()) @@ -879,19 +871,15 @@ public function testRememberLoginInvalidSessionToken(): void { ->willReturn($user); $storedRememberLoginToken = new RememberLoginToken(); - $storedRememberLoginToken->setUid('foo'); - $storedRememberLoginToken->setToken($token); - $storedRememberLoginToken->setCreated(9000); + $storedRememberLoginToken->uid = 'foo'; + $storedRememberLoginToken->token = $token; $this->rememberLoginTokenMapper->expects($this->once()) ->method('findByToken') ->with($token) ->willReturn($storedRememberLoginToken); $this->rememberLoginTokenMapper->expects($this->once()) - ->method('deleteByToken') - ->with($token); - $this->rememberLoginTokenMapper->expects($this->once()) - ->method('insert'); + ->method('rotateToken'); $session->expects($this->once()) ->method('getId') @@ -1227,9 +1215,8 @@ public function testCreateRememberMeToken(): void { ->expects($this->once()) ->method('insert') ->with($this->callback(function (RememberLoginToken $rememberLoginToken): bool { - return $rememberLoginToken->getUid() === 'UserUid' - && $rememberLoginToken->getToken() === 'LongRandomToken' - && $rememberLoginToken->getCreated() === 10000; + return $rememberLoginToken->uid === 'UserUid' + && $rememberLoginToken->token === 'LongRandomToken'; })); $this->userSession ->expects($this->once()) From 92d43c82087f079325d400b8eef6e077b969677f Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Wed, 16 Sep 2026 09:26:54 -0300 Subject: [PATCH 4/4] fix(remember-login-token): avoid unecessary insert when migrating from old table Signed-off-by: Cristian Scheid --- lib/private/User/Session.php | 53 ++++++++----------- tests/Core/Controller/LoginControllerTest.php | 3 +- 2 files changed, 24 insertions(+), 32 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 6576ba7c3a651..becc830dac8eb 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -897,13 +897,25 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { return false; } + $isLegacyRememberLoginToken = false; try { // get stored token $rememberLoginToken = $this->rememberLoginTokenMapper->findByToken($currentToken); + if ($rememberLoginToken->uid !== $uid) { + $this->logger->warning('Tried to login using remember-me token token from a different user', [ + 'app' => 'core', + 'user' => $uid, + ]); + return false; + } } catch (DoesNotExistException $ex) { // TODO: remove this after migration to 'remember_login_tokens' table is finished - $rememberLoginToken = $this->migrateLegacyRememberLoginToken($uid, $currentToken); - if ($rememberLoginToken === null) { + $legacyRememberLoginTokens = $this->config->getUserKeys($uid, 'login_token'); + $isLegacyRememberLoginToken = in_array($currentToken, $legacyRememberLoginTokens, true); + if ($isLegacyRememberLoginToken) { + // remove token from 'preferences' table + $this->config->deleteUserValue($uid, 'login_token', $currentToken); + } else { $this->logger->info('Tried to log in but could not verify token', [ 'app' => 'core', 'user' => $uid, @@ -911,13 +923,6 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { return false; } } - if ($rememberLoginToken->uid !== $uid) { - $this->logger->warning('Tried to login using remember-me token token from a different user', [ - 'app' => 'core', - 'user' => $uid, - ]); - return false; - } try { $oldToken = $this->tokenProvider->getToken($oldSessionId); @@ -938,9 +943,15 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { return false; } - // replace successfully used token with a new one - $newToken = $this->random->generate(32); - $this->rememberLoginTokenMapper->rotateToken($currentToken, $newToken); + if ($isLegacyRememberLoginToken) { + // legacy token was removed from 'preferences' table + // create new one on 'remember_login_tokens' table + $newToken = $this->createRememberLoginToken($uid); + } else { + // replace successfully used token with a new one + $newToken = $this->random->generate(32); + $this->rememberLoginTokenMapper->rotateToken($currentToken, $newToken); + } $this->logger->debug('Remember-me token replaced', [ 'app' => 'core', 'user' => $uid, @@ -1008,24 +1019,6 @@ private function createRememberLoginToken(string $uid): string { return $token; } - /** - * TODO: remove this after migration to 'remember_login_tokens' table is finished - */ - private function migrateLegacyRememberLoginToken(string $uid, string $token): ?RememberLoginToken { - $legacyTokens = $this->config->getUserKeys($uid, 'login_token'); - if (!in_array($token, $legacyTokens, true)) { - return null; - } - - $this->config->deleteUserValue($uid, 'login_token', $token); - - $rememberLoginToken = new RememberLoginToken(); - $rememberLoginToken->uid = $uid; - $rememberLoginToken->token = $token; - - return $this->rememberLoginTokenMapper->insert($rememberLoginToken); - } - /** * logout the user from the session */ diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php index e7a6528247a77..6b2b674ca1dc1 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -82,8 +82,7 @@ class LoginControllerTest extends TestCase { /** @var IAppManager|MockObject */ private $appManager; - /** @var RememberLoginTokenMapper|MockObject */ - private $rememberLoginTokenMapper; + private RememberLoginTokenMapper&MockObject $rememberLoginTokenMapper; #[\Override] protected function setUp(): void {