Skip to content

Commit 07d4598

Browse files
chalasrchalasr
committed
[Security] Allow switching to another user when already switched
1 parent b3b368b commit 07d4598

File tree

12 files changed

+107
-10
lines changed

12 files changed

+107
-10
lines changed

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ CHANGELOG
55
-----
66

77
* Added security configuration for priority-based access decision strategy
8+
* Added `switch_user.allow_already_switched` option to allow switching seamlessly when already switched (default `false`)
89

910
5.0.0
1011
-----

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php

+1
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ private function addFirewallsSection(ArrayNodeDefinition $rootNode, array $facto
238238
->scalarNode('provider')->end()
239239
->scalarNode('parameter')->defaultValue('_switch_user')->end()
240240
->scalarNode('role')->defaultValue('ROLE_ALLOWED_TO_SWITCH')->end()
241+
->booleanNode('allow_already_switched')->defaultFalse()->end()
241242
->end()
242243
->end()
243244
;

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php

+5
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,10 @@ private function createSwitchUserListener(ContainerBuilder $container, string $i
684684
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));
685685
}
686686

687+
if ($stateless && $config['allow_already_switched']) {
688+
throw new InvalidConfigurationException(sprintf('Cannot set "allow_already_switched" to true for the "switch_user" listener on firewall "%s" as it is stateless.', $id));
689+
}
690+
687691
$switchUserListenerId = 'security.authentication.switchuser_listener.'.$id;
688692
$listener = $container->setDefinition($switchUserListenerId, new ChildDefinition('security.authentication.switchuser_listener'));
689693
$listener->replaceArgument(1, new Reference($userProvider));
@@ -692,6 +696,7 @@ private function createSwitchUserListener(ContainerBuilder $container, string $i
692696
$listener->replaceArgument(6, $config['parameter']);
693697
$listener->replaceArgument(7, $config['role']);
694698
$listener->replaceArgument(9, $stateless);
699+
$listener->replaceArgument(10, $config['allow_already_switched']);
695700

696701
return $switchUserListenerId;
697702
}

‎src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml

+1
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@
202202
<argument>ROLE_ALLOWED_TO_SWITCH</argument>
203203
<argument type="service" id="event_dispatcher" on-invalid="null"/>
204204
<argument>false</argument> <!-- Stateless -->
205+
<argument>false</argument> <!-- Consecutive Switching -->
205206
</service>
206207

207208
<service id="security.access_listener" class="Symfony\Component\Security\Http\Firewall\AccessListener">

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ public function testFirewalls()
111111
[
112112
'parameter' => '_switch_user',
113113
'role' => 'ROLE_ALLOWED_TO_SWITCH',
114+
'allow_already_switched' => false,
114115
],
115116
],
116117
[

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

+17-4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bundle\SecurityBundle\Tests\Functional;
1313

1414
use Symfony\Component\HttpFoundation\JsonResponse;
15+
use Symfony\Component\Security\Core\Exception\AlreadySwitchedException;
1516
use Symfony\Component\Security\Http\Firewall\SwitchUserListener;
1617

1718
class SwitchUserTest extends AbstractWebTestCase
@@ -36,8 +37,20 @@ public function testSwitchedUserCannotSwitchToOther()
3637
$client->request('GET', '/profile?_switch_user=user_cannot_switch_1');
3738
$client->request('GET', '/profile?_switch_user=user_cannot_switch_2');
3839

39-
$this->assertEquals(500, $client->getResponse()->getStatusCode());
40-
$this->assertEquals('user_cannot_switch_1', $client->getProfile()->getCollector('security')->getUser());
40+
$this->assertSame(403, $client->getResponse()->getStatusCode());
41+
$this->assertSame(AlreadySwitchedException::class, $client->getProfile()->getCollector('exception')->getException()->getPrevious()->getPrevious()->getClass());
42+
$this->assertSame('user_cannot_switch_1', $client->getProfile()->getCollector('security')->getUser());
43+
}
44+
45+
public function testAlreadySwitchedUserCanSwitch()
46+
{
47+
$client = $this->createAuthenticatedClient('user_can_switch', 'switchuser_already_switched.yml');
48+
49+
$client->request('GET', '/profile?_switch_user=user_cannot_switch_1');
50+
$client->request('GET', '/profile?_switch_user=user_cannot_switch_2');
51+
52+
$this->assertSame(200, $client->getResponse()->getStatusCode());
53+
$this->assertSame('user_cannot_switch_2', $client->getProfile()->getCollector('security')->getUser());
4154
}
4255

4356
public function testSwitchedUserExit()
@@ -73,9 +86,9 @@ public function getTestParameters()
7386
];
7487
}
7588

76-
protected function createAuthenticatedClient($username)
89+
protected function createAuthenticatedClient($username, string $rootConfig = 'switchuser.yml')
7790
{
78-
$client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => 'switchuser.yml']);
91+
$client = $this->createClient(['test_case' => 'StandardFormLogin', 'root_config' => $rootConfig]);
7992
$client->followRedirects(true);
8093

8194
$form = $client->request('GET', '/login')->selectButton('login')->form();

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/app/StandardFormLogin/switchuser_already_switched.yml

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
imports:
2+
- { resource: ./config.yml }
3+
4+
security:
5+
providers:
6+
in_memory:
7+
memory:
8+
users:
9+
user_can_switch: { password: test, roles: [ROLE_USER, ROLE_ALLOWED_TO_SWITCH] }
10+
user_cannot_switch_1: { password: test, roles: [ROLE_USER] }
11+
user_cannot_switch_2: { password: test, roles: [ROLE_USER] }
12+
firewalls:
13+
default:
14+
switch_user: { allow_already_switched: true }

‎src/Symfony/Component/Security/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ CHANGELOG
55
-----
66

77
* Added access decision strategy to override access decisions by voter service priority
8+
* Added `bool $allowAlreadySwitched` argument to the `SwitchUserListener` constructor (default `false`)
89

910
5.0.0
1011
-----

‎src/Symfony/Component/Security/Core/Exception/AlreadySwitchedException.php

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\Exception;
13+
14+
/**
15+
* Thrown when trying to switch to another user while being already switched.
16+
*
17+
* @author Robin Chalas <robin.chalas@gmail.com>
18+
*/
19+
final class AlreadySwitchedException extends AuthenticationException
20+
{
21+
}

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

+12-5
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2121
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
2222
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
23+
use Symfony\Component\Security\Core\Exception\AlreadySwitchedException;
2324
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
2425
use Symfony\Component\Security\Core\Exception\AuthenticationException;
2526
use Symfony\Component\Security\Core\User\UserCheckerInterface;
@@ -51,8 +52,9 @@ class SwitchUserListener extends AbstractListener
5152
private $logger;
5253
private $dispatcher;
5354
private $stateless;
55+
private $allowAlreadySwitched;
5456

55-
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)
57+
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)
5658
{
5759
if (empty($providerKey)) {
5860
throw new \InvalidArgumentException('$providerKey must not be empty.');
@@ -68,6 +70,7 @@ public function __construct(TokenStorageInterface $tokenStorage, UserProviderInt
6870
$this->logger = $logger;
6971
$this->dispatcher = $dispatcher;
7072
$this->stateless = $stateless;
73+
$this->allowAlreadySwitched = $allowAlreadySwitched;
7174
}
7275

7376
/**
@@ -94,8 +97,6 @@ public function supports(Request $request): ?bool
9497

9598
/**
9699
* Handles the switch to another user.
97-
*
98-
* @throws \LogicException if switching to a user failed
99100
*/
100101
public function authenticate(RequestEvent $event)
101102
{
@@ -131,7 +132,7 @@ public function authenticate(RequestEvent $event)
131132
/**
132133
* Attempts to switch to another user and returns the new token if successfully switched.
133134
*
134-
* @throws \LogicException
135+
* @throws AlreadySwitchedException
135136
* @throws AccessDeniedException
136137
*/
137138
private function attemptSwitchUser(Request $request, string $username): ?TokenInterface
@@ -142,9 +143,15 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn
142143
if (null !== $originalToken) {
143144
if ($token->getUsername() === $username) {
144145
return $token;
146+
} elseif (!$this->allowAlreadySwitched) {
147+
$e = new AlreadySwitchedException(sprintf('You are already switched to "%s" user.', $token->getUsername()));
148+
$e->setToken($token);
149+
150+
throw $e;
145151
}
146152

147-
throw new \LogicException(sprintf('You are already switched to "%s" user.', $token->getUsername()));
153+
// Seamlessly exit from already switched user
154+
$token = $this->attemptExitUser($request);
148155
}
149156

150157
$currentUsername = $token->getUsername();

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

+32
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,38 @@ public function testSwitchUser()
223223
$this->assertInstanceOf('Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken', $this->tokenStorage->getToken());
224224
}
225225

226+
public function testSwitchUserAlreadySwitched()
227+
{
228+
$originalToken = new UsernamePasswordToken('original', null, 'key', ['ROLE_FOO']);
229+
$alreadySwitchedToken = new SwitchUserToken('switched_1', null, 'key', ['ROLE_BAR'], $originalToken);
230+
231+
$tokenStorage = new TokenStorage();
232+
$tokenStorage->setToken($alreadySwitchedToken);
233+
234+
$targetUser = new User('kuba', 'password', ['ROLE_FOO', 'ROLE_BAR']);
235+
236+
$this->request->query->set('_switch_user', 'kuba');
237+
238+
$this->accessDecisionManager->expects($this->once())
239+
->method('decide')->with($originalToken, ['ROLE_ALLOWED_TO_SWITCH'], $targetUser)
240+
->willReturn(true);
241+
242+
$this->userProvider->expects($this->exactly(2))
243+
->method('loadUserByUsername')
244+
->withConsecutive(['kuba'])
245+
->will($this->onConsecutiveCalls($targetUser, $this->throwException(new UsernameNotFoundException())));
246+
$this->userChecker->expects($this->once())
247+
->method('checkPostAuth')->with($targetUser);
248+
249+
$listener = new SwitchUserListener($tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager, null, '_switch_user', 'ROLE_ALLOWED_TO_SWITCH', null, false, true);
250+
$listener($this->event);
251+
252+
$this->assertSame([], $this->request->query->all());
253+
$this->assertSame('', $this->request->server->get('QUERY_STRING'));
254+
$this->assertInstanceOf(SwitchUserToken::class, $tokenStorage->getToken());
255+
$this->assertSame('kuba', $tokenStorage->getToken()->getUsername());
256+
}
257+
226258
public function testSwitchUserWorksWithFalsyUsernames()
227259
{
228260
$token = new UsernamePasswordToken('username', '', 'key', ['ROLE_FOO']);

‎src/Symfony/Component/Security/Http/composer.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
],
1818
"require": {
1919
"php": "^7.2.5",
20-
"symfony/security-core": "^4.4|^5.0",
20+
"symfony/security-core": "^5.1",
2121
"symfony/http-foundation": "^4.4|^5.0",
2222
"symfony/http-kernel": "^4.4|^5.0",
2323
"symfony/property-access": "^4.4|^5.0"

0 commit comments

Comments
 (0)