From 0bbfe2e4d94e38b8e7d145e0e60769f7485f0aa8 Mon Sep 17 00:00:00 2001
From: Jonas <jonas@freesources.org>
Date: Tue, 29 Sep 2026 19:02:59 +0200
Subject: [PATCH] fix(session): only regenerate session id after valid
 remember-me cookie

loginWithCookie() regenerated the session id before validating the
remember-me cookie. With stale cookies (e.g. after the session token was
invalidated by an OIDC backchannel logout) every request rotated the
session and failed. Parallel requests then forked the same session, and
the browser could end up with a copy lacking data stored during the
login, such as user_oidc's OIDC state or the login flow v2 state token.

Regenerate the session id only once the cookie has been validated.

Signed-off-by: Jonas <jonas@freesources.org>
Assisted-by: ClaudeCode:claude-opus-5.5
---
 lib/private/User/Session.php   |  5 ++-
 tests/lib/User/SessionTest.php | 58 ++++++++++++++++++++++++++++++++--
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
index 1b075e1e6358c..eee520709d44a 100644
--- a/lib/private/User/Session.php
+++ b/lib/private/User/Session.php
@@ -879,7 +879,6 @@ public function tryTokenLogin(IRequest $request) {
 	 * @return bool
 	 */
 	public function loginWithCookie($uid, $currentToken, $oldSessionId) {
-		$this->session->regenerateId();
 		$this->manager->emit('\OC\User', 'preRememberedLogin', [$uid]);
 		$user = $this->manager->get($uid);
 		if (is_null($user)) {
@@ -917,6 +916,10 @@ public function loginWithCookie($uid, $currentToken, $oldSessionId) {
 			return false;
 		}
 
+		// Only rotate the session once the cookie is known to be valid; failed attempts must not
+		// fork the session, as concurrent requests would otherwise lose its data.
+		$this->session->regenerateId();
+
 		// replace successfully used token with a new one
 		$this->config->deleteUserValue($uid, 'login_token', $currentToken);
 		$newToken = $this->random->generate(32);
diff --git a/tests/lib/User/SessionTest.php b/tests/lib/User/SessionTest.php
index d0ec8d6191611..ffd39b2b7bd5a 100644
--- a/tests/lib/User/SessionTest.php
+++ b/tests/lib/User/SessionTest.php
@@ -839,6 +839,60 @@ public function testRememberLoginInvalidSessionToken(): void {
 		$this->assertFalse($granted);
 	}
 
+	public function testRememberLoginMissingSessionToken(): void {
+		$session = $this->createMock(Memory::class);
+		$managerMethods = get_class_methods(Manager::class);
+		//keep following methods intact in order to ensure hooks are working
+		$mockedManagerMethods = array_diff($managerMethods, ['__construct', 'emit', 'listen']);
+		$manager = $this->getMockBuilder(Manager::class)
+			->onlyMethods($mockedManagerMethods)
+			->setConstructorArgs([
+				$this->config,
+				$this->createMock(ICacheFactory::class),
+				$this->createMock(IEventDispatcher::class),
+				$this->createMock(LoggerInterface::class),
+			])
+			->getMock();
+		$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])
+			->getMock();
+
+		$user = $this->createMock(IUser::class);
+		$token = 'goodToken';
+		$oldSessionId = 'sess321';
+
+		$session->expects($this->never())
+			->method('regenerateId');
+		$manager->expects($this->once())
+			->method('get')
+			->with('foo')
+			->willReturn($user);
+		$this->config->expects($this->once())
+			->method('getUserKeys')
+			->with('foo', 'login_token')
+			->willReturn([$token]);
+		$this->tokenProvider->expects($this->once())
+			->method('getToken')
+			->with($oldSessionId)
+			->willThrowException(new InvalidTokenException());
+
+		$this->config->expects($this->never())
+			->method('deleteUserValue');
+		$this->tokenProvider->expects($this->never())
+			->method('renewSessionToken');
+		$userSession->expects($this->never())
+			->method('setMagicInCookie');
+		$session->expects($this->never())
+			->method('set')
+			->with('user_id', 'foo');
+
+		$granted = $userSession->loginWithCookie('foo', $token, $oldSessionId);
+
+		$this->assertFalse($granted);
+	}
+
 	public function testRememberLoginInvalidToken(): void {
 		$session = $this->createMock(Memory::class);
 		$managerMethods = get_class_methods(Manager::class);
@@ -863,7 +917,7 @@ public function testRememberLoginInvalidToken(): void {
 		$token = 'goodToken';
 		$oldSessionId = 'sess321';
 
-		$session->expects($this->once())
+		$session->expects($this->never())
 			->method('regenerateId');
 		$manager->expects($this->once())
 			->method('get')
@@ -914,7 +968,7 @@ public function testRememberLoginInvalidUser(): void {
 		$token = 'goodToken';
 		$oldSessionId = 'sess321';
 
-		$session->expects($this->once())
+		$session->expects($this->never())
 			->method('regenerateId');
 		$manager->expects($this->once())
 			->method('get')