From 9141eb473f6efb6bedbf12b507bfe46c586485d6 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Mon, 3 Mar 2025 11:38:03 +0100 Subject: [PATCH 1/3] 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 [skip ci] 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 e6e1c282d2b..b21ab2def54 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -41,6 +41,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'; public function __construct( string $appName, diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 4acb7bd7ce6..f16f64f115a 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -929,6 +929,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', @@ -1026,6 +1027,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 c6f32ac6247..811118be8e4 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -962,6 +962,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', @@ -1059,6 +1060,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 d223cfa8f1a..63102dd618c 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -10,6 +10,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; @@ -219,7 +220,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..b3e406adf22 --- /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 abd24287a6c..09030a154a6 100644 --- a/lib/private/Authentication/Login/Chain.php +++ b/lib/private/Authentication/Login/Chain.php @@ -9,67 +9,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 { @@ -80,6 +34,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); + } +} From a25a8f4f53b2157ecd5a82da5dd5e6b869e958cf Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 27 Feb 2025 13:12:55 +0100 Subject: [PATCH 2/3] fix(login): Also check legacy annotation for ephemeral sessions Signed-off-by: Louis Chemineau --- .../AppFramework/DependencyInjection/DIContainer.php | 7 +------ .../Middleware/FlowV2EphemeralSessionsMiddleware.php | 6 ++++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 63102dd618c..a33a385f21b 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -220,12 +220,7 @@ class DIContainer extends SimpleContainer implements IAppContainer { ) ); - $dispatcher->registerMiddleware( - new FlowV2EphemeralSessionsMiddleware( - $c->get(ISession::class), - $c->get(IUserSession::class), - ) - ); + $dispatcher->registerMiddleware($c->get(FlowV2EphemeralSessionsMiddleware::class)); $securityMiddleware = new SecurityMiddleware( $c->get(IRequest::class), diff --git a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php index b3e406adf22..461a8f91884 100644 --- a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php +++ b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php @@ -7,6 +7,7 @@ declare(strict_types=1); */ namespace OC\AppFramework\Middleware; +use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Core\Controller\ClientFlowLoginV2Controller; use OCP\AppFramework\Controller; use OCP\AppFramework\Middleware; @@ -20,6 +21,7 @@ class FlowV2EphemeralSessionsMiddleware extends Middleware { public function __construct( private ISession $session, private IUserSession $userSession, + private ControllerMethodReflector $reflector, ) { } @@ -40,6 +42,10 @@ class FlowV2EphemeralSessionsMiddleware extends Middleware { return; } + if ($this->reflector->hasAnnotation('PublicPage')) { + return; + } + $this->userSession->logout(); $this->session->close(); } From 0c809ccc7fc8e1c29ccb25ca6f6351978920ab42 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 27 Feb 2025 13:13:26 +0100 Subject: [PATCH 3/3] fix(login): Support subfolder install for ephemeral sessions Signed-off-by: Louis Chemineau --- .../Authentication/Login/FlowV2EphemeralSessionsCommand.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php b/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php index b215df1523f..82dd829334d 100644 --- a/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php +++ b/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php @@ -10,15 +10,18 @@ namespace OC\Authentication\Login; use OC\Core\Controller\ClientFlowLoginV2Controller; use OCP\ISession; +use OCP\IURLGenerator; class FlowV2EphemeralSessionsCommand extends ALoginCommand { public function __construct( private ISession $session, + private IURLGenerator $urlGenerator, ) { } public function process(LoginData $loginData): LoginResult { - if (str_starts_with($loginData->getRedirectUrl() ?? '', '/login/v2/grant')) { + $loginV2GrantRoute = $this->urlGenerator->linkToRoute('core.ClientFlowLoginV2.grantPage'); + if (str_starts_with($loginData->getRedirectUrl() ?? '', $loginV2GrantRoute)) { $this->session->set(ClientFlowLoginV2Controller::EPHEMERAL_NAME, true); }