feature #35858 [Security] Deprecated ROLE_PREVIOUS_ADMIN (wouterj)
This PR was merged into the 5.1-dev branch.
Discussion
----------
[Security] Deprecated ROLE_PREVIOUS_ADMIN
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| Deprecations? | yes
| Tickets | n/a
| License | MIT
| Doc PR | https://github.com/symfony/symfony-docs/pull/11487
`ROLE_PREVIOUS_ADMIN` is added to the token roles if the session is an impersonation. Since https://github.com/symfony/symfony/pull/31189 we have the `IS_IMPERSONATOR` attribute which can be used for the same reason. I propose to deprecate the `ROLE_PREVIOUS_ADMIN`:
* This is not what roles are for ([resulting in hacking this exception in `AbstractToken`](https://github.com/symfony/symfony/blob/5.0/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php#L275-L277))
* The role isn't very descriptive
* I don't like having 2 ways of doing exactly the same thing
* While every application with impersonation enabled probably needs to be updated, the update is as simple as replacing `ROLE_PREVIOUS_ADMIN` with `IS_IMPERSONATOR`: `find ./ -type f -exec sed -i 's/ROLE_PREVIOUS_ADMIN/IS_IMPERSONATOR/g' {} +`
---
I'm a bit unsure on how to deprecate this role, but I think having it in `RoleVoter` is probably the safest (`isGranted()` and variants + `AccessDecisionManager#decide()` all use this voter to check if the token has this role).
Commits
-------
dce55f352a
Deprecated ROLE_PREVIOUS_ADMIN
This commit is contained in:
commit
82db995908
|
@ -40,6 +40,10 @@ class RoleVoter implements VoterInterface
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if ('ROLE_PREVIOUS_ADMIN' === $attribute) {
|
||||||
|
trigger_deprecation('symfony/security-core', '5.1', 'The ROLE_PREVIOUS_ADMIN role is deprecated and will be removed in version 6.0, use the IS_IMPERSONATOR attribute instead.');
|
||||||
|
}
|
||||||
|
|
||||||
$result = VoterInterface::ACCESS_DENIED;
|
$result = VoterInterface::ACCESS_DENIED;
|
||||||
foreach ($roles as $role) {
|
foreach ($roles as $role) {
|
||||||
if ($attribute === $role) {
|
if ($attribute === $role) {
|
||||||
|
|
|
@ -44,6 +44,17 @@ class RoleVoterTest extends TestCase
|
||||||
];
|
];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @group legacy
|
||||||
|
* @expectedDeprecation Since symfony/security-core 5.1: The ROLE_PREVIOUS_ADMIN role is deprecated and will be removed in version 6.0, use the IS_IMPERSONATOR attribute instead.
|
||||||
|
*/
|
||||||
|
public function testDeprecatedRolePreviousAdmin()
|
||||||
|
{
|
||||||
|
$voter = new RoleVoter();
|
||||||
|
|
||||||
|
$voter->vote($this->getTokenWithRoleNames(['ROLE_USER', 'ROLE_PREVIOUS_ADMIN']), null, ['ROLE_PREVIOUS_ADMIN']);
|
||||||
|
}
|
||||||
|
|
||||||
protected function getTokenWithRoleNames(array $roles)
|
protected function getTokenWithRoleNames(array $roles)
|
||||||
{
|
{
|
||||||
$token = $this->getMockBuilder(AbstractToken::class)->getMock();
|
$token = $this->getMockBuilder(AbstractToken::class)->getMock();
|
||||||
|
|
|
@ -18,7 +18,8 @@
|
||||||
"require": {
|
"require": {
|
||||||
"php": "^7.2.5",
|
"php": "^7.2.5",
|
||||||
"symfony/event-dispatcher-contracts": "^1.1|^2",
|
"symfony/event-dispatcher-contracts": "^1.1|^2",
|
||||||
"symfony/service-contracts": "^1.1.6|^2"
|
"symfony/service-contracts": "^1.1.6|^2",
|
||||||
|
"symfony/deprecation-contracts": "^2.1"
|
||||||
},
|
},
|
||||||
"require-dev": {
|
"require-dev": {
|
||||||
"psr/container": "^1.0",
|
"psr/container": "^1.0",
|
||||||
|
|
Reference in New Issue