From 845cb88cffdd60e883226000e23bd36b766c6b41 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Mon, 3 Mar 2025 11:38:10 +0100 Subject: [PATCH 1/4] 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 | 19 +++----- .../Login/FlowV2EphemeralSessionsCommand.php | 32 +++++++++++++ 7 files changed, 96 insertions(+), 14 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 51163d200f5ad..fd020a6851761 100644 --- a/core/Controller/ClientFlowLoginV2Controller.php +++ b/core/Controller/ClientFlowLoginV2Controller.php @@ -49,6 +49,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'; private LoginFlowV2Service $loginFlowV2Service; private IURLGenerator $urlGenerator; diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 0f871f4dda841..4917346820763 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -686,6 +686,7 @@ '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', @@ -779,6 +780,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 91b2cfb2c2026..d4555b0cb8440 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -719,6 +719,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', @@ -812,6 +813,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 55278dabad5e2..81be04955da49 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -37,6 +37,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; @@ -236,7 +237,12 @@ public function __construct($appName, $urlParams = [], ServerContainer $server = ) ); - + $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..c73286f3c85b0 --- /dev/null +++ b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php @@ -0,0 +1,45 @@ +session = $session; + $this->userSession = $userSession; + } + + public function beforeController($controller, $methodName) { + if (!$this->session->get(ClientFlowLoginV2Controller::EPHEMERAL_NAME)) { + return; + } + + if ( + $controller instanceof ClientFlowLoginV2Controller && + ($methodName === 'grantPage' || $methodName === 'generateAppPassword') + ) { + return; + } + + $this->userSession->logout(); + $this->session->close(); + } +} diff --git a/lib/private/Authentication/Login/Chain.php b/lib/private/Authentication/Login/Chain.php index 32f44268d520d..4988777a5ed16 100644 --- a/lib/private/Authentication/Login/Chain.php +++ b/lib/private/Authentication/Login/Chain.php @@ -63,6 +63,9 @@ class Chain { /** @var FinishRememberedLoginCommand */ private $finishRememberedLoginCommand; + /** @var FlowV2EphemeralSessionsCommand */ + private $flowV2EphemeralSessionsCommand; + public function __construct(PreLoginHookCommand $preLoginHookCommand, UserDisabledCheckCommand $userDisabledCheckCommand, UidLoginCommand $uidLoginCommand, @@ -74,20 +77,9 @@ public function __construct(PreLoginHookCommand $preLoginHookCommand, UpdateLastPasswordConfirmCommand $updateLastPasswordConfirmCommand, SetUserTimezoneCommand $setUserTimezoneCommand, TwoFactorCommand $twoFactorCommand, - FinishRememberedLoginCommand $finishRememberedLoginCommand + FinishRememberedLoginCommand $finishRememberedLoginCommand, + 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 { @@ -98,6 +90,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..d4ad4a21e8889 --- /dev/null +++ b/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php @@ -0,0 +1,32 @@ +session = $session; + $this->urlGenerator = $urlGenerator; + } + + public function process(LoginData $loginData): LoginResult { + if (str_starts_with($loginData->getRedirectUrl() ?? '', '/login/v2/grant')) { + $this->session->set(ClientFlowLoginV2Controller::EPHEMERAL_NAME, true); + } + + return $this->processNextOrFinishSuccessfully($loginData); + } +} From 90363d231a35b5a9f1c807402476901cbd40076e Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 27 Feb 2025 13:12:55 +0100 Subject: [PATCH 2/4] fix(login): Also check legacy annotation for ephemeral sessions Signed-off-by: Louis Chemineau --- .../AppFramework/DependencyInjection/DIContainer.php | 7 +------ .../Middleware/FlowV2EphemeralSessionsMiddleware.php | 7 +++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php index 81be04955da49..0d234ed628103 100644 --- a/lib/private/AppFramework/DependencyInjection/DIContainer.php +++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php @@ -237,12 +237,7 @@ public function __construct($appName, $urlParams = [], ServerContainer $server = ) ); - $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 c73286f3c85b0..5d9170c4a6d63 100644 --- a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php +++ b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php @@ -7,6 +7,7 @@ */ namespace OC\AppFramework\Middleware; +use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Core\Controller\ClientFlowLoginV2Controller; use OCP\AppFramework\Middleware; use OCP\ISession; @@ -22,9 +23,11 @@ class FlowV2EphemeralSessionsMiddleware extends Middleware { public function __construct( ISession $session, IUserSession $userSession, + ControllerMethodReflector $reflector ) { $this->session = $session; $this->userSession = $userSession; + $this->reflector = $reflector; } public function beforeController($controller, $methodName) { @@ -39,6 +42,10 @@ public function beforeController($controller, $methodName) { return; } + if ($this->reflector->hasAnnotation('PublicPage')) { + return; + } + $this->userSession->logout(); $this->session->close(); } From 3c010c5c116ac1a1ec4d3b3923bfc95532fdc63d Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 27 Feb 2025 13:13:26 +0100 Subject: [PATCH 3/4] 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 d4ad4a21e8889..5dfbbe1723fc1 100644 --- a/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php +++ b/lib/private/Authentication/Login/FlowV2EphemeralSessionsCommand.php @@ -10,6 +10,7 @@ use OC\Core\Controller\ClientFlowLoginV2Controller; use OCP\ISession; +use OCP\IURLGenerator; class FlowV2EphemeralSessionsCommand extends ALoginCommand { private ISession $session; @@ -17,13 +18,15 @@ class FlowV2EphemeralSessionsCommand extends ALoginCommand { public function __construct( ISession $session, + IURLGenerator $urlGenerator ) { $this->session = $session; $this->urlGenerator = $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); } From 261fe815099c0e698c70f24dc336bcbd7317420b Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 3 Mar 2025 16:52:55 +0100 Subject: [PATCH 4/4] ci: Bump upload-artifact from v2 to v4 Signed-off-by: Andy Scherzinger --- .github/workflows/performance.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml index 5a53c6279b7f0..55373956df5d6 100644 --- a/.github/workflows/performance.yml +++ b/.github/workflows/performance.yml @@ -79,7 +79,7 @@ jobs: - name: Upload profiles if: always() - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@4cec3d8aa04e39d1a68397de0c4cd6fb9dce8ec1 with: name: profiles path: |