Skip to content

Commit 29fb7fe

Browse files
fabpotfabpot
committed
feature #22048 [Security] deprecate the Role and SwitchUserRole classes (xabbuh)
This PR was merged into the 4.3-dev branch. Discussion ---------- [Security] deprecate the Role and SwitchUserRole classes | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #20824 | License | MIT | Doc PR | symfony/symfony-docs#11047 In #20801, we deprecated the `RoleInterface`. The next logical step would be to also deprecate the `Role` class. However, we currently have the `SwitchUserRole` class (a sub-class of `Role`) that acts as an indicator to check whether or not the authenticated user switched to another user. This PR proposes an alternative solution to the usage of the special `SwitchUserRole` class by storing the original token inside the `UsernamePasswordToken`. This PR is not complete, but rather acts as a proof of concept of how we could get rid of the `Role` and the `SwitchUserRole` classes. Please share your opinions whether you think this is a valid approach and I will be happy to finalise the PR. Commits ------- d7aaa615b9 deprecate the Role and SwitchUserRole classes
2 parents fb908e4 + db4f755 commit 29fb7fe

File tree

6 files changed

+48
-28
lines changed

6 files changed

+48
-28
lines changed

‎Firewall/ContextListener.php

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
2222
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
2323
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
24+
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
2425
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2526
use Symfony\Component\Security\Core\Exception\UnsupportedUserException;
2627
use Symfony\Component\Security\Core\Exception\UsernameNotFoundException;
@@ -189,10 +190,14 @@ protected function refreshUser(TokenInterface $token)
189190
if (null !== $this->logger) {
190191
$context = ['provider' => \get_class($provider), 'username' => $refreshedUser->getUsername()];
191192

192-
foreach ($token->getRoles() as $role) {
193-
if ($role instanceof SwitchUserRole) {
194-
$context['impersonator_username'] = $role->getSource()->getUsername();
195-
break;
193+
if ($token instanceof SwitchUserToken) {
194+
$context['impersonator_username'] = $token->getOriginalToken()->getUsername();
195+
} else {
196+
foreach ($token->getRoles(false) as $role) {
197+
if ($role instanceof SwitchUserRole) {
198+
$context['impersonator_username'] = $role->getSource(false)->getUsername();
199+
break;
200+
}
196201
}
197202
}
198203

‎Firewall/SwitchUserListener.php

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
use Symfony\Component\HttpFoundation\Request;
1818
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
1919
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
20+
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
2021
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
21-
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2222
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
2323
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
2424
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
@@ -122,7 +122,7 @@ private function attemptSwitchUser(Request $request, $username)
122122
$token = $this->tokenStorage->getToken();
123123
$originalToken = $this->getOriginalToken($token);
124124

125-
if (false !== $originalToken) {
125+
if (null !== $originalToken) {
126126
if ($token->getUsername() === $username) {
127127
return $token;
128128
}
@@ -146,9 +146,9 @@ private function attemptSwitchUser(Request $request, $username)
146146
$this->userChecker->checkPostAuth($user);
147147

148148
$roles = $user->getRoles();
149-
$roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $this->tokenStorage->getToken());
149+
$roles[] = new SwitchUserRole('ROLE_PREVIOUS_ADMIN', $this->tokenStorage->getToken(), false);
150150

151-
$token = new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey, $roles);
151+
$token = new SwitchUserToken($user, $user->getPassword(), $this->providerKey, $roles, $token);
152152

153153
if (null !== $this->dispatcher) {
154154
$switchEvent = new SwitchUserEvent($request, $token->getUser(), $token);
@@ -169,7 +169,7 @@ private function attemptSwitchUser(Request $request, $username)
169169
*/
170170
private function attemptExitUser(Request $request)
171171
{
172-
if (false === $original = $this->getOriginalToken($this->tokenStorage->getToken())) {
172+
if (null === ($currentToken = $this->tokenStorage->getToken()) || null === $original = $this->getOriginalToken($currentToken)) {
173173
throw new AuthenticationCredentialsNotFoundException('Could not find original Token object.');
174174
}
175175

@@ -183,19 +183,18 @@ private function attemptExitUser(Request $request)
183183
return $original;
184184
}
185185

186-
/**
187-
* Gets the original Token from a switched one.
188-
*
189-
* @return TokenInterface|false The original TokenInterface instance, false if the current TokenInterface is not switched
190-
*/
191-
private function getOriginalToken(TokenInterface $token)
186+
private function getOriginalToken(TokenInterface $token): ?TokenInterface
192187
{
193-
foreach ($token->getRoles() as $role) {
188+
if ($token instanceof SwitchUserToken) {
189+
return $token->getOriginalToken();
190+
}
191+
192+
foreach ($token->getRoles(false) as $role) {
194193
if ($role instanceof SwitchUserRole) {
195194
return $role->getSource();
196195
}
197196
}
198197

199-
return false;
198+
return null;
200199
}
201200
}

‎Tests/Controller/UserValueResolverTest.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
1919
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
2020
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
21+
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2122
use Symfony\Component\Security\Core\User\UserInterface;
2223
use Symfony\Component\Security\Http\Controller\UserValueResolver;
2324

@@ -35,7 +36,7 @@ public function testResolveNoToken()
3536
public function testResolveNoUser()
3637
{
3738
$mock = $this->getMockBuilder(UserInterface::class)->getMock();
38-
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
39+
$token = new UsernamePasswordToken('username', 'password', 'provider');
3940
$tokenStorage = new TokenStorage();
4041
$tokenStorage->setToken($token);
4142

@@ -57,8 +58,7 @@ public function testResolveWrongType()
5758
public function testResolve()
5859
{
5960
$user = $this->getMockBuilder(UserInterface::class)->getMock();
60-
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
61-
$token->expects($this->any())->method('getUser')->willReturn($user);
61+
$token = new UsernamePasswordToken($user, 'password', 'provider');
6262
$tokenStorage = new TokenStorage();
6363
$tokenStorage->setToken($token);
6464

@@ -72,8 +72,7 @@ public function testResolve()
7272
public function testIntegration()
7373
{
7474
$user = $this->getMockBuilder(UserInterface::class)->getMock();
75-
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
76-
$token->expects($this->any())->method('getUser')->willReturn($user);
75+
$token = new UsernamePasswordToken($user, 'password', 'provider');
7776
$tokenStorage = new TokenStorage();
7877
$tokenStorage->setToken($token);
7978

@@ -83,7 +82,7 @@ public function testIntegration()
8382

8483
public function testIntegrationNoUser()
8584
{
86-
$token = $this->getMockBuilder(TokenInterface::class)->getMock();
85+
$token = new UsernamePasswordToken('username', 'password', 'provider');
8786
$tokenStorage = new TokenStorage();
8887
$tokenStorage->setToken($token);
8988

‎Tests/Firewall/SwitchUserListenerTest.php

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
1717
use Symfony\Component\HttpKernel\HttpKernelInterface;
1818
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
19+
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
1920
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2021
use Symfony\Component\Security\Core\Role\SwitchUserRole;
2122
use Symfony\Component\Security\Core\User\User;
@@ -93,7 +94,7 @@ public function testExitUserThrowsAuthenticationExceptionIfOriginalTokenCannotBe
9394
public function testExitUserUpdatesToken()
9495
{
9596
$originalToken = new UsernamePasswordToken('username', '', 'key', []);
96-
$this->tokenStorage->setToken(new UsernamePasswordToken('username', '', 'key', [new SwitchUserRole('ROLE_PREVIOUS', $originalToken)]));
97+
$this->tokenStorage->setToken(new SwitchUserToken('username', '', 'key', [new SwitchUserRole('ROLE_PREVIOUS', $originalToken, false)], $originalToken));
9798

9899
$this->request->query->set('_switch_user', SwitchUserListener::EXIT_VALUE);
99100

@@ -107,6 +108,22 @@ public function testExitUserUpdatesToken()
107108
$this->assertSame($originalToken, $this->tokenStorage->getToken());
108109
}
109110

111+
/**
112+
* @group legacy
113+
*/
114+
public function testExitUserBasedOnSwitchUserRoleUpdatesToken()
115+
{
116+
$originalToken = new UsernamePasswordToken('username', '', 'key', array());
117+
$this->tokenStorage->setToken(new UsernamePasswordToken('username', '', 'key', array(new SwitchUserRole('ROLE_PREVIOUS', $originalToken, false)), $originalToken));
118+
119+
$this->request->query->set('_switch_user', SwitchUserListener::EXIT_VALUE);
120+
121+
$listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager);
122+
$listener->handle($this->event);
123+
124+
$this->assertSame($originalToken, $this->tokenStorage->getToken());
125+
}
126+
110127
public function testExitUserDispatchesEventWithRefreshedUser()
111128
{
112129
$originalUser = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock();
@@ -118,7 +135,7 @@ public function testExitUserDispatchesEventWithRefreshedUser()
118135
->with($originalUser)
119136
->willReturn($refreshedUser);
120137
$originalToken = new UsernamePasswordToken($originalUser, '', 'key');
121-
$this->tokenStorage->setToken(new UsernamePasswordToken('username', '', 'key', [new SwitchUserRole('ROLE_PREVIOUS', $originalToken)]));
138+
$this->tokenStorage->setToken(new SwitchUserToken('username', '', 'key', [new SwitchUserRole('ROLE_PREVIOUS', $originalToken, false)], $originalToken));
122139
$this->request->query->set('_switch_user', SwitchUserListener::EXIT_VALUE);
123140

124141
$dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock();
@@ -142,7 +159,7 @@ public function testExitUserDoesNotDispatchEventWithStringUser()
142159
->expects($this->never())
143160
->method('refreshUser');
144161
$originalToken = new UsernamePasswordToken($originalUser, '', 'key');
145-
$this->tokenStorage->setToken(new UsernamePasswordToken('username', '', 'key', [new SwitchUserRole('ROLE_PREVIOUS', $originalToken)]));
162+
$this->tokenStorage->setToken(new SwitchUserToken('username', '', 'key', [new SwitchUserRole('ROLE_PREVIOUS', $originalToken, false)], $originalToken));
146163
$this->request->query->set('_switch_user', SwitchUserListener::EXIT_VALUE);
147164

148165
$dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock();

‎Tests/Logout/LogoutUrlGeneratorTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public function testGuessFromCurrentFirewallContext()
9595

9696
public function testGuessFromTokenWithoutProviderKeyFallbacksToCurrentFirewall()
9797
{
98-
$this->tokenStorage->setToken($this->getMockBuilder(TokenInterface::class)->getMock());
98+
$this->tokenStorage->setToken(new UsernamePasswordToken('username', 'password', 'provider'));
9999
$this->generator->registerListener('secured_area', '/logout', null, null);
100100
$this->generator->setCurrentFirewall('secured_area');
101101

‎composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
],
1818
"require": {
1919
"php": "^7.1.3",
20-
"symfony/security-core": "~3.4|~4.0",
20+
"symfony/security-core": "~4.3",
2121
"symfony/event-dispatcher": "~3.4|~4.0",
2222
"symfony/http-foundation": "~3.4|~4.0",
2323
"symfony/http-kernel": "~3.4|~4.0",

0 commit comments

Comments
 (0)