From 1564985c24d30fa608abe00140f5f053889b390e Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Sun, 23 Feb 2020 15:53:58 +0100 Subject: [PATCH] [Security] Allow switching to another user when already switched --- .../Tests/Functional/SwitchUserTest.php | 6 ++-- src/Symfony/Component/Security/CHANGELOG.md | 1 + .../Http/Firewall/SwitchUserListener.php | 7 ++-- .../Tests/Firewall/SwitchUserListenerTest.php | 33 +++++++++++++++++++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php index 0ccacf583fe57..183b1ad8c4ef8 100644 --- 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/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index fe1cec6f7e518..1c7eb6c44c1a4 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/src/Symfony/Component/Security/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG ----- * Added access decision strategy to override access decisions by voter service priority + * Added `bool $allowAlreadySwitched` argument to the `SwitchUserListener` constructor (default `false`) 5.0.0 ----- diff --git a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php index f6d4c8a587c0a..8dbde38acabc9 100644 --- a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php @@ -51,6 +51,7 @@ class SwitchUserListener extends AbstractListener private $logger; private $dispatcher; private $stateless; + private $allowAlreadySwitched; public function __construct(TokenStorageInterface $tokenStorage, UserProviderInterface $provider, UserCheckerInterface $userChecker, string $providerKey, AccessDecisionManagerInterface $accessDecisionManager, LoggerInterface $logger = null, string $usernameParameter = '_switch_user', string $role = 'ROLE_ALLOWED_TO_SWITCH', EventDispatcherInterface $dispatcher = null, bool $stateless = false) { @@ -94,8 +95,6 @@ public function supports(Request $request): ?bool /** * Handles the switch to another user. - * - * @throws \LogicException if switching to a user failed */ public function authenticate(RequestEvent $event) { @@ -131,7 +130,6 @@ public function authenticate(RequestEvent $event) /** * Attempts to switch to another user and returns the new token if successfully switched. * - * @throws \LogicException * @throws AccessDeniedException */ private function attemptSwitchUser(Request $request, string $username): ?TokenInterface @@ -144,7 +142,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(); diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php index b0ab43efd1643..282cd441f544e 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php @@ -223,6 +223,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, true); + $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']);