Skip to content

Commit

Permalink
bug #28072 [Security] Do not deauthenticate user when the first refre…
Browse files Browse the repository at this point in the history
…shed user has changed (gpekz)

This PR was squashed before being merged into the 3.4 branch (closes #28072).

Discussion
----------

[Security] Do not deauthenticate user when the first refreshed user has changed

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Currently the token is deauthenticated when the first refreshed user has changed. In theory, a second user provider could find a user that is the same than the user stored in the token.

Also, the deauthentication is currently affected by the order of the user providers in the security.yaml and IMHO it does not make sense.

Commits
-------

95dce67 [Security] Do not deauthenticate user when the first refreshed user has changed
  • Loading branch information
Robin Chalas committed Oct 10, 2018
2 parents 0df45a2 + 95dce67 commit 2f0e5d7
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
22 changes: 18 additions & 4 deletions src/Symfony/Component/Security/Http/Firewall/ContextListener.php
Expand Up @@ -161,6 +161,7 @@ protected function refreshUser(TokenInterface $token)
}

$userNotFoundByProvider = false;
$userDeauthenticated = false;

foreach ($this->userProviders as $provider) {
if (!$provider instanceof UserProviderInterface) {
Expand All @@ -169,21 +170,26 @@ protected function refreshUser(TokenInterface $token)

try {
$refreshedUser = $provider->refreshUser($user);
$token->setUser($refreshedUser);
$newToken = unserialize(serialize($token));
$newToken->setUser($refreshedUser);

// tokens can be deauthenticated if the user has been changed.
if (!$token->isAuthenticated()) {
if (!$newToken->isAuthenticated()) {
if ($this->logoutOnUserChange) {
$userDeauthenticated = true;

if (null !== $this->logger) {
$this->logger->debug('Token was deauthenticated after trying to refresh it.', array('username' => $refreshedUser->getUsername(), 'provider' => \get_class($provider)));
$this->logger->debug('Cannot refresh token because user has changed.', array('username' => $refreshedUser->getUsername(), 'provider' => \get_class($provider)));
}

return null;
continue;
}

@trigger_error('Refreshing a deauthenticated user is deprecated as of 3.4 and will trigger a logout in 4.0.', E_USER_DEPRECATED);
}

$token->setUser($refreshedUser);

if (null !== $this->logger) {
$context = array('provider' => \get_class($provider), 'username' => $refreshedUser->getUsername());

Expand All @@ -209,6 +215,14 @@ protected function refreshUser(TokenInterface $token)
}
}

if ($userDeauthenticated) {
if (null !== $this->logger) {
$this->logger->debug('Token was deauthenticated after trying to refresh it.');
}

return null;
}

if ($userNotFoundByProvider) {
return null;
}
Expand Down
Expand Up @@ -273,6 +273,15 @@ public function testIfTokenIsDeauthenticated()
$this->assertNull($tokenStorage->getToken());
}

public function testIfTokenIsNotDeauthenticated()
{
$tokenStorage = new TokenStorage();
$badRefreshedUser = new User('foobar', 'baz');
$goodRefreshedUser = new User('foobar', 'bar');
$this->handleEventWithPreviousSession($tokenStorage, array(new SupportingUserProvider($badRefreshedUser), new SupportingUserProvider($goodRefreshedUser)), $goodRefreshedUser, true);
$this->assertSame($goodRefreshedUser, $tokenStorage->getToken()->getUser());
}

public function testTryAllUserProvidersUntilASupportingUserProviderIsFound()
{
$tokenStorage = new TokenStorage();
Expand Down

0 comments on commit 2f0e5d7

Please sign in to comment.