Merge branch '3.4' into 4.4

chalasr · chalasr · commit 3057c68b93d7 · 2020-02-26T11:27:30.000+01:00
* 3.4:
  [Security] Allow switching to another user when already switched
diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php
@@ -29,15 +29,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()
diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json
@@ -24,7 +24,7 @@
         "symfony/security-core": "^4.4",
         "symfony/security-csrf": "^4.2|^5.0",
         "symfony/security-guard": "^4.2|^5.0",
-        "symfony/security-http": "^4.4.3"
+        "symfony/security-http": "^4.4.5"
     },
     "require-dev": {
         "doctrine/doctrine-bundle": "^1.5|^2.0",
diff --git a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php
@@ -154,7 +154,8 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn
                 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);
         }
 
         $currentUsername = $token->getUsername();
@@ -189,7 +190,7 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn
         $this->userChecker->checkPostAuth($user);
 
         $roles = $user->getRoles();
-        $roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $this->tokenStorage->getToken(), false);
+        $roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $token, false);
 
         $token = new SwitchUserToken($user, $user->getPassword(), $this->providerKey, $roles, $token);
 
diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php
@@ -240,6 +240,39 @@ 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 SwitchUserToken('switched_1', null, 'key', ['ROLE_BAR'], $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'], $targetUser)
+            ->willReturn(true);
+
+        $this->userProvider->expects($this->exactly(2))
+            ->method('loadUserByUsername')
+            ->withConsecutive(['kuba'])
+            ->will($this->onConsecutiveCalls($targetUser, $this->throwException(new UsernameNotFoundException())));
+        $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($this->event);
+
+        $this->assertSame([], $this->request->query->all());
+        $this->assertSame('', $this->request->server->get('QUERY_STRING'));
+        $this->assertInstanceOf(SwitchUserToken::class, $tokenStorage->getToken());
+        $this->assertSame('kuba', $tokenStorage->getToken()->getUsername());
+        $this->assertSame($originalToken, $tokenStorage->getToken()->getOriginalToken());
+    }
+
     public function testSwitchUserWorksWithFalsyUsernames()
     {
         $token = new UsernamePasswordToken('username', '', 'key', ['ROLE_FOO']);

Original file line number	Diff line number	Diff line change
`@@ -29,15 +29,15 @@ public function testSwitchUser($originalUser, $targetUser, $expectedUser, $expec`
`29`	`29`	`$this->assertEquals($expectedUser, $client->getProfile()->getCollector('security')->getUser());`
`30`	`30`	`}`
`31`	`31`
`32`		`- public function testSwitchedUserCannotSwitchToOther()`
	`32`	`+ public function testSwitchedUserCanSwitchToOther()`
`33`	`33`	`{`
`34`	`34`	`$client = $this->createAuthenticatedClient('user_can_switch');`
`35`	`35`
`36`	`36`	`$client->request('GET', '/profile?_switch_user=user_cannot_switch_1');`
`37`	`37`	`$client->request('GET', '/profile?_switch_user=user_cannot_switch_2');`
`38`	`38`
`39`		`- $this->assertEquals(500, $client->getResponse()->getStatusCode());`
`40`		`- $this->assertEquals('user_cannot_switch_1', $client->getProfile()->getCollector('security')->getUser());`
	`39`	`+ $this->assertEquals(200, $client->getResponse()->getStatusCode());`
	`40`	`+ $this->assertEquals('user_cannot_switch_2', $client->getProfile()->getCollector('security')->getUser());`
`41`	`41`	`}`
`42`	`42`
`43`	`43`	`public function testSwitchedUserExit()`
Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,8 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn`
`154`	`154`	`return $token;`
`155`	`155`	`}`
`156`	`156`
`157`		`- throw new \LogicException(sprintf('You are already switched to "%s" user.', $token->getUsername()));`
	`157`	`+ // User already switched, exit before seamlessly switching to another user`
	`158`	`+ $token = $this->attemptExitUser($request);`
`158`	`159`	`}`
`159`	`160`
`160`	`161`	`$currentUsername = $token->getUsername();`
`@@ -189,7 +190,7 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn`
`189`	`190`	`$this->userChecker->checkPostAuth($user);`
`190`	`191`
`191`	`192`	`$roles = $user->getRoles();`
`192`		`- $roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $this->tokenStorage->getToken(), false);`
	`193`	`+ $roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $token, false);`
`193`	`194`
`194`	`195`	`$token = new SwitchUserToken($user, $user->getPassword(), $this->providerKey, $roles, $token);`
`195`	`196`