From b985d882990515824402ba709a33e76d38205aad Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Fri, 11 Sep 2026 08:49:23 -0300 Subject: [PATCH 1/5] 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 | 139 +++++++++++------- 12 files changed, 368 insertions(+), 71 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 f5067cbca275a..67c00fb1b1af5 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -14,6 +14,7 @@ use OC\Authentication\Login\AlternativeLoginService; 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 OCA\User_LDAP\Configuration; @@ -71,6 +72,7 @@ public function __construct( private readonly IL10N $l10n, private readonly IAppManager $appManager, private readonly AlternativeLoginService $alternativeLoginService, + private readonly RememberLoginTokenMapper $rememberLoginTokenMapper, ) { parent::__construct($appName, $request); } @@ -82,7 +84,11 @@ public function logout(): RedirectResponse { $loginToken = $this->request->getCookie('nc_token'); $uid = $this->userSession->getUser()?->getUID(); if ($loginToken !== null && $uid !== null) { - $this->userConfig->deleteUserConfig($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 70ebb77e1aaa6..bd7fa9a736862 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1331,6 +1331,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', @@ -1739,6 +1741,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 75746428ed294..4ebf27330b137 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1372,6 +1372,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', @@ -1780,6 +1782,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 ed0ba5c04f741..5546d78fa7be6 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; @@ -444,8 +445,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( @@ -458,6 +461,7 @@ public function __construct( $c->get(ILockdownManager::class), $c->get(LoggerInterface::class), $c->get(IEventDispatcher::class), + $rememberLoginTokenMapper, ); $dispatcher = $this->get(IEventDispatcher::class); $dispatcher->addListener(UserLoggedInEvent::class, function (UserLoggedInEvent $event): 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 3bbefd7199fa1..e8ca0bc4fe3af 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; @@ -79,6 +82,7 @@ public function __construct( private ILockdownManager $lockdownManager, private LoggerInterface $logger, private IEventDispatcher $dispatcher, + private ?RememberLoginTokenMapper $rememberLoginTokenMapper, ) { } @@ -904,11 +908,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, ]); @@ -935,9 +950,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, @@ -989,11 +1003,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 ea986861dd110..d210d69bcae11 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -14,6 +14,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; @@ -56,6 +57,9 @@ class LoginControllerTest extends TestCase { private IAppManager&MockObject $appManager; private AlternativeLoginService&MockObject $alternativeLoginService; + /** @var RememberLoginTokenMapper|MockObject */ + private $rememberLoginTokenMapper; + #[\Override] protected function setUp(): void { parent::setUp(); @@ -75,6 +79,7 @@ protected function setUp(): void { $this->l = $this->createMock(IL10N::class); $this->appManager = $this->createMock(IAppManager::class); $this->alternativeLoginService = $this->createMock(AlternativeLoginService::class); + $this->rememberLoginTokenMapper = $this->createMock(RememberLoginTokenMapper::class); $this->l->expects($this->any()) ->method('t') @@ -110,6 +115,7 @@ protected function setUp(): void { $this->l, $this->appManager, $this->alternativeLoginService, + $this->rememberLoginTokenMapper, ); } @@ -126,9 +132,9 @@ public function testLogoutWithoutToken(): void { ->expects($this->once()) ->method('isUserAgent') ->willReturn(false); - $this->userConfig + $this->rememberLoginTokenMapper ->expects($this->never()) - ->method('deleteUserConfig'); + ->method('deleteByToken'); $this->urlGenerator ->expects($this->once()) ->method('linkToRouteAbsolute') @@ -185,10 +191,10 @@ public function testLogoutWithToken(): void { ->expects($this->once()) ->method('getUser') ->willReturn($user); - $this->userConfig + $this->rememberLoginTokenMapper ->expects($this->once()) - ->method('deleteUserConfig') - ->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 20348f63cde34..0e95d804884c1 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.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\PublicKeyToken; use OC\Security\CSRF\CsrfTokenManager; @@ -21,6 +23,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\Authentication\Exceptions\InvalidTokenException; use OCP\Authentication\Token\IToken; @@ -60,6 +63,7 @@ class SessionTest extends TestCase { private ILockdownManager&MockObject $lockdownManager; private LoggerInterface&MockObject $logger; private IEventDispatcher&MockObject $dispatcher; + private RememberLoginTokenMapper&MockObject $rememberLoginTokenMapper; #[\Override] protected function setUp(): void { @@ -81,6 +85,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, @@ -92,6 +97,7 @@ protected function setUp(): void { $this->lockdownManager, $this->logger, $this->dispatcher, + $this->rememberLoginTokenMapper, ]) ->onlyMethods([ 'setMagicInCookie', @@ -115,7 +121,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' ]) @@ -140,7 +146,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); } @@ -187,7 +193,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' ]) @@ -250,7 +256,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'); } @@ -268,7 +274,7 @@ public function testLoginInvalidPassword(): 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); @@ -311,7 +317,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'); @@ -355,7 +361,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'); @@ -388,7 +394,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'); @@ -415,7 +421,7 @@ public function testLogClientInNoTokenPasswordWith2fa(): void { $request = $this->createMock(IRequest::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(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -473,7 +479,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(); @@ -498,7 +504,7 @@ public function testLogClientInWithTokenPassword(): void { $request = $this->createMock(IRequest::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(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -539,7 +545,7 @@ public function testLogClientInNoTokenPasswordNo2fa(): void { $request = $this->createMock(IRequest::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(['login', 'isTwoFactorEnforced']) ->getMock(); @@ -753,7 +759,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); @@ -767,20 +773,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()) @@ -849,7 +865,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); @@ -863,15 +879,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') @@ -923,7 +945,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); @@ -936,13 +958,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'); @@ -976,7 +1001,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'; @@ -987,10 +1012,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'); @@ -1024,7 +1047,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' ]) @@ -1044,7 +1067,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); @@ -1085,7 +1108,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); @@ -1129,7 +1152,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); @@ -1176,7 +1199,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'; @@ -1202,10 +1225,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') @@ -1271,7 +1298,8 @@ public function testTryBasicAuthLoginValid(): void { $this->random, $this->lockdownManager, $this->logger, - $this->dispatcher + $this->dispatcher, + $this->rememberLoginTokenMapper, ]) ->onlyMethods([ 'logClientIn', @@ -1322,7 +1350,8 @@ public function testTryBasicAuthLoginNoLogin(): void { $this->random, $this->lockdownManager, $this->logger, - $this->dispatcher + $this->dispatcher, + $this->rememberLoginTokenMapper, ]) ->onlyMethods([ 'logClientIn', @@ -1349,7 +1378,7 @@ public function testLogClientInThrottlerUsername(): void { $request = $this->createMock(IRequest::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(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); @@ -1408,7 +1437,7 @@ public function testLogClientInThrottlerEmail(): void { $request = $this->createMock(IRequest::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(['login', 'supportsCookies', 'createSessionToken', 'getUser']) ->getMock(); From e2e44f5edfb8adcc0b1f03b0651c159378f2875f Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Mon, 14 Sep 2026 14:48:13 -0300 Subject: [PATCH 2/5] 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 76a517d51f344b187654ad74d78f0cbb9103f295 Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Mon, 14 Sep 2026 14:50:26 -0300 Subject: [PATCH 3/5] 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 e8ca0bc4fe3af..8ff065b364847 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -922,7 +922,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, @@ -950,8 +950,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, @@ -1013,9 +1013,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; @@ -1030,13 +1029,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 0e95d804884c1..e4e1bf1aa59c3 100644 --- a/tests/lib/User/SessionTest.php +++ b/tests/lib/User/SessionTest.php @@ -775,28 +775,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()) @@ -881,19 +873,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') @@ -1229,9 +1217,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 55e716d6adefcd2231e5edcd1970954d4b65c5bb Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Wed, 16 Sep 2026 09:26:54 -0300 Subject: [PATCH 4/5] fix(remember-login-token): avoid unecessary insert when migrating from old table Signed-off-by: Cristian Scheid --- core/Controller/LoginController.php | 2 +- lib/private/User/Session.php | 53 ++++++++----------- tests/Core/Controller/LoginControllerTest.php | 3 +- 3 files changed, 25 insertions(+), 33 deletions(-) diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php index 67c00fb1b1af5..e7e53a279e9b7 100644 --- a/core/Controller/LoginController.php +++ b/core/Controller/LoginController.php @@ -87,7 +87,7 @@ public function logout(): RedirectResponse { $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->userConfig->deleteUserConfig($uid, 'login_token', $loginToken); } } $this->userSession->logout(); diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 8ff065b364847..e93f4d3289c43 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -908,13 +908,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, @@ -922,13 +934,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); @@ -949,9 +954,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, @@ -1020,24 +1031,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 d210d69bcae11..fc870e45c7ff6 100644 --- a/tests/Core/Controller/LoginControllerTest.php +++ b/tests/Core/Controller/LoginControllerTest.php @@ -57,8 +57,7 @@ class LoginControllerTest extends TestCase { private IAppManager&MockObject $appManager; private AlternativeLoginService&MockObject $alternativeLoginService; - /** @var RememberLoginTokenMapper|MockObject */ - private $rememberLoginTokenMapper; + private RememberLoginTokenMapper&MockObject $rememberLoginTokenMapper; #[\Override] protected function setUp(): void { From 9c2d5982a13399adfbb39debeecb3373558f26ec Mon Sep 17 00:00:00 2001 From: Cristian Scheid Date: Wed, 23 Sep 2026 14:34:33 +0200 Subject: [PATCH 5/5] fix(session): guard usages of RememberLoginTokenMapper Signed-off-by: Cristian Scheid --- lib/private/User/Session.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index e93f4d3289c43..2f835264d2b2f 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -86,6 +86,16 @@ public function __construct( ) { } + /** + * @throws \Exception + */ + private function getRememberLoginTokenMapper(): RememberLoginTokenMapper { + if ($this->rememberLoginTokenMapper === null) { + throw new \Exception('Remember login token mapper is not available'); + } + return $this->rememberLoginTokenMapper; + } + /** * @param IProvider $provider */ @@ -911,7 +921,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { $isLegacyRememberLoginToken = false; try { // get stored token - $rememberLoginToken = $this->rememberLoginTokenMapper->findByToken($currentToken); + $rememberLoginToken = $this->getRememberLoginTokenMapper()->findByToken($currentToken); if ($rememberLoginToken->uid !== $uid) { $this->logger->warning('Tried to login using remember-me token token from a different user', [ 'app' => 'core', @@ -961,7 +971,7 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) { } else { // replace successfully used token with a new one $newToken = $this->random->generate(32); - $this->rememberLoginTokenMapper->rotateToken($currentToken, $newToken); + $this->getRememberLoginTokenMapper()->rotateToken($currentToken, $newToken); } $this->logger->debug('Remember-me token replaced', [ 'app' => 'core', @@ -1026,7 +1036,7 @@ private function createRememberLoginToken(string $uid): string { $rememberLoginToken = new RememberLoginToken(); $rememberLoginToken->uid = $uid; $rememberLoginToken->token = $token; - $this->rememberLoginTokenMapper->insert($rememberLoginToken); + $this->getRememberLoginTokenMapper()->insert($rememberLoginToken); return $token; }