Skip to content

Commit

Permalink
[Security] Migrate the session on login only when the user changes
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas authored and wouterj committed Feb 26, 2023
1 parent f1cca39 commit 22d3c7e
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 6 deletions.
12 changes: 10 additions & 2 deletions Firewall/GuardAuthenticationListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\AccountStatusException;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
Expand Down Expand Up @@ -49,12 +50,13 @@ class GuardAuthenticationListener extends AbstractListener
private $logger;
private $rememberMeServices;
private $hideUserNotFoundExceptions;
private $tokenStorage;

/**
* @param string $providerKey The provider (i.e. firewall) key
* @param iterable<array-key, AuthenticatorInterface> $guardAuthenticators The authenticators, with keys that match what's passed to GuardAuthenticationProvider
*/
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, string $providerKey, iterable $guardAuthenticators, LoggerInterface $logger = null, bool $hideUserNotFoundExceptions = true)
public function __construct(GuardAuthenticatorHandler $guardHandler, AuthenticationManagerInterface $authenticationManager, string $providerKey, iterable $guardAuthenticators, LoggerInterface $logger = null, bool $hideUserNotFoundExceptions = true, TokenStorageInterface $tokenStorage = null)
{
if (empty($providerKey)) {
throw new \InvalidArgumentException('$providerKey must not be empty.');
Expand All @@ -66,6 +68,7 @@ public function __construct(GuardAuthenticatorHandler $guardHandler, Authenticat
$this->guardAuthenticators = $guardAuthenticators;
$this->logger = $logger;
$this->hideUserNotFoundExceptions = $hideUserNotFoundExceptions;
$this->tokenStorage = $tokenStorage;
}

/**
Expand Down Expand Up @@ -135,6 +138,7 @@ public function authenticate(RequestEvent $event)
private function executeGuardAuthenticator(string $uniqueGuardKey, AuthenticatorInterface $guardAuthenticator, RequestEvent $event)
{
$request = $event->getRequest();
$previousToken = $this->tokenStorage ? $this->tokenStorage->getToken() : null;
try {
if (null !== $this->logger) {
$this->logger->debug('Calling getCredentials() on guard authenticator.', ['firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)]);
Expand Down Expand Up @@ -162,7 +166,11 @@ private function executeGuardAuthenticator(string $uniqueGuardKey, Authenticator
}

// sets the token on the token storage, etc
$this->guardHandler->authenticateWithToken($token, $request, $this->providerKey);
if ($this->tokenStorage) {
$this->guardHandler->authenticateWithToken($token, $request, $this->providerKey, $previousToken);
} else {
$this->guardHandler->authenticateWithToken($token, $request, $this->providerKey);
}
} catch (AuthenticationException $e) {
// oh no! Authentication failed!

Expand Down
17 changes: 13 additions & 4 deletions GuardAuthenticatorHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ public function __construct(TokenStorageInterface $tokenStorage, EventDispatcher
/**
* Authenticates the given token in the system.
*/
public function authenticateWithToken(TokenInterface $token, Request $request, string $providerKey = null)
public function authenticateWithToken(TokenInterface $token, Request $request, string $providerKey = null, TokenInterface $previousToken = null)
{
$this->migrateSession($request, $token, $providerKey);
$this->migrateSession($request, $token, $providerKey, 3 < \func_num_args() ? $previousToken : $this->tokenStorage->getToken());
$this->tokenStorage->setToken($token);

if (null !== $this->dispatcher) {
Expand Down Expand Up @@ -91,7 +91,7 @@ public function authenticateUserAndHandleSuccess(UserInterface $user, Request $r
// create an authenticated token for the User
$token = $authenticator->createAuthenticatedToken($user, $providerKey);
// authenticate this in the system
$this->authenticateWithToken($token, $request, $providerKey);
$this->authenticateWithToken($token, $request, $providerKey, $this->tokenStorage->getToken());

// return the success metric
return $this->handleAuthenticationSuccess($token, $request, $authenticator, $providerKey);
Expand Down Expand Up @@ -122,12 +122,21 @@ public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyIn
$this->sessionStrategy = $sessionStrategy;
}

private function migrateSession(Request $request, TokenInterface $token, ?string $providerKey)
private function migrateSession(Request $request, TokenInterface $token, ?string $providerKey, ?TokenInterface $previousToken)
{
if (\in_array($providerKey, $this->statelessProviderKeys, true) || !$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

if ($previousToken) {
$user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername();
$previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername();

if ('' !== ($user ?? '') && $user === $previousUser) {
return;
}
}

$this->sessionStrategy->onAuthentication($request, $token);
}
}

0 comments on commit 22d3c7e

Please sign in to comment.