bug #28072 [Security] Do not deauthenticate user when the first refreshed 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
This commit is contained in:
Robin Chalas 2018-10-10 10:05:32 +02:00
commit 2f0e5d7a83
2 changed files with 27 additions and 4 deletions

View File

@ -161,6 +161,7 @@ class ContextListener implements ListenerInterface
}
$userNotFoundByProvider = false;
$userDeauthenticated = false;
foreach ($this->userProviders as $provider) {
if (!$provider instanceof UserProviderInterface) {
@ -169,21 +170,26 @@ class ContextListener implements ListenerInterface
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());
@ -209,6 +215,14 @@ class ContextListener implements ListenerInterface
}
}
if ($userDeauthenticated) {
if (null !== $this->logger) {
$this->logger->debug('Token was deauthenticated after trying to refresh it.');
}
return null;
}
if ($userNotFoundByProvider) {
return null;
}

View File

@ -273,6 +273,15 @@ class ContextListenerTest extends TestCase
$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();