Skip to content

Commit

Permalink
feat: Close sessions created for login flow v2
Browse files Browse the repository at this point in the history
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 <louis@chmn.me>
  • Loading branch information
artonge committed Feb 27, 2025
1 parent bfaa1ae commit c68656a
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 61 deletions.
2 changes: 2 additions & 0 deletions core/Controller/ClientFlowLoginV2Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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,
Expand Down
2 changes: 2 additions & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,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',
Expand Down Expand Up @@ -948,6 +949,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',
Expand Down
2 changes: 2 additions & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,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',
Expand Down Expand Up @@ -981,6 +982,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',
Expand Down
8 changes: 7 additions & 1 deletion lib/private/AppFramework/DependencyInjection/DIContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -244,7 +245,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),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-only
*/
namespace OC\AppFramework\Middleware;

use OC\Core\Controller\ClientFlowLoginV2Controller;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Middleware;
use OCP\ISession;
use OCP\IUserSession;
use ReflectionMethod;

// Will close the session if the user session is ephemeral.
// Happens when the user logs in via the login flow v2.
class FlowV2EphemeralSessionsMiddleware extends Middleware {
public function __construct(
private ISession $session,
private IUserSession $userSession,
) {
}

public function beforeController(Controller $controller, string $methodName) {
if (!$this->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();
}
}
75 changes: 15 additions & 60 deletions lib/private/Authentication/Login/Chain.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,67 +26,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 {
Expand All @@ -97,6 +51,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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OC\Authentication\Login;

use OC\Core\Controller\ClientFlowLoginV2Controller;
use OCP\ISession;

class FlowV2EphemeralSessionsCommand extends ALoginCommand {
public function __construct(
private ISession $session,
) {
}

public function process(LoginData $loginData): LoginResult {
if (str_contains($loginData->getRedirectUrl() ?? '', '/login/v2/grant')) {
$this->session->set(ClientFlowLoginV2Controller::EPHEMERAL_NAME, true);
}

return $this->processNextOrFinishSuccessfully($loginData);
}
}

0 comments on commit c68656a

Please sign in to comment.