From 612856509eb0453db3b5a593dea13391044d8c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 21 May 2026 15:30:31 +0200 Subject: [PATCH 1/2] fix: Use token expiration for ephemeral sessions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This simplifies the code a lot. Signed-off-by: Côme Chilliet --- .../ClientFlowLoginV2Controller.php | 2 - lib/composer/composer/autoload_classmap.php | 2 - lib/composer/composer/autoload_static.php | 2 - .../DependencyInjection/DIContainer.php | 3 - .../FlowV2EphemeralSessionsMiddleware.php | 75 ------------------- lib/private/Authentication/Login/Chain.php | 2 - .../Login/CreateSessionTokenCommand.php | 18 ++++- .../Login/FlowV2EphemeralSessionsCommand.php | 33 -------- .../Authentication/Token/IProvider.php | 1 + lib/private/Authentication/Token/Manager.php | 2 + .../Token/PublicKeyTokenProvider.php | 12 ++- lib/private/User/Session.php | 20 ++--- 12 files changed, 38 insertions(+), 134 deletions(-) delete mode 100644 lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php delete mode 100644 lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php index 85b82b388f412..d54d175193e9f 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -46,8 +46,6 @@ class ClientFlowLoginV2Controller extends Controller { public const TOKEN_NAME = 'client.flow.v2.login.token'; public const STATE_NAME = 'client.flow.v2.state.token'; - // Denotes that the session was created for the login flow and should therefore be ephemeral. - public const EPHEMERAL_NAME = 'client.flow.v2.state.ephemeral'; public function __construct( string $appName, diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 008a172f819c2..a487722185721 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1114,7 +1114,6 @@ 'OC\\AppFramework\\Http\\RequestId' => $baseDir . '/lib/private/AppFramework/Http/RequestId.php', 'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php', 'OC\\AppFramework\\Middleware\\CompressionMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php', - 'OC\\AppFramework\\Middleware\\FlowV2EphemeralSessionsMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php', 'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => $baseDir . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php', 'OC\\AppFramework\\Middleware\\MiddlewareUtils' => $baseDir . '/lib/private/AppFramework/Middleware/MiddlewareUtils.php', 'OC\\AppFramework\\Middleware\\NotModifiedMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php', @@ -1213,7 +1212,6 @@ 'OC\\Authentication\\Login\\CompleteLoginCommand' => $baseDir . '/lib/private/Authentication/Login/CompleteLoginCommand.php', 'OC\\Authentication\\Login\\CreateSessionTokenCommand' => $baseDir . '/lib/private/Authentication/Login/CreateSessionTokenCommand.php', 'OC\\Authentication\\Login\\FinishRememberedLoginCommand' => $baseDir . '/lib/private/Authentication/Login/FinishRememberedLoginCommand.php', - 'OC\\Authentication\\Login\\FlowV2EphemeralSessionsCommand' => $baseDir . '/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php', 'OC\\Authentication\\Login\\LoggedInCheckCommand' => $baseDir . '/lib/private/Authentication/Login/LoggedInCheckCommand.php', 'OC\\Authentication\\Login\\LoginData' => $baseDir . '/lib/private/Authentication/Login/LoginData.php', 'OC\\Authentication\\Login\\LoginResult' => $baseDir . '/lib/private/Authentication/Login/LoginResult.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index c92c28f5ec8ee..52f593153133e 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1155,7 +1155,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Http\\RequestId' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Http/RequestId.php', 'OC\\AppFramework\\Middleware\\AdditionalScriptsMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/AdditionalScriptsMiddleware.php', 'OC\\AppFramework\\Middleware\\CompressionMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/CompressionMiddleware.php', - 'OC\\AppFramework\\Middleware\\FlowV2EphemeralSessionsMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php', 'OC\\AppFramework\\Middleware\\MiddlewareDispatcher' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/MiddlewareDispatcher.php', 'OC\\AppFramework\\Middleware\\MiddlewareUtils' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/MiddlewareUtils.php', 'OC\\AppFramework\\Middleware\\NotModifiedMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php', @@ -1254,7 +1253,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Authentication\\Login\\CompleteLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/CompleteLoginCommand.php', 'OC\\Authentication\\Login\\CreateSessionTokenCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/CreateSessionTokenCommand.php', 'OC\\Authentication\\Login\\FinishRememberedLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/FinishRememberedLoginCommand.php', - 'OC\\Authentication\\Login\\FlowV2EphemeralSessionsCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php', 'OC\\Authentication\\Login\\LoggedInCheckCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/LoggedInCheckCommand.php', 'OC\\Authentication\\Login\\LoginData' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/LoginData.php', 'OC\\Authentication\\Login\\LoginResult' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/LoginResult.php', diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index d2f33ece04aec..a4f9da30d2cd6 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -17,7 +17,6 @@ use OC\AppFramework\Http\Output; use OC\AppFramework\Middleware\AdditionalScriptsMiddleware; use OC\AppFramework\Middleware\CompressionMiddleware; -use OC\AppFramework\Middleware\FlowV2EphemeralSessionsMiddleware; use OC\AppFramework\Middleware\MiddlewareDispatcher; use OC\AppFramework\Middleware\MiddlewareUtils; use OC\AppFramework\Middleware\NotModifiedMiddleware; @@ -207,8 +206,6 @@ public function __construct( $dispatcher->registerMiddleware($c->get(CORSMiddleware::class)); $dispatcher->registerMiddleware($c->get(OCSMiddleware::class)); - $dispatcher->registerMiddleware($c->get(FlowV2EphemeralSessionsMiddleware::class)); - $securityMiddleware = new SecurityMiddleware( $c->get(IRequest::class), $c->get(MiddlewareUtils::class), diff --git a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php deleted file mode 100644 index 56f5babcc6fc8..0000000000000 --- a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php +++ /dev/null @@ -1,75 +0,0 @@ -session->get(ClientFlowLoginV2Controller::EPHEMERAL_NAME); - - // Not an ephemeral session. - if ($sessionCreationTime === null) { - return; - } - - // Lax enforcement until TTL is reached. - if ($this->timeFactory->getTime() < $sessionCreationTime + self::EPHEMERAL_SESSION_TTL) { - return; - } - - // Allow certain controllers/methods to proceed without logging out. - if ( - $controller instanceof ClientFlowLoginV2Controller - && ($methodName === 'grantPage' || $methodName === 'generateAppPassword') - ) { - return; - } - - if ($controller instanceof TwoFactorChallengeController - || $controller instanceof ALoginSetupController) { - return; - } - - if ($this->reflector->hasAnnotationOrAttribute('PublicPage', PublicPage::class)) { - return; - } - - $this->logger->info('Closing user and PHP session for ephemeral session', [ - 'controller' => $controller::class, - 'method' => $methodName, - ]); - $this->userSession->logout(); - $this->session->close(); - } -} diff --git a/lib/private/Authentication/Login/Chain.php b/lib/private/Authentication/Login/Chain.php index fc90d9225a7cd..baf59195b971f 100644 --- a/lib/private/Authentication/Login/Chain.php +++ b/lib/private/Authentication/Login/Chain.php @@ -21,7 +21,6 @@ public function __construct( private SetUserTimezoneCommand $setUserTimezoneCommand, private TwoFactorCommand $twoFactorCommand, private FinishRememberedLoginCommand $finishRememberedLoginCommand, - private FlowV2EphemeralSessionsCommand $flowV2EphemeralSessionsCommand, ) { } @@ -32,7 +31,6 @@ public function process(LoginData $loginData): LoginResult { ->setNext($this->uidLoginCommand) ->setNext($this->loggedInCheckCommand) ->setNext($this->completeLoginCommand) - ->setNext($this->flowV2EphemeralSessionsCommand) ->setNext($this->createSessionTokenCommand) ->setNext($this->clearLostPasswordTokensCommand) ->setNext($this->updateLastPasswordConfirmCommand) diff --git a/lib/private/Authentication/Login/CreateSessionTokenCommand.php b/lib/private/Authentication/Login/CreateSessionTokenCommand.php index 8ad098cebe11e..4c9f2941eadda 100644 --- a/lib/private/Authentication/Login/CreateSessionTokenCommand.php +++ b/lib/private/Authentication/Login/CreateSessionTokenCommand.php @@ -10,12 +10,18 @@ use OC\Authentication\Token\IToken; use OC\User\Session; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IConfig; +use OCP\IURLGenerator; class CreateSessionTokenCommand extends ALoginCommand { + private const EPHEMERAL_SESSION_TTL = 5 * 60; // 5 minutes + public function __construct( private IConfig $config, private Session $userSession, + private IURLGenerator $urlGenerator, + private ITimeFactory $timeFactory, ) { } @@ -30,13 +36,20 @@ public function process(LoginData $loginData): LoginResult { $tokenType = IToken::DO_NOT_REMEMBER; } + $loginV2GrantRoute = $this->urlGenerator->linkToRoute('core.ClientFlowLoginV2.grantPage'); + $expires = null; + if (str_starts_with($loginData->getRedirectUrl() ?? '', $loginV2GrantRoute)) { + $expires = $this->timeFactory->getTime() + self::EPHEMERAL_SESSION_TTL; + } + if ($loginData->getPassword() === '') { $this->userSession->createSessionToken( $loginData->getRequest(), $loginData->getUser()->getUID(), $loginData->getUsername(), null, - $tokenType + $tokenType, + $expires, ); $this->userSession->updateTokens( $loginData->getUser()->getUID(), @@ -48,7 +61,8 @@ public function process(LoginData $loginData): LoginResult { $loginData->getUser()->getUID(), $loginData->getUsername(), $loginData->getPassword(), - $tokenType + $tokenType, + $expires, ); $this->userSession->updateTokens( $loginData->getUser()->getUID(), diff --git a/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php b/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php deleted file mode 100644 index 40b78229306d1..0000000000000 --- a/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php +++ /dev/null @@ -1,33 +0,0 @@ -urlGenerator->linkToRoute('core.ClientFlowLoginV2.grantPage'); - if (str_starts_with($loginData->getRedirectUrl() ?? '', $loginV2GrantRoute)) { - $this->session->set(ClientFlowLoginV2Controller::EPHEMERAL_NAME, $this->timeFactory->getTime()); - } - - return $this->processNextOrFinishSuccessfully($loginData); - } -} diff --git a/lib/private/Authentication/Token/IProvider.php b/lib/private/Authentication/Token/IProvider.php index d47427e79bf06..04a24bc559e9f 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -37,6 +37,7 @@ public function generateToken(string $token, int $type = OCPIToken::TEMPORARY_TOKEN, int $remember = OCPIToken::DO_NOT_REMEMBER, ?array $scope = null, + ?int $expires = null, ): OCPIToken; /** diff --git a/lib/private/Authentication/Token/Manager.php b/lib/private/Authentication/Token/Manager.php index 836484dbf7f06..62af5debadbf9 100644 --- a/lib/private/Authentication/Token/Manager.php +++ b/lib/private/Authentication/Token/Manager.php @@ -43,6 +43,7 @@ public function generateToken(string $token, int $type = OCPIToken::TEMPORARY_TOKEN, int $remember = OCPIToken::DO_NOT_REMEMBER, ?array $scope = null, + ?int $expires = null, ): OCPIToken { if (mb_strlen($name) > 128) { $name = mb_substr($name, 0, 120) . '…'; @@ -58,6 +59,7 @@ public function generateToken(string $token, $type, $remember, $scope, + $expires, ); } catch (Exception $e) { if ($e->getReason() !== Exception::REASON_UNIQUE_CONSTRAINT_VIOLATION) { diff --git a/lib/private/Authentication/Token/PublicKeyTokenProvider.php b/lib/private/Authentication/Token/PublicKeyTokenProvider.php index a43d4a7f88819..f6d44e577ffea 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -65,6 +65,7 @@ public function generateToken(string $token, int $type = OCPIToken::TEMPORARY_TOKEN, int $remember = OCPIToken::DO_NOT_REMEMBER, ?array $scope = null, + ?int $expires = null, ): OCPIToken { if (strlen($token) < self::TOKEN_MIN_LENGTH) { $exception = new InvalidTokenException('Token is too short, minimum of ' . self::TOKEN_MIN_LENGTH . ' characters is required, ' . strlen($token) . ' characters given'); @@ -81,7 +82,7 @@ public function generateToken(string $token, $randomOldToken = $this->mapper->getFirstTokenForUser($uid); $oldTokenMatches = $randomOldToken && $randomOldToken->getPasswordHash() && $password !== null && $this->hasher->verify(sha1($password) . $password, $randomOldToken->getPasswordHash()); - $dbToken = $this->newToken($token, $uid, $loginName, $password, $name, $type, $remember); + $dbToken = $this->newToken($token, $uid, $loginName, $password, $name, $type, $remember, $expires); if ($oldTokenMatches) { $dbToken->setPasswordHash($randomOldToken->getPasswordHash()); @@ -232,6 +233,7 @@ public function renewSessionToken(string $oldSessionId, string $sessionId): OCPI OCPIToken::TEMPORARY_TOKEN, $token->getRemember(), $scope, + $token->getExpires(), ); $this->cacheToken($newToken); @@ -444,7 +446,9 @@ private function newToken(string $token, $password, string $name, int $type, - int $remember): PublicKeyToken { + int $remember, + ?int $expires, + ): PublicKeyToken { $dbToken = new PublicKeyToken(); $dbToken->setUid($uid); $dbToken->setLoginName($loginName); @@ -489,7 +493,9 @@ private function newToken(string $token, $dbToken->setLastCheck($this->time->getTime()); $dbToken->setVersion(PublicKeyToken::VERSION); - if ($type === OCPIToken::ONETIME_TOKEN) { + if ($expires !== null) { + $dbToken->setExpires($expires); + } elseif ($type === OCPIToken::ONETIME_TOKEN) { // Minimum duration is 2 minutes as shown in the UI $expirationDuration = max(120, $this->config->getSystemValueInt('auth_onetime_token_validity', 120)); $dbToken->setExpires($this->time->getTime() + $expirationDuration); diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 8154178e455dd..dc02e6b1fdbe7 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -637,15 +637,15 @@ private function loginWithToken($token) { /** * Create a new session token for the given user credentials - * - * @param IRequest $request - * @param string $uid user UID - * @param string $loginName login name - * @param string $password - * @param int $remember - * @return boolean */ - public function createSessionToken(IRequest $request, $uid, $loginName, $password = null, $remember = IToken::DO_NOT_REMEMBER) { + public function createSessionToken( + IRequest $request, + string $uid, + string $loginName, + ?string $password = null, + int $remember = IToken::DO_NOT_REMEMBER, + ?int $expires = null, + ): bool { if (is_null($this->manager->get($uid))) { // User does not exist return false; @@ -655,9 +655,9 @@ public function createSessionToken(IRequest $request, $uid, $loginName, $passwor $sessionId = $this->session->getId(); $pwd = $this->getPassword($password); // Make sure the current sessionId has no leftover tokens - $this->atomic(function () use ($sessionId, $uid, $loginName, $pwd, $name, $remember): void { + $this->atomic(function () use ($sessionId, $uid, $loginName, $pwd, $name, $remember, $expires): void { $this->tokenProvider->invalidateToken($sessionId); - $this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember); + $this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember, expires:$expires); }, Server::get(IDBConnection::class)); return true; } catch (SessionNotAvailableException $ex) { From d29d978900d479f0e913f54dd210894d5b945253 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 21 May 2026 16:24:48 +0200 Subject: [PATCH 2/2] chore: Fix CreateSessionTokenCommandTest and add test for ephemeral session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../Login/CreateSessionTokenCommandTest.php | 77 ++++++++++++++++--- 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/tests/lib/Authentication/Login/CreateSessionTokenCommandTest.php b/tests/lib/Authentication/Login/CreateSessionTokenCommandTest.php index 2728fc254791a..b2c94d105958a 100644 --- a/tests/lib/Authentication/Login/CreateSessionTokenCommandTest.php +++ b/tests/lib/Authentication/Login/CreateSessionTokenCommandTest.php @@ -1,26 +1,27 @@ config = $this->createMock(IConfig::class); $this->userSession = $this->createMock(Session::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->timeFactory = $this->createMock(ITimeFactory::class); $this->cmd = new CreateSessionTokenCommand( $this->config, - $this->userSession + $this->userSession, + $this->urlGenerator, + $this->timeFactory, ); } public function testProcess(): void { + // Just return the route name as path to not return an empty string + $this->urlGenerator->expects(self::once()) + ->method('linkToRoute') + ->willReturnArgument(0); $data = $this->getLoggedInLoginData(); $this->config->expects($this->once()) ->method('getSystemValueInt') @@ -54,7 +63,8 @@ public function testProcess(): void { $this->username, $this->username, $this->password, - IToken::REMEMBER + IToken::REMEMBER, + null ); $this->userSession->expects($this->once()) ->method('updateTokens') @@ -69,6 +79,10 @@ public function testProcess(): void { } public function testProcessDoNotRemember(): void { + // Just return the route name as path to not return an empty string + $this->urlGenerator->expects(self::once()) + ->method('linkToRoute') + ->willReturnArgument(0); $data = $this->getLoggedInLoginData(); $this->config->expects($this->once()) ->method('getSystemValueInt') @@ -87,7 +101,8 @@ public function testProcessDoNotRemember(): void { $this->username, $this->username, $this->password, - IToken::DO_NOT_REMEMBER + IToken::DO_NOT_REMEMBER, + null ); $this->userSession->expects($this->once()) ->method('updateTokens') @@ -101,4 +116,46 @@ public function testProcessDoNotRemember(): void { $this->assertTrue($result->isSuccess()); $this->assertFalse($data->isRememberLogin()); } + + public function testLoginFlowEphemeral(): void { + $this->redirectUrl = 'EPHEMERAL_ROUTE'; + $this->urlGenerator->expects(self::once()) + ->method('linkToRoute') + ->willReturn($this->redirectUrl); + $this->timeFactory->expects(self::once()) + ->method('getTime') + ->willReturn(1000); + + $data = $this->getLoggedInLoginDataWithRedirectUrl(); + $this->config->expects($this->once()) + ->method('getSystemValueInt') + ->with( + 'remember_login_cookie_lifetime', + 60 * 60 * 24 * 15 + ) + ->willReturn(100); + $this->user->expects($this->any()) + ->method('getUID') + ->willReturn($this->username); + $this->userSession->expects($this->once()) + ->method('createSessionToken') + ->with( + $this->request, + $this->username, + $this->username, + $this->password, + IToken::REMEMBER, + 1000 + 5 * 60 + ); + $this->userSession->expects($this->once()) + ->method('updateTokens') + ->with( + $this->username, + $this->password + ); + + $result = $this->cmd->process($data); + + $this->assertTrue($result->isSuccess()); + } }