From 242164f0fd360926e16193df2ea6e43e3bb7b3a0 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Mon, 3 Mar 2025 11:38:10 +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 | 45 +++++++++++ lib/private/Authentication/Login/Chain.php | 75 ++++--------------- .../Login/FlowV2EphemeralSessionsCommand.php | 27 +++++++ 7 files changed, 100 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 ef16cfbd04b..7b05df878cd 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -50,6 +50,8 @@ use OCP\Security\ISecureRandom; 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'; private LoginFlowV2Service $loginFlowV2Service; private IURLGenerator $urlGenerator; diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index f455e9b010e..869d6b463ab 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -702,6 +702,7 @@ return array( 'OC\\AppFramework\\Logger' => $baseDir . '/lib/private/AppFramework/Logger.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', @@ -795,6 +796,7 @@ return array( '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 37affb1babc..4e4ebabddde 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -735,6 +735,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\AppFramework\\Logger' => __DIR__ . '/../../..' . '/lib/private/AppFramework/Logger.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', @@ -828,6 +829,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 25e5aac453b..f35e0e605c0 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -37,6 +37,7 @@ use OC; 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; @@ -242,7 +243,12 @@ class DIContainer extends SimpleContainer implements IAppContainer { ) ); - + $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 00000000000..63c76f5be88 --- /dev/null +++ b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php @@ -0,0 +1,45 @@ +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 3c3179472c4..c59203ac19b 100644 --- a/lib/private/Authentication/Login/Chain.php +++ b/lib/private/Authentication/Login/Chain.php @@ -26,67 +26,21 @@ declare(strict_types=1); 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 { @@ -97,6 +51,7 @@ class Chain { ->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 00000000000..b215df1523f --- /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); + } +}