bug #35839 [Security] Allow switching to another user when already sw…

…itched (chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Allow switching to another user when already switched

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34690
| License       | MIT
| Doc PR        | -

Commits
-------

0353077 [Security] Allow switching to another user when already switched

src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php

@@ -33,15 +33,15 @@ public function testSwitchUser($originalUser, $targetUser, $expectedUser, $expec
         $this->assertEquals($expectedUser, $client->getProfile()->getCollector('security')->getUser());
     }
 
-    public function testSwitchedUserCannotSwitchToOther()
+    public function testSwitchedUserCanSwitchToOther()
     {
         $client = $this->createAuthenticatedClient('user_can_switch');
 
         $client->request('GET', '/profile?_switch_user=user_cannot_switch_1');
         $client->request('GET', '/profile?_switch_user=user_cannot_switch_2');
 
-        $this->assertEquals(500, $client->getResponse()->getStatusCode());
-        $this->assertEquals('user_cannot_switch_1', $client->getProfile()->getCollector('security')->getUser());
+        $this->assertEquals(200, $client->getResponse()->getStatusCode());
+        $this->assertEquals('user_cannot_switch_2', $client->getProfile()->getCollector('security')->getUser());
     }
 
     public function testSwitchedUserExit()

src/Symfony/Bundle/SecurityBundle/composer.json

@@ -19,7 +19,7 @@
         "php": "^5.5.9|>=7.0.8",
         "ext-xml": "*",
         "symfony/config": "~3.4|~4.0",
-        "symfony/security": "~3.4.37|~4.3.10|^4.4.3",
+        "symfony/security": "~3.4.38|~4.3.10|^4.4.5",
         "symfony/dependency-injection": "^3.4.3|^4.0.3",
         "symfony/http-kernel": "~3.4|~4.0",
         "symfony/polyfill-php70": "~1.0"

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

@@ -134,7 +134,8 @@ private function attemptSwitchUser(Request $request, $username)
                 return $token;
             }
 
-            throw new \LogicException(sprintf('You are already switched to "%s" user.', $token->getUsername()));
+            // User already switched, exit before seamlessly switching to another user
+            $token = $this->attemptExitUser($request);
         }
 
         if (false === $this->accessDecisionManager->decide($token, [$this->role])) {
@@ -152,7 +153,7 @@ private function attemptSwitchUser(Request $request, $username)
         $this->userChecker->checkPostAuth($user);
 
         $roles = $user->getRoles();
-        $roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $this->tokenStorage->getToken());
+        $roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $token);
 
         $token = new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey, $roles);
 

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

@@ -191,6 +191,36 @@ public function testSwitchUser()
         $this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $this->tokenStorage->getToken());
     }
 
+    public function testSwitchUserAlreadySwitched()
+    {
+        $originalToken = new UsernamePasswordToken('original', null, 'key', ['ROLE_FOO']);
+        $alreadySwitchedToken = new UsernamePasswordToken('switched_1', null, 'key', [new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $originalToken)]);
+
+        $tokenStorage = new TokenStorage();
+        $tokenStorage->setToken($alreadySwitchedToken);
+
+        $targetUser = new User('kuba', 'password', ['ROLE_FOO', 'ROLE_BAR']);
+        $this->request->query->set('_switch_user', 'kuba');
+
+        $this->accessDecisionManager->expects($this->once())
+            ->method('decide')->with($originalToken, ['ROLE_ALLOWED_TO_SWITCH'])
+            ->willReturn(true);
+        $this->userProvider->expects($this->once())
+            ->method('loadUserByUsername')
+            ->with('kuba')
+            ->willReturn($targetUser);
+        $this->userChecker->expects($this->once())
+            ->method('checkPostAuth')->with($targetUser);
+
+        $listener = new SwitchUserListener($tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager, null, '_switch_user', 'ROLE_ALLOWED_TO_SWITCH', null, false);
+        $listener->handle($this->event);
+
+        $this->assertSame([], $this->request->query->all());
+        $this->assertSame('', $this->request->server->get('QUERY_STRING'));
+        $this->assertSame('kuba', $tokenStorage->getToken()->getUsername());
+        $this->assertSame($originalToken, $tokenStorage->getToken()->getRoles()[2]->getSource());
+    }
+
     public function testSwitchUserWorksWithFalsyUsernames()
     {
         $token = new UsernamePasswordToken('username', '', 'key', ['ROLE_FOO']);