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

Author: Robin Chalas

Firewall/ContextListener.php

@@ -161,6 +161,7 @@ protected function refreshUser(TokenInterface $token)
         }
 
         $userNotFoundByProvider = false;
+        $userDeauthenticated = false;
 
         foreach ($this->userProviders as $provider) {
             if (!$provider instanceof UserProviderInterface) {
@@ -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());
 
@@ -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;
         }

Tests/Firewall/ContextListenerTest.php

@@ -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();