From ed0ff4788db972eeebec67f46f46f4f4fd2e1e55 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 Signed-off-by: Frank Karlitschek --- 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 4467e4e6179c2..8d206f80bd17f 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -924,6 +924,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', @@ -2360,6 +2361,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 dac59af04087a..5364325d18832 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -965,6 +965,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', @@ -2401,6 +2402,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 f1dff3b0693df..ccb69ae2923d6 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -1037,6 +1037,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(); + } +}