Skip to content
Permalink
Browse files

feature #22048 [Security] deprecate the Role and SwitchUserRole class…

…es (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
-------

d7aaa61 deprecate the Role and SwitchUserRole classes
  • Loading branch information...
fabpot committed Feb 25, 2019
2 parents 3895acd + d7aaa61 commit d2e9a7051f84f3a110ec1b5e0adc9a735f224adb
Showing with 652 additions and 206 deletions.
  1. +7 −0 UPGRADE-4.3.md
  2. +5 −0 UPGRADE-5.0.md
  3. +7 −1 src/Symfony/Bridge/Monolog/Processor/TokenProcessor.php
  4. +1 −2 src/Symfony/Bridge/Monolog/Tests/Processor/TokenProcessorTest.php
  5. +24 −8 src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php
  6. +1 −0 src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
  7. +41 −29 src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php
  8. +1 −1 src/Symfony/Bundle/SecurityBundle/composer.json
  9. +7 −0 src/Symfony/Component/Security/CHANGELOG.md
  10. +8 −2 src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php
  11. +15 −4 src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php
  12. +1 −3 src/Symfony/Component/Security/Core/Authentication/Token/AnonymousToken.php
  13. +4 −6 src/Symfony/Component/Security/Core/Authentication/Token/PreAuthenticatedToken.php
  14. +4 −0 src/Symfony/Component/Security/Core/Authentication/Token/Storage/TokenStorage.php
  15. +60 −0 src/Symfony/Component/Security/Core/Authentication/Token/SwitchUserToken.php
  16. +4 −0 src/Symfony/Component/Security/Core/Authentication/Token/TokenInterface.php
  17. +4 −6 src/Symfony/Component/Security/Core/Authentication/Token/UsernamePasswordToken.php
  18. +22 −4 src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php
  19. +18 −1 src/Symfony/Component/Security/Core/Authorization/Voter/RoleHierarchyVoter.php
  20. +8 −2 src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php
  21. +11 −0 src/Symfony/Component/Security/Core/Role/Role.php
  22. +33 −0 src/Symfony/Component/Security/Core/Role/RoleHierarchy.php
  23. +2 −0 src/Symfony/Component/Security/Core/Role/RoleHierarchyInterface.php
  24. +16 −1 src/Symfony/Component/Security/Core/Role/SwitchUserRole.php
  25. +1 −1 ...ponent/Security/Core/Tests/Authentication/Provider/PreAuthenticatedAuthenticationProviderTest.php
  26. +1 −2 ...ny/Component/Security/Core/Tests/Authentication/Provider/RememberMeAuthenticationProviderTest.php
  27. +34 −3 src/Symfony/Component/Security/Core/Tests/Authentication/Provider/UserAuthenticationProviderTest.php
  28. +76 −74 src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php
  29. +1 −2 src/Symfony/Component/Security/Core/Tests/Authentication/Token/AnonymousTokenTest.php
  30. +1 −2 src/Symfony/Component/Security/Core/Tests/Authentication/Token/PreAuthenticatedTokenTest.php
  31. +1 −2 src/Symfony/Component/Security/Core/Tests/Authentication/Token/RememberMeTokenTest.php
  32. +2 −1 src/Symfony/Component/Security/Core/Tests/Authentication/Token/Storage/TokenStorageTest.php
  33. +41 −0 src/Symfony/Component/Security/Core/Tests/Authentication/Token/SwitchUserTokenTest.php
  34. +1 −2 src/Symfony/Component/Security/Core/Tests/Authentication/Token/UsernamePasswordTokenTest.php
  35. +4 −7 src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php
  36. +25 −0 src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php
  37. +33 −0 src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleHierarchyVoterTest.php
  38. +39 −0 src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php
  39. +17 −0 src/Symfony/Component/Security/Core/Tests/Role/RoleHierarchyTest.php
  40. +3 −0 src/Symfony/Component/Security/Core/Tests/Role/RoleTest.php
  41. +3 −0 src/Symfony/Component/Security/Core/Tests/Role/SwitchUserRoleTest.php
  42. +3 −4 src/Symfony/Component/Security/Guard/Token/PostAuthenticationGuardToken.php
  43. +9 −4 src/Symfony/Component/Security/Http/Firewall/ContextListener.php
  44. +12 −13 src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php
  45. +5 −6 src/Symfony/Component/Security/Http/Tests/Controller/UserValueResolverTest.php
  46. +20 −3 src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php
  47. +1 −1 src/Symfony/Component/Security/Http/Tests/Logout/LogoutUrlGeneratorTest.php
  48. +1 −1 src/Symfony/Component/Security/Http/composer.json
  49. +12 −6 src/Symfony/Component/Workflow/EventListener/GuardListener.php
  50. +2 −2 src/Symfony/Component/Workflow/Tests/EventListener/GuardListenerTest.php
@@ -53,6 +53,13 @@ HttpFoundation
Security
--------

* The `Role` and `SwitchUserRole` classes are deprecated and will be removed in 5.0. Use strings for roles
instead.
* The `RoleHierarchyInterface` is deprecated and will be removed in 5.0.
* The `getReachableRoles()` method of the `RoleHierarchy` class is deprecated and will be removed in 5.0.
Use the `getReachableRoleNames()` method instead.
* The `getRoles()` method of the `TokenInterface` is deprecated. Tokens must implement the `getRoleNames()`
method instead and return roles as strings.
* The `AbstractToken::serialize()`, `AbstractToken::unserialize()`,
`AuthenticationException::serialize()` and `AuthenticationException::unserialize()`
methods are now final, use `getState()` and `setState()` instead.
@@ -226,6 +226,11 @@ Process
Security
--------

* The `Role` and `SwitchUserRole` classes have been removed.
* The `RoleHierarchyInterface` has been removed.
* The `getReachableRoles()` method of the `RoleHierarchy` class has been removed.
* The `getRoles()` method has been removed from the `TokenInterface`. It has been replaced by the new
`getRoleNames()` method.
* The `ContextListener::setLogoutOnUserChange()` method has been removed.
* The `Symfony\Component\Security\Core\User\AdvancedUserInterface` has been removed.
* The `ExpressionVoter::addExpressionLanguageProvider()` method has been removed.
@@ -31,10 +31,16 @@ public function __invoke(array $records)
{
$records['extra']['token'] = null;
if (null !== $token = $this->tokenStorage->getToken()) {
if (method_exists($token, 'getRoleNames')) {
$roles = $token->getRoleNames();
} else {
$roles = array_map(function ($role) { return $role->getRole(); }, $token->getRoles(false));
}
$records['extra']['token'] = [
'username' => $token->getUsername(),
'authenticated' => $token->isAuthenticated(),
'roles' => array_map(function ($role) { return $role->getRole(); }, $token->getRoles()),
'roles' => $roles,
];
}
@@ -36,7 +36,6 @@ public function testProcessor()
$this->assertArrayHasKey('token', $record['extra']);
$this->assertEquals($token->getUsername(), $record['extra']['token']['username']);
$this->assertEquals($token->isAuthenticated(), $record['extra']['token']['authenticated']);
$roles = array_map(function ($role) { return $role->getRole(); }, $token->getRoles());
$this->assertEquals($roles, $record['extra']['token']['roles']);
$this->assertEquals(['ROLE_USER'], $record['extra']['token']['roles']);
}
}
@@ -18,10 +18,12 @@
use Symfony\Component\HttpKernel\DataCollector\DataCollector;
use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager;
use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter;
use Symfony\Component\Security\Core\Role\Role;
use Symfony\Component\Security\Core\Role\RoleHierarchy;
use Symfony\Component\Security\Core\Role\RoleHierarchyInterface;
use Symfony\Component\Security\Core\Role\SwitchUserRole;
use Symfony\Component\Security\Http\Firewall\SwitchUserListener;
@@ -91,18 +93,32 @@ public function collect(Request $request, Response $response, \Exception $except
];
} else {
$inheritedRoles = [];
$assignedRoles = $token->getRoles();
if (method_exists($token, 'getRoleNames')) {
$assignedRoles = $token->getRoleNames();
} else {
$assignedRoles = array_map(function (Role $role) { return $role->getRole(); }, $token->getRoles(false));
}
$impersonatorUser = null;
foreach ($assignedRoles as $role) {
if ($role instanceof SwitchUserRole) {
$impersonatorUser = $role->getSource()->getUsername();
break;
if ($token instanceof SwitchUserToken) {
$impersonatorUser = $token->getOriginalToken()->getUsername();
} else {
foreach ($token->getRoles(false) as $role) {
if ($role instanceof SwitchUserRole) {
$impersonatorUser = $role->getSource()->getUsername();
break;
}
}
}
if (null !== $this->roleHierarchy) {
$allRoles = $this->roleHierarchy->getReachableRoles($assignedRoles);
if ($this->roleHierarchy instanceof RoleHierarchy) {
$allRoles = $this->roleHierarchy->getReachableRoleNames($assignedRoles);
} else {
$allRoles = array_map(function (Role $role) { return (string) $role; }, $this->roleHierarchy->getReachableRoles($token->getRoles(false)));
}
foreach ($allRoles as $role) {
if (!\in_array($role, $assignedRoles, true)) {
$inheritedRoles[] = $role;
@@ -129,8 +145,8 @@ public function collect(Request $request, Response $response, \Exception $except
'token_class' => $this->hasVarDumper ? new ClassStub(\get_class($token)) : \get_class($token),
'logout_url' => $logoutUrl,
'user' => $token->getUsername(),
'roles' => array_map(function (Role $role) { return $role->getRole(); }, $assignedRoles),
'inherited_roles' => array_unique(array_map(function (Role $role) { return $role->getRole(); }, $inheritedRoles)),
'roles' => $assignedRoles,
'inherited_roles' => array_unique($inheritedRoles),
'supports_role_hierarchy' => null !== $this->roleHierarchy,
];
}
@@ -98,6 +98,7 @@
<argument>%security.role_hierarchy.roles%</argument>
</service>
<service id="Symfony\Component\Security\Core\Role\RoleHierarchyInterface" alias="security.role_hierarchy" />
<service id="Symfony\Component\Security\Core\Role\RoleHierarchy" alias="security.role_hierarchy" />


<!-- Security Voters -->
@@ -18,9 +18,12 @@
use Symfony\Bundle\SecurityBundle\Security\FirewallMap;
use Symfony\Component\EventDispatcher\EventDispatcher;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\HttpKernelInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager;
@@ -38,7 +41,7 @@ class SecurityDataCollectorTest extends TestCase
public function testCollectWhenSecurityIsDisabled()
{
$collector = new SecurityDataCollector();
$collector->collect($this->getRequest(), $this->getResponse());
$collector->collect(new Request(), new Response());
$this->assertSame('security', $collector->getName());
$this->assertFalse($collector->isEnabled());
@@ -58,7 +61,7 @@ public function testCollectWhenAuthenticationTokenIsNull()
{
$tokenStorage = new TokenStorage();
$collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
$collector->collect($this->getRequest(), $this->getResponse());
$collector->collect(new Request(), new Response());
$this->assertTrue($collector->isEnabled());
$this->assertFalse($collector->isAuthenticated());
@@ -80,7 +83,7 @@ public function testCollectAuthenticationTokenAndRoles(array $roles, array $norm
$tokenStorage->setToken(new UsernamePasswordToken('hhamon', 'P4$$w0rD', 'provider', $roles));
$collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
$collector->collect($this->getRequest(), $this->getResponse());
$collector->collect(new Request(), new Response());
$collector->lateCollect();
$this->assertTrue($collector->isEnabled());
@@ -95,6 +98,9 @@ public function testCollectAuthenticationTokenAndRoles(array $roles, array $norm
$this->assertSame('hhamon', $collector->getUser());
}
/**
* @group legacy
*/
public function testCollectImpersonatedToken()
{
$adminToken = new UsernamePasswordToken('yceruto', 'P4$$w0rD', 'provider', ['ROLE_ADMIN']);
@@ -108,7 +114,7 @@ public function testCollectImpersonatedToken()
$tokenStorage->setToken(new UsernamePasswordToken('hhamon', 'P4$$w0rD', 'provider', $userRoles));
$collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
$collector->collect($this->getRequest(), $this->getResponse());
$collector->collect(new Request(), new Response());
$collector->lateCollect();
$this->assertTrue($collector->isEnabled());
@@ -122,10 +128,32 @@ public function testCollectImpersonatedToken()
$this->assertSame('hhamon', $collector->getUser());
}
public function testCollectSwitchUserToken()
{
$adminToken = new UsernamePasswordToken('yceruto', 'P4$$w0rD', 'provider', ['ROLE_ADMIN']);
$tokenStorage = new TokenStorage();
$tokenStorage->setToken(new SwitchUserToken('hhamon', 'P4$$w0rD', 'provider', ['ROLE_USER', 'ROLE_PREVIOUS_ADMIN'], $adminToken));
$collector = new SecurityDataCollector($tokenStorage, $this->getRoleHierarchy());
$collector->collect(new Request(), new Response());
$collector->lateCollect();
$this->assertTrue($collector->isEnabled());
$this->assertTrue($collector->isAuthenticated());
$this->assertTrue($collector->isImpersonated());
$this->assertSame('yceruto', $collector->getImpersonatorUser());
$this->assertSame(SwitchUserToken::class, $collector->getTokenClass()->getValue());
$this->assertTrue($collector->supportsRoleHierarchy());
$this->assertSame(['ROLE_USER', 'ROLE_PREVIOUS_ADMIN'], $collector->getRoles()->getValue(true));
$this->assertSame([], $collector->getInheritedRoles()->getValue(true));
$this->assertSame('hhamon', $collector->getUser());
}
public function testGetFirewall()
{
$firewallConfig = new FirewallConfig('dummy', 'security.request_matcher.dummy', 'security.user_checker.dummy');
$request = $this->getRequest();
$request = new Request();
$firewallMap = $this
->getMockBuilder(FirewallMap::class)
@@ -138,7 +166,7 @@ public function testGetFirewall()
->willReturn($firewallConfig);
$collector = new SecurityDataCollector(null, null, null, null, $firewallMap, new TraceableFirewallListener($firewallMap, new EventDispatcher(), new LogoutUrlGenerator()));
$collector->collect($request, $this->getResponse());
$collector->collect($request, new Response());
$collector->lateCollect();
$collected = $collector->getFirewall();
@@ -158,8 +186,8 @@ public function testGetFirewall()
public function testGetFirewallReturnsNull()
{
$request = $this->getRequest();
$response = $this->getResponse();
$request = new Request();
$response = new Response();
// Don't inject any firewall map
$collector = new SecurityDataCollector();
@@ -192,9 +220,9 @@ public function testGetFirewallReturnsNull()
*/
public function testGetListeners()
{
$request = $this->getRequest();
$request = new Request();
$event = new GetResponseEvent($this->getMockBuilder(HttpKernelInterface::class)->getMock(), $request, HttpKernelInterface::MASTER_REQUEST);
$event->setResponse($response = $this->getResponse());
$event->setResponse($response = new Response());
$listener = $this->getMockBuilder(ListenerInterface::class)->getMock();
$listener
->expects($this->once())
@@ -345,7 +373,7 @@ public function testCollectDecisionLog(string $strategy, array $decisionLog, arr
->willReturn($decisionLog);
$dataCollector = new SecurityDataCollector(null, null, null, $accessDecisionManager);
$dataCollector->collect($this->getRequest(), $this->getResponse());
$dataCollector->collect(new Request(), new Response());
$this->assertEquals($dataCollector->getAccessDecisionLog(), $expectedDecisionLog, 'Wrong value returned by getAccessDecisionLog');
@@ -367,7 +395,7 @@ public function provideRoles()
[],
],
[
[new Role('ROLE_USER')],
[new Role('ROLE_USER', false)],
['ROLE_USER'],
[],
],
@@ -378,7 +406,7 @@ public function provideRoles()
['ROLE_USER', 'ROLE_ALLOWED_TO_SWITCH'],
],
[
[new Role('ROLE_ADMIN')],
[new Role('ROLE_ADMIN', false)],
['ROLE_ADMIN'],
['ROLE_USER', 'ROLE_ALLOWED_TO_SWITCH'],
],
@@ -397,20 +425,4 @@ private function getRoleHierarchy()
'ROLE_OPERATOR' => ['ROLE_USER'],
]);
}
private function getRequest()
{
return $this
->getMockBuilder('Symfony\Component\HttpFoundation\Request')
->disableOriginalConstructor()
->getMock();
}
private function getResponse()
{
return $this
->getMockBuilder('Symfony\Component\HttpFoundation\Response')
->disableOriginalConstructor()
->getMock();
}
}
@@ -21,7 +21,7 @@
"symfony/config": "^4.2",
"symfony/dependency-injection": "^4.2",
"symfony/http-kernel": "^4.1",
"symfony/security-core": "~4.2",
"symfony/security-core": "~4.3",
"symfony/security-csrf": "~4.2",
"symfony/security-guard": "~4.2",
"symfony/security-http": "~4.2"
@@ -4,6 +4,13 @@ CHANGELOG
4.3.0
-----

* The `Role` and `SwitchUserRole` classes are deprecated and will be removed in 5.0. Use strings for roles
instead.
* The `RoleHierarchyInterface` is deprecated and will be removed in 5.0.
* The `getReachableRoles()` method of the `RoleHierarchy` class is deprecated and will be removed in 5.0.
Use the `getReachableRoleNames()` method instead.
* The `getRoles()` method of the `TokenInterface` is deprecated. Tokens must implement the `getRoleNames()`
method instead and return roles as strings.
* Made the `serialize()` and `unserialize()` methods of `AbstractToken` and
`AuthenticationException` final, use `getState()`/`setState()` instead

@@ -11,6 +11,7 @@
namespace Symfony\Component\Security\Core\Authentication\Provider;
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
@@ -87,7 +88,12 @@ public function authenticate(TokenInterface $token)
throw $e;
}
$authenticatedToken = new UsernamePasswordToken($user, $token->getCredentials(), $this->providerKey, $this->getRoles($user, $token));
if ($token instanceof SwitchUserToken) {
$authenticatedToken = new SwitchUserToken($user, $token->getCredentials(), $this->providerKey, $this->getRoles($user, $token), $token->getOriginalToken());
} else {
$authenticatedToken = new UsernamePasswordToken($user, $token->getCredentials(), $this->providerKey, $this->getRoles($user, $token));
}
$authenticatedToken->setAttributes($token->getAttributes());
return $authenticatedToken;
@@ -110,7 +116,7 @@ private function getRoles(UserInterface $user, TokenInterface $token)
{
$roles = $user->getRoles();
foreach ($token->getRoles() as $role) {
foreach ($token->getRoles(false) as $role) {
if ($role instanceof SwitchUserRole) {
$roles[] = $role;
Oops, something went wrong.

0 comments on commit d2e9a70

Please sign in to comment.
You can’t perform that action at this time.