8000 [Security] Migrate the session on login only when the user changes · symfony/symfony@238f25c · GitHub
[go: up one dir, main page]
Skip to content

Commit 238f25c

Browse files
nicolas-grekaswouterj
nicolas-grekas
authored and
wouterj
committed
[Security] Migrate the session on login only when the user changes
1 parent 5bc4d55 commit 238f25c

14 files changed

+125
-131
lines changed

‎src/Symfony/Bundle/SecurityBundle/Resources/config/guard.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
abstract_arg('Authenticators'),
5050
service('logger')->nullOnInvalid(),
5151
param('security.authentication.hide_user_not_found'),
52+
service('security.token_storage'),
5253
])
5354
->tag('monolog.logger', ['channel' => 'security'])
5455
->deprecate('symfony/security-bundle', '5.3', 'The "%service_id%" service is deprecated, use the new authenticator system instead.')

‎src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\HttpFoundation\Response;
1717
use Symfony\Component\HttpKernel\Event\RequestEvent;
1818
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
19+
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
1920
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2021
use Symfony\Component\Security\Core\Exception\AccountStatusException;
2122
use Symfony\Component\Security\Core\Exception\AuthenticationException;
@@ -49,12 +50,13 @@ class GuardAuthenticationListener extends AbstractListener
4950
private $logger;
5051
private $rememberMeServices;
5152
private $hideUserNotFoundExceptions;
53+
private $tokenStorage;
5254

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

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

164168
// sets the token on the token storage, etc
165-
$this->guardHandler->authenticateWithToken($token, $request, $this->providerKey);
169+
if ($this->tokenStorage) {
170+
$this->guardHandler->authenticateWithToken($token, $request, $this->providerKey, $previousToken);
171+
} else {
172+
$this->guardHandler->authenticateWithToken($token, $request, $this->providerKey);
173+
}
166174
} catch (AuthenticationException $e) {
167175
// oh no! Authentication failed!
168176

‎src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ public function __construct(TokenStorageInterface $tokenStorage, EventDispatcher
5656
/**
5757
* Authenticates the given token in the system.
5858
*/
59-
public function authenticateWithToken(TokenInterface $token, Request $request, string $providerKey = null)
59+
public function authenticateWithToken(TokenInterface $token, Request $request, string $providerKey = null, TokenInterface $previousToken = null)
6060
{
61-
$this->migrateSession($request, $token, $providerKey);
61+
$this->migrateSession($request, $token, $providerKey, 3 < \func_num_args() ? $previousToken : $this->tokenStorage->getToken());
6262
$this->tokenStorage->setToken($token);
6363

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

9696
// return the success metric
9797
return $this->handleAuthenticationSuccess($token, $request, $authenticator, $providerKey);
@@ -122,12 +122,21 @@ public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyIn
122122
$this->sessionStrategy = $sessionStrategy;
123123
}
124124

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

131+
if ($previousToken) {
132+
$user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername();
133+
$previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername();
134+
135+
if ('' !== ($user ?? '') && $user === $previousUser) {
136+
return;
137+
}
138+
}
139+
131140
$this->sessionStrategy->onAuthentication($request, $token);
132141
}
133142
}

‎src/Symfony/Component/Security/Http/Authentication/AuthenticatorManager.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public function authenticateUser(UserInterface $user, AuthenticatorInterface $au
8484
$token = $this->eventDispatcher->dispatch(new AuthenticationTokenCreatedEvent($token, $passport))->getAuthenticatedToken();
8585

8686
// authenticate this in the system
87-
return $this->handleAuthenticationSuccess($token, $passport, $request, $authenticator);
87+
return $this->handleAuthenticationSuccess($token, $passport, $request, $authenticator, $this->tokenStorage->getToken());
8888
}
8989

9090
public function supports(Request $request): ?bool
@@ -174,6 +174,7 @@ private function executeAuthenticators(array $authenticators, Request $request):
174174
private function executeAuthenticator(AuthenticatorInterface $authenticator, Request $request): ?Response
175175
{
176176
$passport = null;
177+
$previousToken = $this->tokenStorage->getToken();
177178

178179
try {
179180
// get the passport from the Authenticator
@@ -224,7 +225,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
224225
}
225226

226227
// success! (sets the token on the token storage, etc)
227-
$response = $this->handleAuthenticationSuccess($authenticatedToken, $passport, $request, $authenticator);
228+
$response = $this->handleAuthenticationSuccess($authenticatedToken, $passport, $request, $authenticator, $previousToken);
228229
if ($response instanceof Response) {
229230
return $response;
230231
}
@@ -236,7 +237,7 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
236237
return null;
237238
}
238239

239-
private function handleAuthenticationSuccess(TokenInterface $authenticatedToken, PassportInterface $passport, Request $request, AuthenticatorInterface $authenticator): ?Response
240+
private function handleAuthenticationSuccess(TokenInterface $authenticatedToken, PassportInterface $passport, Request $request, AuthenticatorInterface $authenticator, ?TokenInterface $previousToken): ?Response
240241
{
241242
// @deprecated since Symfony 5.3
242243
$user = $authenticatedToken->getUser();
@@ -252,7 +253,7 @@ private function handleAuthenticationSuccess(TokenInterface $authenticatedToken,
252253
$this->eventDispatcher->dispatch($loginEvent, SecurityEvents::INTERACTIVE_LOGIN);
253254
}
254255

255-
$this->eventDispatcher->dispatch($loginSuccessEvent = new LoginSuccessEvent($authenticator, $passport, $authenticatedToken, $request, $response, $this->firewallName));
256+
$this->eventDispatcher->dispatch($loginSuccessEvent = new LoginSuccessEvent($authenticator, $passport, $authenticatedToken, $request, $response, $this->firewallName, $previousToken));
256257

257258
return $loginSuccessEvent->getResponse();
258259
}

‎src/Symfony/Component/Security/Http/Event/LoginSuccessEvent.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,15 @@ class LoginSuccessEvent extends Event
3737
private $authenticator;
3838
private $passport;
3939
private $authenticatedToken;
40+
private $previousToken;
4041
private $request;
4142
private $response;
4243
private $firewallName;
4344

4445
/**
4546
* @param Passport $passport
4647
*/
47-
public function __construct(AuthenticatorInterface $authenticator, PassportInterface $passport, TokenInterface $authenticatedToken, Request $request, ?Response $response, string $firewallName)
48+
public function __construct(AuthenticatorInterface $authenticator, PassportInterface $passport, TokenInterface $authenticatedToken, Request $request, ?Response $response, string $firewallName, TokenInterface $previousToken = null)
4849
{
4950
if (!$passport instanceof Passport) {
5051
trigger_deprecation('symfony/security-http', '5.4', 'Not passing an instance of "%s" as "$passport" argument of "%s()" is deprecated, "%s" given.', Passport::class, __METHOD__, get_debug_type($passport));
@@ -53,6 +54,7 @@ public function __construct(AuthenticatorInterface $authenticator, PassportInter
5354
$this->authenticator = $authenticator;
5455
$this->passport = $passport;
5556
$this->authenticatedToken = $authenticatedToken;
57+
$this->previousToken = $previousToken;
5658
$this->request = $request;
5759
$this->response = $response;
5860
$this->firewallName = $firewallName;
@@ -83,6 +85,11 @@ public function getAuthenticatedToken(): TokenInterface
8385
return $this->authenticatedToken;
8486
}
8587

88+
public function getPreviousToken(): ?TokenInterface
89+
{
90+
return $this->previousToken;
91+
}
92+
8693
public function getRequest(): Request
8794
{
8895
return $this->request;

‎src/Symfony/Component/Security/Http/EventListener/SessionStrategyListener.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ public function onSuccessfulLogin(LoginSuccessEvent $event): void
4343
return;
4444
}
4545

46+
if ($previousToken = $event->getPreviousToken()) {
47+
// @deprecated since Symfony 5.3, change to $token->getUserIdentifier() in 6.0
48+
$user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername();
49+
$previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername();
50+
51+
if ('' !== ($user ?? '') && $user === $previousUser) {
52+
return;
53+
}
54+
}
55+
4656
$this->sessionAuthenticationStrategy->onAuthentication($request, $token);
4757
}
4858

‎src/Symfony/Component/Security/Http/Firewall/AbstractAuthenticationListener.php

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,14 @@ public function authenticate(RequestEvent $event)
133133
throw new SessionUnavailableException('Your session has timed out, or you have disabled cookies.');
134134
}
135135

136+
$previousToken = $this->tokenStorage->getToken();
137+
136138
if (null === $returnValue = $this->attemptAuthentication($request)) {
137139
return;
138140
}
139141

140142
if ($returnValue instanceof TokenInterface) {
141-
$this->sessionStrategy->onAuthentication($request, $returnValue);
143+
$this->migrateSession($request, $returnValue, $previousToken);
142144

143145
$response = $this->onSuccess($request, $returnValue);
144146
} elseif ($returnValue instanceof Response) {
@@ -226,4 +228,18 @@ private function onSuccess(Request $request, TokenInterface $token): Response
226228

227229
return $response;
228230
}
231+
232+
private function migrateSession(Request $request, TokenInterface $token, ?TokenInterface $previousToken)
233+
{
234+
if ($previousToken) {
235+
$user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername();
236+
$previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername();
237+
238+
if ('' !== ($user ?? '') && $user === $previousUser) {
239+
return;
240+
}
241+
}
242+
243+
$this->sessionStrategy->onAuthentication($request, $token);
244+
}
229245
}

‎src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,14 @@ public function authenticate(RequestEvent $event)
9898
}
9999

100100
try {
101+
$previousToken = $token;
101102
$token = $this->authenticationManager->authenticate(new PreAuthenticatedToken($user, $credentials, $this->providerKey));
102103

103104
if (null !== $this->logger) {
104105
$this->logger->info('Pre-authentication successful.', ['token' => (string) $token]);
105106
}
106107

107-
$this->migrateSession($request, $token);
108+
$this->migrateSession($request, $token, $previousToken);
108109

109110
$this->tokenStorage->setToken($token);
110111

@@ -149,12 +150,21 @@ private function clearToken(AuthenticationException $exception)
149150
*/
150151
abstract protected function getPreAuthenticatedData(Request $request);
151152

152-
private function migrateSession(Request $request, TokenInterface $token)
153+
private function migrateSession(Request $request, TokenInterface $token, ?TokenInterface $previousToken)
153154
{
154155
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
155156
return;
156157
}
157158

159+
if ($previousToken) {
160+
$user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername();
161+
$previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername();
162+
163+
if ('' !== ($user ?? '') && $user === $previousUser) {
164+
return;
165+
}
166+
}
167+
158168
$this->sessionStrategy->onAuthentication($request, $token);
159169
}
160170
}

‎src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,10 @@ public function authenticate(RequestEvent $event)
8888
}
8989

9090
try {
91+
$previousToken = $token;
9192
$token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey));
9293

93-
$this->migrateSession($request, $token);
94+
$this->migrateSession($request, $token, $previousToken);
9495

9596
$this->tokenStorage->setToken($token);
9697
} catch (AuthenticationException $e) {
@@ -121,12 +122,21 @@ public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyIn
121122
$this->sessionStrategy = $sessionStrategy;
122123
}
123124

124-
private function migrateSession(Request $request, TokenInterface $token)
125+
private function migrateSession(Request $request, TokenInterface $token, ?TokenInterface $previousToken)
125126
{
126127
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
127128
return;
128129
}
129130

131+
if ($previousToken) {
132+
$user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername();
133+
$previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername();
134+
135+
if ('' !== ($user ?? '') && $user === $previousUser) {
136+
return;
137+
}
138+
}
139+
130140
$this->sessionStrategy->onAuthentication($request, $token);
131141
}
132142
}

‎src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ public function authenticate(RequestEvent $event)
101101
{
102102
$request = $event->getRequest();
103103
$data = json_decode($request->getContent());
104+
$previousToken = $this->tokenStorage->getToken();
104105

105106
try {
106107
if (!$data instanceof \stdClass) {
@@ -134,7 +135,7 @@ public function authenticate(RequestEvent $event)
134135
$token = new UsernamePasswordToken($username, $password, $this->providerKey);
135136

136137
$authenticatedToken = $this->authenticationManager->authenticate($token);
137-
$response = $this->onSuccess($request, $authenticatedToken);
138+
$response = $this->onSuccess($request, $authenticatedToken, $previousToken);
138139
} catch (AuthenticationException $e) {
139140
$response = $this->onFailure($request, $e);
140141
} catch (BadRequestHttpException $e) {
@@ -150,14 +151,14 @@ public function authenticate(RequestEvent $event)
150151
$event->setResponse($response);
151152
}
152153

153-
private function onSuccess(Request $request, TokenInterface $token): ?Response
154+
private function onSuccess(Request $request, TokenInterface $token, ?TokenInterface $previousToken): ?Response
154155
{
155156
if (null !== $this->logger) {
156157
// @deprecated since Symfony 5.3, change to $token->getUserIdentifier() in 6.0
157158
$this->logger->info('User has been authenticated successfully.', ['username' => method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername()]);
158159
}
159160

160-
$this->migrateSession($request, $token);
161+
$this->migrateSession($request, $token, $previousToken);
161162

162163
$this->tokenStorage->setToken($token);
163164

@@ -224,12 +225,21 @@ public function setTranslator(TranslatorInterface $translator)
224225
$this->translator = $translator;
225226
}
226227

227-
private function migrateSession(Request $request, TokenInterface $token)
228+
private function migrateSession(Request $request, TokenInterface $token, ?TokenInterface $previousToken)
228229
{
229230
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
230231
return;
231232
}
232233

234+
if ($previousToken) {
235+
$user = method_exists($token, 'getUserIdentifier') ? $token->getUserIdentifier() : $token->getUsername();
236+
$previousUser = method_exists($previousToken, 'getUserIdentifier') ? $previousToken->getUserIdentifier() : $previousToken->getUsername();
237+
238+
if ('' !== ($user ?? '') && $user === $previousUser) {
239+
return;
240+
}
241+
}
242+
233243
$this->sessionStrategy->onAuthentication($request, $token);
234244
}
235245
}

‎src/Symfony/Component/Security/Http/Tests/EventListener/PasswordMigratingListenerTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\HttpFoundation\Request;
1616
use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
1717
use Symfony\Component\PasswordHasher\PasswordHasherInterface;
18+
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1819
use Symfony\Component\Security\Core\User\InMemoryUser;
1920
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
2021
use Symfony\Component\Security\Core\User\PasswordUpgraderInterface;
@@ -28,7 +29,6 @@
2829
use Symfony\Component\Security\Http\Event\LoginSuccessEvent;
2930
use Symfony\Component\Security\Http\EventListener\PasswordMigratingListener;
3031
use Symfony\Component\Security\Http\Tests\Fixtures\DummyAuthenticator;
31-
use Symfony\Component\Security\Http\Tests\Fixtures\DummyToken;
3232

3333
class PasswordMigratingListenerTest extends TestCase
3434
{
@@ -140,7 +140,7 @@ private static function createPasswordUpgrader()
140140

141141
private static function createEvent(PassportInterface $passport)
142142
{
143-
return new LoginSuccessEvent(new DummyAuthenticator(), $passport, new DummyToken(), new Request(), null, 'main');
143+
return new LoginSuccessEvent(new DummyAuthenticator(), $passport, new NullToken(), new Request(), null, 'main');
144144
}
145145
}
146146

0 commit comments

Comments
 (0)
0