From c6293204a231ec8a9a18dec21ed340b8bdae3ec4 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Mon, 10 Feb 2025 16:42:48 +0100 Subject: [PATCH] feat: Close sessions created for login flow v2 Sessions created during the login flow v2 should be short lived to not leave an unexpected opened session in the browser. This commit add a property to the session object to track its origin, and will close it as soon as possible, i.e., on the first non public page request. Signed-off-by: Louis Chemineau --- .../ClientFlowLoginV2Controller.php | 2 + lib/composer/composer/autoload_classmap.php | 2 + lib/composer/composer/autoload_static.php | 2 + .../DependencyInjection/DIContainer.php | 8 +- .../FlowV2EphemeralSessionsMiddleware.php | 46 ++++++++++++ lib/private/Authentication/Login/Chain.php | 75 ++++--------------- .../Login/FlowV2EphemeralSessionsCommand.php | 27 +++++++ 7 files changed, 101 insertions(+), 61 deletions(-) create mode 100644 lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php create mode 100644 lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php diff --git a/core/Controller/ClientFlowLoginV2Controller.php b/core/Controller/ClientFlowLoginV2Controller.php index 4bd2cddd12bb5..1ce43c1993222 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -41,6 +41,8 @@ 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 b6095032c4c2d..26389031e2ad5 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -982,6 +982,7 @@ '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', @@ -1079,6 +1080,7 @@ '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 66df44dfb0644..d76c97ecc0710 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1031,6 +1031,7 @@ 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', @@ -1128,6 +1129,7 @@ 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 be4f6897da8e3..e1a2fefc55ab8 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -10,6 +10,7 @@ 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; @@ -216,7 +217,12 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta ) ); - + $dispatcher->registerMiddleware( + new FlowV2EphemeralSessionsMiddleware( + $c->get(ISession::class), + $c->get(IUserSession::class), + ) + ); $securityMiddleware = new SecurityMiddleware( $c->get(IRequest::class), diff --git a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php new file mode 100644 index 0000000000000..b3e406adf22fc --- /dev/null +++ b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php @@ -0,0 +1,46 @@ +session->get(ClientFlowLoginV2Controller::EPHEMERAL_NAME)) { + return; + } + + if ( + $controller instanceof ClientFlowLoginV2Controller && + ($methodName === 'grantPage' || $methodName === 'generateAppPassword') + ) { + return; + } + + $reflectionMethod = new ReflectionMethod($controller, $methodName); + if (!empty($reflectionMethod->getAttributes('PublicPage'))) { + return; + } + + $this->userSession->logout(); + $this->session->close(); + } +} diff --git a/lib/private/Authentication/Login/Chain.php b/lib/private/Authentication/Login/Chain.php index 3cba396afdd3e..09030a154a68a 100644 --- a/lib/private/Authentication/Login/Chain.php +++ b/lib/private/Authentication/Login/Chain.php @@ -9,67 +9,21 @@ namespace OC\Authentication\Login; class Chain { - /** @var PreLoginHookCommand */ - private $preLoginHookCommand; - - /** @var UserDisabledCheckCommand */ - private $userDisabledCheckCommand; - - /** @var UidLoginCommand */ - private $uidLoginCommand; - - /** @var EmailLoginCommand */ - private $emailLoginCommand; - - /** @var LoggedInCheckCommand */ - private $loggedInCheckCommand; - - /** @var CompleteLoginCommand */ - private $completeLoginCommand; - - /** @var CreateSessionTokenCommand */ - private $createSessionTokenCommand; - - /** @var ClearLostPasswordTokensCommand */ - private $clearLostPasswordTokensCommand; - - /** @var UpdateLastPasswordConfirmCommand */ - private $updateLastPasswordConfirmCommand; - - /** @var SetUserTimezoneCommand */ - private $setUserTimezoneCommand; - - /** @var TwoFactorCommand */ - private $twoFactorCommand; - - /** @var FinishRememberedLoginCommand */ - private $finishRememberedLoginCommand; - - public function __construct(PreLoginHookCommand $preLoginHookCommand, - UserDisabledCheckCommand $userDisabledCheckCommand, - UidLoginCommand $uidLoginCommand, - EmailLoginCommand $emailLoginCommand, - LoggedInCheckCommand $loggedInCheckCommand, - CompleteLoginCommand $completeLoginCommand, - CreateSessionTokenCommand $createSessionTokenCommand, - ClearLostPasswordTokensCommand $clearLostPasswordTokensCommand, - UpdateLastPasswordConfirmCommand $updateLastPasswordConfirmCommand, - SetUserTimezoneCommand $setUserTimezoneCommand, - TwoFactorCommand $twoFactorCommand, - FinishRememberedLoginCommand $finishRememberedLoginCommand, + public function __construct( + private PreLoginHookCommand $preLoginHookCommand, + private UserDisabledCheckCommand $userDisabledCheckCommand, + private UidLoginCommand $uidLoginCommand, + private EmailLoginCommand $emailLoginCommand, + private LoggedInCheckCommand $loggedInCheckCommand, + private CompleteLoginCommand $completeLoginCommand, + private CreateSessionTokenCommand $createSessionTokenCommand, + private ClearLostPasswordTokensCommand $clearLostPasswordTokensCommand, + private UpdateLastPasswordConfirmCommand $updateLastPasswordConfirmCommand, + private SetUserTimezoneCommand $setUserTimezoneCommand, + private TwoFactorCommand $twoFactorCommand, + private FinishRememberedLoginCommand $finishRememberedLoginCommand, + private FlowV2EphemeralSessionsCommand $flowV2EphemeralSessionsCommand, ) { - $this->preLoginHookCommand = $preLoginHookCommand; - $this->userDisabledCheckCommand = $userDisabledCheckCommand; - $this->uidLoginCommand = $uidLoginCommand; - $this->emailLoginCommand = $emailLoginCommand; - $this->loggedInCheckCommand = $loggedInCheckCommand; - $this->completeLoginCommand = $completeLoginCommand; - $this->createSessionTokenCommand = $createSessionTokenCommand; - $this->clearLostPasswordTokensCommand = $clearLostPasswordTokensCommand; - $this->updateLastPasswordConfirmCommand = $updateLastPasswordConfirmCommand; - $this->setUserTimezoneCommand = $setUserTimezoneCommand; - $this->twoFactorCommand = $twoFactorCommand; - $this->finishRememberedLoginCommand = $finishRememberedLoginCommand; } public function process(LoginData $loginData): LoginResult { @@ -80,6 +34,7 @@ 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/FlowV2EphemeralSessionsCommand.php b/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php new file mode 100644 index 0000000000000..b215df1523f48 --- /dev/null +++ b/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php @@ -0,0 +1,27 @@ +getRedirectUrl() ?? '', '/login/v2/grant')) { + $this->session->set(ClientFlowLoginV2Controller::EPHEMERAL_NAME, true); + } + + return $this->processNextOrFinishSuccessfully($loginData); + } +}