diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php index f53f247a03c4b..1274b5c41b565 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -47,8 +47,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 159d2b3abbf96..f1b053c3407f8 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1108,7 +1108,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', @@ -1205,7 +1204,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 2376057d7c3f4..a133c7c8b8c12 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1149,7 +1149,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', @@ -1246,7 +1245,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 6bb02f6b51a32..3d599ea5212c6 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; @@ -205,8 +204,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 c4db6733822ee..0000000000000 --- a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php +++ /dev/null @@ -1,76 +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 530112b516164..50be4ecb56c1a 100644 --- a/lib/private/Authentication/Login/Chain.php +++ b/lib/private/Authentication/Login/Chain.php @@ -22,7 +22,6 @@ public function __construct( private SetUserTimezoneCommand $setUserTimezoneCommand, private TwoFactorCommand $twoFactorCommand, private FinishRememberedLoginCommand $finishRememberedLoginCommand, - private FlowV2EphemeralSessionsCommand $flowV2EphemeralSessionsCommand, ) { } @@ -33,7 +32,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 63f906b52ba45..03d6014bfbb08 100644 --- a/lib/private/Authentication/Login/CreateSessionTokenCommand.php +++ b/lib/private/Authentication/Login/CreateSessionTokenCommand.php @@ -11,12 +11,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, ) { } @@ -31,13 +37,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(), @@ -49,7 +62,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 6aaa57d0fd0f9..0000000000000 --- a/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php +++ /dev/null @@ -1,34 +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 542aeb118a85d..39209709c1581 100644 --- a/lib/private/Authentication/Token/IProvider.php +++ b/lib/private/Authentication/Token/IProvider.php @@ -38,6 +38,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 4643837737639..21e6ce810e452 100644 --- a/lib/private/Authentication/Token/Manager.php +++ b/lib/private/Authentication/Token/Manager.php @@ -44,6 +44,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) . '…'; @@ -59,6 +60,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 9f0d4ccbe9662..f3328a388dbe7 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -66,6 +66,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'); @@ -82,7 +83,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()); @@ -233,6 +234,7 @@ public function renewSessionToken(string $oldSessionId, string $sessionId): OCPI OCPIToken::TEMPORARY_TOKEN, $token->getRemember(), $scope, + $token->getExpires(), ); $this->cacheToken($newToken); @@ -445,7 +447,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); @@ -490,7 +494,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 0ccd5f921ffc2..8ebc4f9f7928a 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -643,15 +643,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; @@ -661,9 +661,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) { 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()); + } }