From 5f80b53f22661c239360ed396860dd8899eab669 Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Fri, 23 Oct 2026 19:12:13 +0200 Subject: [PATCH] feat(session): allow apps to renew the session id while keeping the login's token Add ISessionIdRenewer. It gives the current session a new id, e.g. after the user proved their identity once more in this session. Unlike ISession::regenerateId() with $updateToken, the auth token is not replaced by a new one: the same token row, with its id, name and stored password, is moved to the new session id. Anything that refers to the token by its id, such as the pending second-factor marker or an app's own records, stays valid. A remember-me cookie that points to the old session id follows the new one, and the old id no longer finds the token, also not from the token cache. Assisted-by: ClaudeCode:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- lib/composer/composer/autoload_classmap.php | 2 + lib/composer/composer/autoload_static.php | 2 + lib/private/Server.php | 1 + lib/private/Session/SessionIdRenewer.php | 44 ++++++ lib/public/Session/ISessionIdRenewer.php | 34 +++++ tests/lib/Session/SessionIdRenewerTest.php | 148 ++++++++++++++++++++ 6 files changed, 231 insertions(+) create mode 100644 lib/private/Session/SessionIdRenewer.php create mode 100644 lib/public/Session/ISessionIdRenewer.php create mode 100644 tests/lib/Session/SessionIdRenewerTest.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index aaf5602ba47e4..3e57d0bfb9cc6 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -922,6 +922,7 @@ 'OCP\\Server' => $baseDir . '/lib/public/Server.php', 'OCP\\ServerVersion' => $baseDir . '/lib/public/ServerVersion.php', 'OCP\\Session\\Exceptions\\SessionNotAvailableException' => $baseDir . '/lib/public/Session/Exceptions/SessionNotAvailableException.php', + 'OCP\\Session\\ISessionIdRenewer' => $baseDir . '/lib/public/Session/ISessionIdRenewer.php', 'OCP\\Settings\\DeclarativeSettingsTypes' => $baseDir . '/lib/public/Settings/DeclarativeSettingsTypes.php', 'OCP\\Settings\\Events\\DeclarativeSettingsGetValueEvent' => $baseDir . '/lib/public/Settings/Events/DeclarativeSettingsGetValueEvent.php', 'OCP\\Settings\\Events\\DeclarativeSettingsRegisterFormEvent' => $baseDir . '/lib/public/Settings/Events/DeclarativeSettingsRegisterFormEvent.php', @@ -2357,6 +2358,7 @@ 'OC\\Session\\Internal' => $baseDir . '/lib/private/Session/Internal.php', 'OC\\Session\\Memory' => $baseDir . '/lib/private/Session/Memory.php', 'OC\\Session\\Session' => $baseDir . '/lib/private/Session/Session.php', + 'OC\\Session\\SessionIdRenewer' => $baseDir . '/lib/private/Session/SessionIdRenewer.php', 'OC\\Settings\\AuthorizedGroup' => $baseDir . '/lib/private/Settings/AuthorizedGroup.php', 'OC\\Settings\\AuthorizedGroupMapper' => $baseDir . '/lib/private/Settings/AuthorizedGroupMapper.php', 'OC\\Settings\\DeclarativeManager' => $baseDir . '/lib/private/Settings/DeclarativeManager.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index d07e0aaa6b3bd..32aaf716ce4ef 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -963,6 +963,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\Server' => __DIR__ . '/../../..' . '/lib/public/Server.php', 'OCP\\ServerVersion' => __DIR__ . '/../../..' . '/lib/public/ServerVersion.php', 'OCP\\Session\\Exceptions\\SessionNotAvailableException' => __DIR__ . '/../../..' . '/lib/public/Session/Exceptions/SessionNotAvailableException.php', + 'OCP\\Session\\ISessionIdRenewer' => __DIR__ . '/../../..' . '/lib/public/Session/ISessionIdRenewer.php', 'OCP\\Settings\\DeclarativeSettingsTypes' => __DIR__ . '/../../..' . '/lib/public/Settings/DeclarativeSettingsTypes.php', 'OCP\\Settings\\Events\\DeclarativeSettingsGetValueEvent' => __DIR__ . '/../../..' . '/lib/public/Settings/Events/DeclarativeSettingsGetValueEvent.php', 'OCP\\Settings\\Events\\DeclarativeSettingsRegisterFormEvent' => __DIR__ . '/../../..' . '/lib/public/Settings/Events/DeclarativeSettingsRegisterFormEvent.php', @@ -2398,6 +2399,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Session\\Internal' => __DIR__ . '/../../..' . '/lib/private/Session/Internal.php', 'OC\\Session\\Memory' => __DIR__ . '/../../..' . '/lib/private/Session/Memory.php', 'OC\\Session\\Session' => __DIR__ . '/../../..' . '/lib/private/Session/Session.php', + 'OC\\Session\\SessionIdRenewer' => __DIR__ . '/../../..' . '/lib/private/Session/SessionIdRenewer.php', 'OC\\Settings\\AuthorizedGroup' => __DIR__ . '/../../..' . '/lib/private/Settings/AuthorizedGroup.php', 'OC\\Settings\\AuthorizedGroupMapper' => __DIR__ . '/../../..' . '/lib/private/Settings/AuthorizedGroupMapper.php', 'OC\\Settings\\DeclarativeManager' => __DIR__ . '/../../..' . '/lib/private/Settings/DeclarativeManager.php', diff --git a/lib/private/Server.php b/lib/private/Server.php index 439974df99762..be395e89afe29 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1036,6 +1036,7 @@ public function __construct( $session = $c->get(IUserSession::class); return $session->getSession(); }, false); + $this->registerAlias(\OCP\Session\ISessionIdRenewer::class, \OC\Session\SessionIdRenewer::class); $this->registerAlias(IShareHelper::class, ShareHelper::class); $this->registerAlias(IContactsStore::class, ContactsStore::class); diff --git a/lib/private/Session/SessionIdRenewer.php b/lib/private/Session/SessionIdRenewer.php new file mode 100644 index 0000000000000..1d9b241913a75 --- /dev/null +++ b/lib/private/Session/SessionIdRenewer.php @@ -0,0 +1,44 @@ +userSession->getSession(); + $oldId = $session->getId(); + try { + $token = $this->tokenProvider->getToken($oldId); + } catch (InvalidTokenException) { + $token = null; + } + + $session->regenerateId(true, false); + if ($token === null) { + return; + } + + $this->tokenProvider->rotate($token, $oldId, $session->getId()); + // No token has the old id any more; this drops it from the token cache. + $this->tokenProvider->invalidateToken($oldId); + $this->userSession->renewMagicSessionId($oldId); + } +} diff --git a/lib/public/Session/ISessionIdRenewer.php b/lib/public/Session/ISessionIdRenewer.php new file mode 100644 index 0000000000000..f50e6a2320937 --- /dev/null +++ b/lib/public/Session/ISessionIdRenewer.php @@ -0,0 +1,34 @@ +currentId = self::OLD_ID; + $this->session = $this->createMock(ISession::class); + $this->session->method('getId') + ->willReturnCallback(fn (): string => $this->currentId); + $this->userSession = $this->createMock(UserSession::class); + $this->userSession->method('getSession') + ->willReturn($this->session); + } + + #[\Override] + protected function tearDown(): void { + $provider = Server::get(PublicKeyTokenProvider::class); + $provider->invalidateToken(self::OLD_ID); + $provider->invalidateToken(self::NEW_ID); + parent::tearDown(); + } + + private function expectRegenerate(): void { + $this->session->expects($this->once()) + ->method('regenerateId') + ->with(true, false) + ->willReturnCallback(function (): void { + $this->currentId = self::NEW_ID; + }); + } + + public function testIsRegistered(): void { + $this->assertInstanceOf(SessionIdRenewer::class, Server::get(ISessionIdRenewer::class)); + } + + public function testKeepsTheTokenUnderTheNewId(): void { + $provider = Server::get(PublicKeyTokenProvider::class); + $token = $provider->generateToken(self::OLD_ID, 'renew-user', 'renew-login', 'secret password', 'Firefox', IToken::TEMPORARY_TOKEN, IToken::REMEMBER); + $this->expectRegenerate(); + $this->userSession->expects($this->once()) + ->method('renewMagicSessionId') + ->with(self::OLD_ID); + + (new SessionIdRenewer($this->userSession, $provider))->renewSessionId(); + + $renewed = $provider->getToken(self::NEW_ID); + $this->assertSame($token->getId(), $renewed->getId()); + $this->assertSame('Firefox', $renewed->getName()); + $this->assertSame(IToken::REMEMBER, $renewed->getRemember()); + $this->assertSame('secret password', $provider->getPassword($renewed, self::NEW_ID)); + $this->expectException(InvalidTokenException::class); + $provider->getToken(self::OLD_ID); + } + + public function testMovesTheTokenBeforeTheRememberMeCookie(): void { + $token = $this->createMock(PublicKeyToken::class); + $provider = $this->createMock(IProvider::class); + $provider->method('getToken') + ->with(self::OLD_ID) + ->willReturn($token); + $this->expectRegenerate(); + $calls = []; + $provider->expects($this->once()) + ->method('rotate') + ->with($token, self::OLD_ID, self::NEW_ID) + ->willReturnCallback(function () use (&$calls, $token): PublicKeyToken { + $calls[] = 'rotate'; + return $token; + }); + $provider->expects($this->once()) + ->method('invalidateToken') + ->with(self::OLD_ID) + ->willReturnCallback(function () use (&$calls): void { + $calls[] = 'forget'; + }); + $this->userSession->expects($this->once()) + ->method('renewMagicSessionId') + ->with(self::OLD_ID) + ->willReturnCallback(function () use (&$calls): void { + $calls[] = 'cookie'; + }); + + (new SessionIdRenewer($this->userSession, $provider))->renewSessionId(); + + $this->assertSame(['rotate', 'forget', 'cookie'], $calls); + } + + public function testRenewsTheIdOfASessionWithoutToken(): void { + $provider = $this->createMock(IProvider::class); + $provider->method('getToken') + ->willThrowException(new InvalidTokenException()); + $this->expectRegenerate(); + $provider->expects($this->never())->method('rotate'); + $provider->expects($this->never())->method('invalidateToken'); + $this->userSession->expects($this->never())->method('renewMagicSessionId'); + + (new SessionIdRenewer($this->userSession, $provider))->renewSessionId(); + + $this->assertSame(self::NEW_ID, $this->currentId); + } + + public function testWithoutSessionNothingChanges(): void { + $session = $this->createMock(ISession::class); + $session->method('getId') + ->willThrowException(new SessionNotAvailableException()); + $session->expects($this->never())->method('regenerateId'); + $userSession = $this->createMock(UserSession::class); + $userSession->method('getSession')->willReturn($session); + $provider = $this->createMock(IProvider::class); + $provider->expects($this->never())->method('rotate'); + + $this->expectException(SessionNotAvailableException::class); + (new SessionIdRenewer($userSession, $provider))->renewSessionId(); + } +}