bug symfony#58754 [Security] Store original token in token storage when implicitly exiting impersonation (wouterj)

This PR was merged into the 5.4 branch.

Discussion
----------

[Security] Store original token in token storage when implicitly exiting impersonation

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

If you impersonate user A and then start impersonation for user B, Symfony explicitly exits the first impersonation before starting the second one. However, we did not update the token in the token storage at this moment.

This creates issues when using a custom voter [like the one documented](https://symfony.com/doc/current/security/impersonating_user.html#limiting-user-switching), as this uses `Security::isGranted()`, which relies on the token in the token storage. So instead of checking if the original user can impersonate, it will check if user A can impersonate.

Commits
-------

a496ecf [Security] Store original token in token storage when implicitly exiting impersonation

Author: fabpot

src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php

@@ -109,7 +109,7 @@ public function authenticate(RequestEvent $event)
         }
 
         if (self::EXIT_VALUE === $username) {
-            $this->tokenStorage->setToken($this->attemptExitUser($request));
+            $this->attemptExitUser($request);
         } else {
             try {
                 $this->tokenStorage->setToken($this->attemptSwitchUser($request, $username));
@@ -221,6 +221,8 @@ private function attemptExitUser(Request $request): TokenInterface
             $original = $switchEvent->getToken();
         }
 
+        $this->tokenStorage->setToken($original);
+
         return $original;
     }
 

src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php

@@ -18,6 +18,7 @@
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
 use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
 use Symfony\Component\Security\Core\Exception\AccessDeniedException;
@@ -228,7 +229,10 @@ public function testSwitchUserAlreadySwitched()
 
         $targetsUser = $this->callback(function ($user) { return 'kuba' === $user->getUserIdentifier(); });
         $this->accessDecisionManager->expects($this->once())
-            ->method('decide')->with($originalToken, ['ROLE_ALLOWED_TO_SWITCH'], $targetsUser)
+            ->method('decide')->with(self::callback(function (TokenInterface $token) use ($originalToken, $tokenStorage) {
+                // the token storage should also contain the original token for voters depending on it
+                return $token === $originalToken && $tokenStorage->getToken() === $originalToken;
+            }), ['ROLE_ALLOWED_TO_SWITCH'], $targetsUser)
             ->willReturn(true);
 
         $this->userChecker->expects($this->once())