8000 feature #27305 [Security/Core] Add "is_granted()" to security express… · symfony/symfony@ec6d46c · GitHub
[go: up one dir, main page]
Skip to content

Commit ec6d46c

Browse files
fabpotfabpot
committed
feature #27305 [Security/Core] Add "is_granted()" to security expressions, deprecate "has_role()" (nicolas-grekas)
This PR was merged into the 4.2-dev branch. Discussion ---------- [Security/Core] Add "is_granted()" to security expressions, deprecate "has_role()" | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #23084 | License | MIT | Doc PR | - Because `has_role()` doesn't use the auth checker, it is confusing. Let's move `is_granted()` to core (it's provided by SensioFrameworkExtraBundle / ApiPlatform for now. Commits ------- 9dbf399 [Security/Core] Add "is_granted()" to security expressions, deprecate "has_role()"
2 parents 2ae7ad9 + 9dbf399 commit ec6d46c

File tree

10 files changed

+91
-15
lines changed

10 files changed

+91
-15
lines changed

‎UPGRADE-4.2.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
UPGRADE FROM 4.1 to 4.2
2+
=======================
3+
4+
Security
5+
--------
6+
7+
* Using the `has_role()` function in security expressions is deprecated, use the `is_granted()` function instead.

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,7 @@ private function addWorkflowSection(ArrayNodeDefinition $rootNode)
370370
->scalarNode('guard')
371371
->cannotBeEmpty()
372372
->info('An expression to block the transition')
373-
->example('is_fully_authenticated() and has_role(\'ROLE_JOURNALIST\') and subject.getTitle() == \'My first article\'')
373+
->example('is_fully_authenticated() and is_granted(\'ROLE_JOURNALIST\') and subject.getTitle() == \'My first article\'')
374374
->end()
375375
->arrayNode('from')
376376
->beforeNormalization()

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@
119119
<service id="security.access.expression_voter" class="Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter">
120120
<argument type="service" id="security.expression_language" />
121121
<argument type="service" id="security.authentication.trust_resolver" />
122+
<argument type="service" id="security.authorization_checker" />
122123
<argument type="service" id="security.role_hierarchy" on-invalid="null" />
123124
<tag name="security.voter" priority="245" />
124125
</service>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,5 @@ security:
4040
- { path: ^/secured-by-one-ip$, ip: 10.10.10.10, roles: IS_AUTHENTICATED_ANONYMOUSLY }
4141
- { path: ^/secured-by-two-ips$, ips: [1.1.1.1, 2.2.2.2], roles: IS_AUTHENTICATED_ANONYMOUSLY }
4242
- { path: ^/highly_protected_resource$, roles: IS_ADMIN }
43-
- { path: ^/protected-via-expression$, allow_if: "(is_anonymous() and request.headers.get('user-agent') matches '/Firefox/i') or has_role('ROLE_USER')" }
43+
- { path: ^/protected-via-expression$, allow_if: "(is_anonymous() and request.headers.get('user-agent') matches '/Firefox/i') or is_granted('ROLE_USER')" }
4444
- { path: .*, roles: IS_AUTHENTICATED_FULLY }

‎src/Symfony/Bundle/SecurityBundle/composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
"require": {
1919
"php": "^7.1.3",
2020
"ext-xml": "*",
21-
"symfony/security": "~4.1",
21+
"symfony/security": "~4.2",
2222
"symfony/dependency-injection": "^3.4.3|^4.0.3",
2323
"symfony/http-kernel": "^4.1"
2424
},

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
CHANGELOG
22
=========
33

4+
4.2.0
5+
-----
6+
7+
* added the `is_granted()` function in security expressions
8+
* deprecated the `has_role()` function in security expressions, use `is_granted()` instead
9+
410
4.1.0
511
-----
612

‎src/Symfony/Component/Security/Core/Authorization/ExpressionLanguageProvider.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,25 @@ public function getFunctions()
4242
return $variables['trust_resolver']->isFullFledged($variables['token']);
4343
}),
4444

45+
new ExpressionFunction('is_granted', function ($attributes, $object = 'null') {
46+
return sprintf('$auth_checker->isGranted(%s, %s)', $attributes, $object);
47+
}, function (array $variables, $attributes, $object = null) {
48+
return $variables['auth_checker']->isGranted($attributes, $object);
49+
}),
50+
4551
new ExpressionFunction('is_remember_me', function () {
4652
return '$trust_resolver->isRememberMe($token)';
4753
}, function (array $variables) {
4854
return $variables['trust_resolver']->isRememberMe($variables['token']);
4955
}),
5056

5157
new ExpressionFunction('has_role', function ($role) {
58+
@trigger_error('Using the "has_role()" function in security expressions is deprecated since Symfony 4.2, use "is_granted()" instead.', E_USER_DEPRECATED);
59+
5260
return sprintf('in_array(%s, $roles)', $role);
5361
}, function (array $variables, $role) {
62+
@trigger_error('Using the "has_role()" function in security expressions is deprecated since Symfony 4.2, use "is_granted()" instead.', E_USER_DEPRECATED);
63+
5464
return in_array($role, $variables['roles']);
5565
}),
5666
);

‎src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1515
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
16+
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
1617
use Symfony\Component\Security\Core\Authorization\ExpressionLanguage;
1718
use Symfony\Component\Security\Core\Role\RoleHierarchyInterface;
1819
use Symfony\Component\ExpressionLanguage\ExpressionFunctionProviderInterface;
@@ -28,12 +29,27 @@ class ExpressionVoter implements VoterInterface
2829
{
2930
private $expressionLanguage;
3031
private $trustResolver;
32+
private $authChecker;
3133
private $roleHierarchy;
3234

33-
public function __construct(ExpressionLanguage $expressionLanguage, AuthenticationTrustResolverInterface $trustResolver, RoleHierarchyInterface $roleHierarchy = null)
35+
/**
36+
* @param AuthorizationCheckerInterface $authChecker
37+
*/
38+
public function __construct(ExpressionLanguage $expressionLanguage, AuthenticationTrustResolverInterface $trustResolver, $authChecker = null, RoleHierarchyInterface $roleHierarchy = null)
3439
{
40+
if ($authChecker instanceof RoleHierarchyInterface) {
41+
@trigger_error(sprintf('Passing a RoleHierarchyInterface to "%s()" is deprecated since Symfony 4.2. Pass an AuthorizationCheckerInterface instead.', __METHOD__), E_USER_DEPRECATED);
42+
$roleHierarchy = $authChecker;
43+
$authChecker = null;
44+
} elseif (null === $authChecker) {
45+
@trigger_error(sprintf('Argument 3 passed to "%s()" should be an instanceof AuthorizationCheckerInterface, not passing it is deprecated since Symfony 4.2.', __METHOD__), E_USER_DEPRECATED);
46+
} elseif (!$authChecker instanceof AuthorizationCheckerInterface) {
47+
throw new \InvalidArgumentException(sprintf('Argument 3 passed to %s() must be an instance of %s or null, %s given.', __METHOD__, AuthorizationCheckerInterface::class, is_object($authChecker) ? get_class($authChecker) : gettype($authChecker)));
48+
}
49+
3550
$this->expressionLanguage = $expressionLanguage;
3651
$this->trustResolver = $trustResolver;
52+
$this->authChecker = $authChecker;
3753
$this->roleHierarchy = $roleHierarchy;
3854
}
3955

@@ -87,6 +103,7 @@ private function getVariables(TokenInterface $token, $subject)
87103
'subject' => $subject,
88104
'roles' => array_map(function ($role) { return $role->getRole(); }, $roles),
89105
'trust_resolver' => $this->trustResolver,
106+
'auth_checker' => $this->authChecker,
90107
);
91108

92109
// this is mainly to propose a better experience when the expression is used

‎src/Symfony/Component/Security/Core/Tests/Authorization/ExpressionLanguageTest.php‎

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,15 @@
1212
namespace Symfony\Component\Security\Core\Tests\Authorization;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
16+
use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
1517
use Symfony\Component\Security\Core\Authorization\ExpressionLanguage;
18+
use Symfony\Component\Security\Core\Authorization\Voter\RoleVoter;
19+
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
1620
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver;
1721
use Symfony\Component\Security\Core\Authentication\Token\AnonymousToken;
1822
use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken;
23+
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
1924
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2025
use Symfony\Component\Security\Core\User\User;
2126

@@ -24,17 +29,21 @@ class ExpressionLanguageTest extends TestCase
2429
/**
2530
* @dataProvider provider
2631
*/
27-
public function testIsAuthenticated($token, $expression, $result, array $roles = array())
32+
public function testIsAuthenticated($token, $expression, $result)
2833
{
2934
$anonymousTokenClass = 'Symfony\\Component\\Security\\Core\\Authentication\\Token\\AnonymousToken';
3035
$rememberMeTokenClass = 'Symfony\\Component\\Security\\Core\\Authentication\\Token\\RememberMeToken';
3136
$expressionLanguage = new ExpressionLanguage();
3237
$trustResolver = new AuthenticationTrustResolver($anonymousTokenClass, $rememberMeTokenClass);
38+
$tokenStorage = new TokenStorage();
39+
$tokenStorage->setToken($token);
40+
$accessDecisionManager = new AccessDecisionManager(array(new RoleVoter()));
41+
$authChecker = new AuthorizationChecker($tokenStorage, $this->getMockBuilder(AuthenticationManagerInterface::class)->getMock(), $accessDecisionManager);
3342

3443
$context = array();
3544
$context['trust_resolver'] = $trustResolver;
45+
$context['auth_checker'] = $authChecker;
3646
$context['token'] = $token;
37-
$context['roles'] = $roles;
3847

3948
$this->assertEquals($result, $expressionLanguage->evaluate($expression, $context));
4049
}
@@ -54,27 +63,52 @@ public function provider()
5463
array($noToken, 'is_authenticated()', false),
5564
array($noToken, 'is_fully_authenticated()', false),
5665
array($noToken, 'is_remember_me()', false),
57-
array($noToken, "has_role('ROLE_USER')", false),
5866

5967
array($anonymousToken, 'is_anonymous()', true),
6068
array($anonymousToken, 'is_authenticated()', false),
6169
array($anonymousToken, 'is_fully_authenticated()', false),
6270
array($anonymousToken, 'is_remember_me()', false),
63-
array($anonymousToken, "has_role('ROLE_USER')", false),
71+
array($anonymousToken, "is_granted('ROLE_USER')", false),
6472

6573
array($rememberMeToken, 'is_anonymous()', false),
6674
array($rememberMeToken, 'is_authenticated()', true),
6775
array($rememberMeToken, 'is_fully_authenticated()', false),
6876
array($rememberMeToken, 'is_remember_me()', true),
69-
array($rememberMeToken, "has_role('ROLE_FOO')", false, $roles),
70-
array($rememberMeToken, "has_role('ROLE_USER')", true, $roles),
77+
array($rememberMeToken, "is_granted('ROLE_FOO')", false),
78+
array($rememberMeToken, "is_granted('ROLE_USER')", true),
7179

7280
array($usernamePasswordToken, 'is_anonymous()', false),
7381
array($usernamePasswordToken, 'is_authenticated()', true),
7482
array($usernamePasswordToken, 'is_fully_authenticated()', true),
7583
array($usernamePasswordToken, 'is_remember_me()', false),
76-
array($usernamePasswordToken, "has_role('ROLE_FOO')", false, $roles),
77-
array($usernamePasswordToken, "has_role('ROLE_USER')", true, $roles),
84+
array($usernamePasswordToken, "is_granted('ROLE_FOO')", false),
85+
array($usernamePasswordToken, "is_granted('ROLE_USER')", true),
86+
);
87+
}
88+
89+
/**
90+
* @dataProvider provideLegacyHasRole
91+
* @group legacy
92+
*/
93+
public function testLegacyHasRole($expression, $result, $roles = array())
94+
{
95+
$expressionLanguage = new ExpressionLanguage();
96+
$context = array('roles' => $roles);
97+
98+
$this->assertEquals($result, $expressionLanguage->evaluate($expression, $context));
99+
}
100+
101+
public function provideLegacyHasRole()
102+
{
103+
$roles = array('ROLE_USER', 'ROLE_ADMIN');
104+
105+
return array(
106+
array("has_role('ROLE_FOO')", false),
107+
array("has_role('ROLE_USER')", false),
108+
array("has_role('ROLE_ADMIN')", false),
109+
array("has_role('ROLE_FOO')", false, $roles),
110+
array("has_role('ROLE_USER')", true, $roles),
111+
array("has_role('ROLE_ADMIN')", true, $roles),
78112
);
79113
}
80114
}

‎src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Security\Core\Tests\Authorization\Voter;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
1516
use Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter;
1617
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
1718
use Sy 8B92 mfony\Component\Security\Core\Role\Role;
@@ -23,7 +24,7 @@ class ExpressionVoterTest extends TestCase
2324
*/
2425
public function testVote($roles, $attributes, $expected, $tokenExpectsGetRoles = true, $expressionLanguageExpectsEvaluate = true)
2526
{
26-
$voter = new ExpressionVoter($this->createExpressionLanguage($expressionLanguageExpectsEvaluate), $this->createTrustResolver());
27+
$voter = new ExpressionVoter($this->createExpressionLanguage($expressionLanguageExpectsEvaluate), $this->createTrustResolver(), $this->createAuthorizationChecker());
2728

2829
$this->assertSame($expected, $voter->vote($this->getToken($roles, $tokenExpectsGetRoles), null, $attributes));
2930
}
@@ -75,9 +76,9 @@ protected function createTrustResolver()
7576
return $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface')->getMock();
7677
}
7778

78-
protected function createRoleHierarchy()
79+
protected function createAuthorizationChecker()
7980
{
80-
return $this->getMockBuilder('Symfony\Component\Security\Core\Role\RoleHierarchyInterface')->getMock();
81+
return $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
8182
}
8283

8384
protected function createExpression()

0 commit comments

Comments
 (0)
0