diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php index 877831df502a5..0afae8163d1fe 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -42,8 +42,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 ba4a9339d938e..4c25c8113039d 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1002,7 +1002,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\\NotModifiedMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php', 'OC\\AppFramework\\Middleware\\OCSMiddleware' => $baseDir . '/lib/private/AppFramework/Middleware/OCSMiddleware.php', @@ -1100,7 +1099,6 @@ 'OC\\Authentication\\Login\\CreateSessionTokenCommand' => $baseDir . '/lib/private/Authentication/Login/CreateSessionTokenCommand.php', 'OC\\Authentication\\Login\\EmailLoginCommand' => $baseDir . '/lib/private/Authentication/Login/EmailLoginCommand.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 ede13cc818693..3f9744d06a099 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1051,7 +1051,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\\NotModifiedMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/NotModifiedMiddleware.php', 'OC\\AppFramework\\Middleware\\OCSMiddleware' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Middleware/OCSMiddleware.php', @@ -1149,7 +1148,6 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Authentication\\Login\\CreateSessionTokenCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/CreateSessionTokenCommand.php', 'OC\\Authentication\\Login\\EmailLoginCommand' => __DIR__ . '/../../..' . '/lib/private/Authentication/Login/EmailLoginCommand.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 b6e2df4ce7b49..946c08dbec91c 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -10,7 +10,6 @@ use OC\AppFramework\Http; use OC\AppFramework\Http\Dispatcher; use OC\AppFramework\Http\Output; -use OC\AppFramework\Middleware\FlowV2EphemeralSessionsMiddleware; use OC\AppFramework\Middleware\MiddlewareDispatcher; use OC\AppFramework\Middleware\OCSMiddleware; use OC\AppFramework\Middleware\Security\CORSMiddleware; @@ -217,8 +216,6 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta ) ); - $dispatcher->registerMiddleware($c->get(FlowV2EphemeralSessionsMiddleware::class)); - $securityMiddleware = new SecurityMiddleware( $c->get(IRequest::class), $c->get(IControllerMethodReflector::class), diff --git a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php deleted file mode 100644 index e39bf68a81ddd..0000000000000 --- a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php +++ /dev/null @@ -1,80 +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; - } - - $reflectionMethod = new ReflectionMethod($controller, $methodName); - if (!empty($reflectionMethod->getAttributes(PublicPage::class))) { - return; - } - - if ($this->reflector->hasAnnotation('PublicPage')) { - 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 09030a154a68a..c2a0e7483442d 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, ) { } @@ -34,7 +33,6 @@ public function process(LoginData $loginData): LoginResult { ->setNext($this->emailLoginCommand) ->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 21f0433d948c4..db01084544a3c 100644 --- a/lib/private/Authentication/Login/CreateSessionTokenCommand.php +++ b/lib/private/Authentication/Login/CreateSessionTokenCommand.php @@ -10,19 +10,19 @@ use OC\Authentication\Token\IToken; use OC\User\Session; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\IConfig; +use OCP\IURLGenerator; class CreateSessionTokenCommand extends ALoginCommand { - /** @var IConfig */ - private $config; + private const EPHEMERAL_SESSION_TTL = 5 * 60; // 5 minutes - /** @var Session */ - private $userSession; - - public function __construct(IConfig $config, - Session $userSession) { - $this->config = $config; - $this->userSession = $userSession; + public function __construct( + private IConfig $config, + private Session $userSession, + private IURLGenerator $urlGenerator, + private ITimeFactory $timeFactory, + ) { } public function process(LoginData $loginData): LoginResult { @@ -35,13 +35,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(), @@ -53,7 +60,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 362aab7933e39..0000000000000 --- a/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php +++ /dev/null @@ -1,32 +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 b55970a497908..2fa0c5e6e1d49 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 12c3a1d535bd1..cf9456113da09 100644 --- a/lib/private/Authentication/Token/PublicKeyTokenProvider.php +++ b/lib/private/Authentication/Token/PublicKeyTokenProvider.php @@ -87,6 +87,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'); @@ -103,7 +104,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()); @@ -251,6 +252,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,6 +494,10 @@ private function newToken(string $token, $dbToken->setLastCheck($this->time->getTime()); $dbToken->setVersion(PublicKeyToken::VERSION); + if ($expires !== null) { + $dbToken->setExpires($expires); + } + return $dbToken; } diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php index 94b15133dfb73..ef0b62ce402a6 100644 --- a/lib/private/User/Session.php +++ b/lib/private/User/Session.php @@ -35,6 +35,7 @@ use OCP\Lockdown\ILockdownManager; use OCP\Security\Bruteforce\IThrottler; use OCP\Security\ISecureRandom; +use OCP\Server; use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\User\Events\PostLoginEvent; use OCP\User\Events\UserFirstTimeLoggedInEvent; @@ -652,15 +653,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; @@ -670,10 +671,10 @@ 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) { + $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); - }, \OCP\Server::get(IDBConnection::class)); + $this->tokenProvider->generateToken($sessionId, $uid, $loginName, $pwd, $name, IToken::TEMPORARY_TOKEN, $remember, expires:$expires); + }, Server::get(IDBConnection::class)); return true; } catch (SessionNotAvailableException $ex) { // This can happen with OCC, where a memory session is used diff --git a/tests/lib/Authentication/Login/CreateSessionTokenCommandTest.php b/tests/lib/Authentication/Login/CreateSessionTokenCommandTest.php index fd6a154ef5d0a..335fcac84a26a 100644 --- a/tests/lib/Authentication/Login/CreateSessionTokenCommandTest.php +++ b/tests/lib/Authentication/Login/CreateSessionTokenCommandTest.php @@ -1,40 +1,49 @@ 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') @@ -53,7 +62,8 @@ public function testProcess(): void { $this->username, $this->username, $this->password, - IToken::REMEMBER + IToken::REMEMBER, + null ); $this->userSession->expects($this->once()) ->method('updateTokens') @@ -68,6 +78,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') @@ -86,7 +100,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') @@ -100,4 +115,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()); + } }