Skip to content

[Security] deprecate the Role and SwitchUserRole classes #22048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 25, 2019
Merged

[Security] deprecate the Role and SwitchUserRole classes #22048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions UPGRADE-4.3.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions UPGRADE-5.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 7 additions & 1 deletion src/Symfony/Bridge/Monolog/Processor/TokenProcessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
];
}

Expand Down
3 changes: 1 addition & 2 deletions src/Symfony/Bridge/Monolog/Tests/Processor/TokenProcessorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
}
}
32 changes: 24 additions & 8 deletions src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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,
];
}
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
Expand Down
70 changes: 41 additions & 29 deletions src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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());
Expand All @@ -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']);
Expand All @@ -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());
Expand All @@ -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)
Expand All @@ -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();

Expand All @@ -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();
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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');

Expand All @@ -367,7 +395,7 @@ public function provideRoles()
[],
],
[
[new Role('ROLE_USER')],
[new Role('ROLE_USER', false)],
['ROLE_USER'],
[],
],
Expand All @@ -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'],
],
Expand All @@ -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();
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/SecurityBundle/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 7 additions & 0 deletions src/Symfony/Component/Security/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 8 additions & 2 deletions src/Symfony/Component/Security/Core/Authentication/Provider/UserAuthenticationProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down
Loading