diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md index de5208aa1a412..0fd8d4805c0bd 100644 --- a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md +++ b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md @@ -5,6 +5,7 @@ CHANGELOG ----- * Added security configuration for priority-based access decision strategy + * Added `switch_user.allow_already_switched` option to allow switching seamlessly when already switched (default `false`) 5.0.0 ----- diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php index 6361b0b4c36c8..a9fc5c9fbc933 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php @@ -238,6 +238,7 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto ->scalarNode('provider')->end() ->scalarNode('parameter')->defaultValue('_switch_user')->end() ->scalarNode('role')->defaultValue('ROLE_ALLOWED_TO_SWITCH')->end() + ->booleanNode('allow_already_switched')->defaultFalse()->end() ->end() ->end() ; diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php index baf395b4c4a94..847deaad323bf 100644 --- a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php +++ b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php @@ -684,6 +684,10 @@ private function createSwitchUserListener(ContainerBuilder $container, string $i throw new InvalidConfigurationException(sprintf('Not configuring explicitly the provider for the "switch_user" listener on "%s" firewall is ambiguous as there is more than one registered provider.', $id)); } + if ($stateless && $config['allow_already_switched']) { + throw new InvalidConfigurationException(sprintf('Cannot set "allow_already_switched" to true for the "switch_user" listener on firewall "%s" as it is stateless.', $id)); + } + $switchUserListenerId = 'security.authentication.switchuser_listener.'.$id; $listener = $container->setDefinition($switchUserListenerId, new ChildDefinition('security.authentication.switchuser_listener')); $listener->replaceArgument(1, new Reference($userProvider)); @@ -692,6 +696,7 @@ private function createSwitchUserListener(ContainerBuilder $container, string $i $listener->replaceArgument(6, $config['parameter']); $listener->replaceArgument(7, $config['role']); $listener->replaceArgument(9, $stateless); + $listener->replaceArgument(10, $config['allow_already_switched']); return $switchUserListenerId; } diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml index 6b4b441c98359..68d3211299e24 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml +++ b/src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml @@ -202,6 +202,7 @@ ROLE_ALLOWED_TO_SWITCH false + false diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php index 95e3e6e345306..c4563ee341861 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php @@ -111,6 +111,7 @@ public function testFirewalls() [ 'parameter' => '_switch_user', 'role' => 'ROLE_ALLOWED_TO_SWITCH', + 'allow_already_switched' => false, ], ], [ diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php index 0ccacf583fe57..05b074fa83da2 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/SwitchUserTest.php @@ -12,6 +12,7 @@ namespace Symfony\Bundle\SecurityBundle\Tests\Functional; use Symfony\Component\HttpFoundation\JsonResponse; +use Symfony\Component\Security\Core\Exception\AlreadySwitchedException; use Symfony\Component\Security\Http\Firewall\SwitchUserListener; class SwitchUserTest extends AbstractWebTestCase @@ -36,8 +37,20 @@ public function testSwitchedUserCannotSwitchToOther() $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->assertSame(403, $client->getResponse()->getStatusCode()); + $this->assertSame(AlreadySwitchedException::class, $client->getProfile()->getCollector('exception')->getException()->getPrevious()->getPrevious()->getClass()); + $this->assertSame('user_cannot_switch_1', $client->getProfile()->getCollector('security')->getUser()); + } + + public function testAlreadySwitchedUserCanSwitch() + { + $client = $this->createAuthenticatedClient('user_can_switch', 'switchuser_already_switched.yml'); + + $client->request('GET', '/profile?_switch_user=user_cannot_switch_1'); + $client->request('GET', '/profile?_switch_user=user_cannot_switch_2'); + + $this->assertSame(200, $client->getResponse()->getStatusCode()); + $this->assertSame('user_cannot_switch_2', $client->getProfile()->getCollector('security')->getUser()); } public function testSwitchedUserExit() @@ -73,9 +86,9 @@ public function getTestParameters() ]; } - protected function createAuthenticatedClient($username) + protected function createAuthenticatedClient($username, string $rootConfig = 'switchuser.yml') { - $client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => 'switchuser.yml']); + $client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => $rootConfig]); $client->followRedirects(true); $form = $client->request('GET', '/login')->selectButton('login')->form(); diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/switchuser_already_switched.yml b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/switchuser_already_switched.yml new file mode 100644 index 0000000000000..0ef6571dd3f92 --- /dev/null +++ b/src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/switchuser_already_switched.yml @@ -0,0 +1,14 @@ +imports: + - { resource: ./config.yml } + +security: + providers: + in_memory: + memory: + users: + user_can_switch: { password: test, roles: [ROLE_USER, ROLE_ALLOWED_TO_SWITCH] } + user_cannot_switch_1: { password: test, roles: [ROLE_USER] } + user_cannot_switch_2: { password: test, roles: [ROLE_USER] } + firewalls: + default: + switch_user: { allow_already_switched: true } diff --git a/src/Symfony/Bundle/SecurityBundle/composer.json b/src/Symfony/Bundle/SecurityBundle/composer.json index 4888fc17ff90b..26065548ee1cc 100644 --- a/src/Symfony/Bundle/SecurityBundle/composer.json +++ b/src/Symfony/Bundle/SecurityBundle/composer.json @@ -21,7 +21,7 @@ "symfony/config": "^4.4|^5.0", "symfony/dependency-injection": "^4.4|^5.0", "symfony/http-kernel": "^5.0", - "symfony/security-core": "^4.4|^5.0", + "symfony/security-core": "^5.1", "symfony/security-csrf": "^4.4|^5.0", "symfony/security-guard": "^4.4|^5.0", "symfony/security-http": "^5.1" 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/Core/Exception/AlreadySwitchedException.php b/src/Symfony/Component/Security/Core/Exception/AlreadySwitchedException.php new file mode 100644 index 0000000000000..4b91dd60b36b3 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Exception/AlreadySwitchedException.php @@ -0,0 +1,21 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Exception; + +/** + * Thrown when trying to switch to another user while being already switched. + * + * @author Robin Chalas + */ +final class AlreadySwitchedException extends AuthenticationException +{ +} diff --git a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php index f6d4c8a587c0a..a13d4d1a78b52 100644 --- a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php @@ -20,6 +20,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; +use Symfony\Component\Security\Core\Exception\AlreadySwitchedException; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\User\UserCheckerInterface; @@ -51,8 +52,9 @@ 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) + 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, bool $allowAlreadySwitched = false) { if (empty($providerKey)) { throw new \InvalidArgumentException('$providerKey must not be empty.'); @@ -68,6 +70,7 @@ public function __construct(TokenStorageInterface $tokenStorage, UserProviderInt $this->logger = $logger; $this->dispatcher = $dispatcher; $this->stateless = $stateless; + $this->allowAlreadySwitched = $allowAlreadySwitched; } /** @@ -94,8 +97,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 +132,7 @@ public function authenticate(RequestEvent $event) /** * Attempts to switch to another user and returns the new token if successfully switched. * - * @throws \LogicException + * @throws AlreadySwitchedException * @throws AccessDeniedException */ private function attemptSwitchUser(Request $request, string $username): ?TokenInterface @@ -142,9 +143,15 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn if (null !== $originalToken) { if ($token->getUsername() === $username) { return $token; + } elseif (!$this->allowAlreadySwitched) { + $e = new AlreadySwitchedException(sprintf('You are already switched to "%s" user.', $token->getUsername())); + $e->setToken($token); + + throw $e; } - throw new \LogicException(sprintf('You are already switched to "%s" user.', $token->getUsername())); + // Seamlessly exit from already switched 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']); diff --git a/src/Symfony/Component/Security/Http/composer.json b/src/Symfony/Component/Security/Http/composer.json index 52b59d083df5f..113e97fd8da74 100644 --- a/src/Symfony/Component/Security/Http/composer.json +++ b/src/Symfony/Component/Security/Http/composer.json @@ -17,7 +17,7 @@ ], "require": { "php": "^7.2.5", - "symfony/security-core": "^4.4|^5.0", + "symfony/security-core": "^5.1", "symfony/http-foundation": "^4.4|^5.0", "symfony/http-kernel": "^4.4|^5.0", "symfony/property-access": "^4.4|^5.0"