8000 [Security] Merge UserAuthorizationCheckerInterface into Authorization… · symfony/symfony@d4a5ea2 · GitHub
[go: up one dir, main page]
Skip to content

Commit d4a5ea2

Browse files
nicolas-grekasnicolas-grekas
committed
[Security] Merge UserAuthorizationCheckerInterface into AuthorizationCheckerInterface
1 parent f48e068 commit d4a5ea2

16 files changed

+136
-248
lines changed

‎UPGRADE-7.3.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Ldap
1616
Security
1717
--------
1818

19+
* Add `AuthorizationCheckerInterface::isGrantedForUser()` to test user authorization without relying on the session
1920
* Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`;
2021
erase credentials e.g. using `__serialize()` instead
2122

‎src/Symfony/Bridge/Twig/Extension/SecurityExtension.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use Symfony\Component\Security\Acl\Voter\FieldVote;
1515
use Symfony\Component\Security\Core\Authorization\AccessDecision;
1616
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
17-
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
1817
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
1918
use Symfony\Component\Security\Core\User\UserInterface;
2019
use Symfony\Component\Security\Http\Impersonate\ImpersonateUrlGenerator;
@@ -31,7 +30,6 @@ final class SecurityExtension extends AbstractExtension
3130
public function __construct(
3231
private ?AuthorizationCheckerInterface $securityChecker = null,
3332
private ?ImpersonateUrlGenerator $impersonateUrlGenerator = null,
34-
private ?UserAuthorizationCheckerInterface $userSecurityChecker = null,
3533
) {
3634
}
3735

@@ -58,8 +56,8 @@ public function isGranted(mixed $role, mixed $object = null, ?string $field = nu
5856

5957
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null, ?AccessDecision $accessDecision = null): bool
6058
{
61-
if (!$this->userSecurityChecker) {
62-
throw new \LogicException(\sprintf('An instance of "%s" must be provided to use "%s()".', UserAuthorizationCheckerInterface::class, __METHOD__));
59+
if (null === $this->securityChecker) {
60+
return false;
6361
}
6462

6563
if (null !== $field) {
@@ -71,7 +69,7 @@ public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $s
7169
}
7270

7371
try {
74-
return $this->userSecurityChecker->isGrantedForUser($user, $attribute, $subject, $accessDecision);
72+
return $this->securityChecker->isGrantedForUser($user, $attribute, $subject, $accessDecision);
7573
} catch (AuthenticationCredentialsNotFoundException) {
7674
return false;
7775
}

‎src/Symfony/Bridge/Twig/Tests/Extension/SecurityExtensionTest.php

Lines changed: 58 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,23 @@
1515
use Symfony\Bridge\PhpUnit\ClassExistsMock;
1616
use Symfony\Bridge\Twig\Extension\SecurityExtension;
1717
use Symfony\Component\Security\Acl\Voter\FieldVote;
18+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
19+
use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
1820
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
19-
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
2021
use Symfony\Component\Security\Core\User\UserInterface;
2122

2223
class SecurityExtensionTest extends TestCase
2324
{
25+
public static function setUpBeforeClass(): void
26+
{
27+
ClassExistsMock::register(SecurityExtension::class);
28+
}
29+
30+
protected function tearDown(): void
31+
{
32+
ClassExistsMock::withMockedClasses([FieldVote::class => true]);
33+
}
34+
2435
/**
2536
* @dataProvider provideObjectFieldAclCases
2637
*/
@@ -39,17 +50,16 @@ public function testIsGrantedCreatesFieldVoteObjectWhenFieldNotNull($object, $fi
3950

4051
public function testIsGrantedThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist()
4152
{
42-
if (!class_exists(UserAuthorizationCheckerInterface::class)) {
53+
if (!method_exists(AuthorizationChecker::class, 'isGrantedForUser')) {
4354
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.');
4455
}
4556

4657
$securityChecker = $this->createMock(AuthorizationCheckerInterface::class);
4758

48-
ClassExistsMock::register(SecurityExtension::class);
4959
ClassExistsMock::withMockedClasses([FieldVote::class => false]);
5060

5161
$this->expectException(\LogicException::class);
52-
$this->expectExceptionMessageMatches('Passing a $field to the "is_granted()" function requires symfony/acl.');
62+
$this->expectExceptionMessage('Passing a $field to the "is_granted()" function requires symfony/acl.');
5363

5464
$securityExtension = new SecurityExtension($securityChecker);
5565
$securityExtension->isGranted('ROLE', 'object', 'bar');
@@ -60,38 +70,41 @@ public function testIsGrantedThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist
6070
*/
6171
public function testIsGrantedForUserCreatesFieldVoteObjectWhenFieldNotNull($object, $field, $expectedSubject)
6272
{
63-
if (!class_exists(UserAuthorizationCheckerInterface::class)) {
73+
if (!method_exists(AuthorizationChecker::class, 'isGrantedForUser')) {
6474
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.');
6575
}
6676

6777
$user = $this->createMock(UserInterface::class);
68-
$userSecurityChecker = $this->createMock(UserAuthorizationCheckerInterface::class);
69-
$userSecurityChecker
70-
->expects($this->once())
71-
->method('isGrantedForUser')
72-
->with($user, 'ROLE', $expectedSubject)
73-
->willReturn(true);
78+
$securityChecker = new class implements AuthorizationCheckerInterface {
79+
public UserInterface $user;
80+
public mixed $attribute;
81+
public mixed $subject;
82+
83+
public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
84+
{
85+
throw new \BadMethodCallException('This method should not be called.');
86+
}
87+
88+
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
89+
{
90+
$this->user = $user;
91+
$this->attribute = $attribute;
92+
$this->subject = $subject;
93+
94+
return true;
95+
}
96+
};
7497

75-
$securityExtension = new SecurityExtension(null, null, $userSecurityChecker);
98+
$securityExtension = new SecurityExtension($securityChecker);
7699
$this->assertTrue($securityExtension->isGrantedForUser($user, 'ROLE', $object, $field));
77-
}
100+
$this->assertSame($user, $securityChecker->user);
101+
$this->assertSame('ROLE', $securityChecker->attribute);
78102

79-
public function testIsGrantedForUserThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist()
80-
{
81-
if (!class_exists(UserAuthorizationCheckerInterface::class)) {
82-
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.');
103+
if (null === $field) {
104+
$this->assertSame($object, $securityChecker->subject);
105+
} else {
106+
$this->assertEquals($expectedSubject, $securityChecker->subject);
83107
}
84-
85-
$securityChecker = $this->createMock(UserAuthorizationCheckerInterface::class);
86-
87-
ClassExistsMock::register(SecurityExtension::class);
88-
ClassExistsMock::withMockedClasses([FieldVote::class => false]);
89-
90-
$this->expectException(\LogicException::class);
91-
$this->expectExceptionMessageMatches('Passing a $field to the "is_granted_for_user()" function requires symfony/acl.');
92-
93-
$securityExtension = new SecurityExtension(null, null, $securityChecker);
94-
$securityExtension->isGrantedForUser($this->createMock(UserInterface::class), 'object', 'bar');
95108
}
96109

97110
public static function provideObjectFieldAclCases()
@@ -105,4 +118,21 @@ public static function provideObjectFieldAclCases()
105118
['object', 'field', new FieldVote('object', 'field')],
106119
];
107120
}
121+
122+
public function testIsGrantedForUserThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist()
123+
{
124+
if (!method_exists(AuthorizationChecker::class, 'isGrantedForUser')) {
125+
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.');
126+
}
127+
128+
$securityChecker = $this->createMock(AuthorizationCheckerInterface::class);
129+
130+
ClassExistsMock::withMockedClasses([FieldVote::class => false]);
131+
132+
$this->expectException(\LogicException::class);
133+
$this->expectExceptionMessage('Passing a $field to the "is_granted_for_user()" function requires symfony/acl.');
134+
135+
$securityExtension = new SecurityExtension($securityChecker);
136+
$securityExtension->isGrantedForUser($this->createMock(UserInterface::class), 'ROLE', 'object', 'bar');
137+
}
108138
}

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@
3131
use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
3232
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
3333
use Symfony\Component\Security\Core\Authorization\ExpressionLanguage;
34-
use Symfony\Component\Security\Core\Authorization\UserAuthorizationChecker;
35-
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
3634
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
3735
use Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter;
3836
use Symfony\Component\Security\Core\Authorization\Voter\RoleHierarchyVoter;
@@ -69,12 +67,6 @@
6967
])
7068
->alias(AuthorizationCheckerInterface::class, 'security.authorization_checker')
7169

72-
->set('security.user_authorization_checker', UserAuthorizationChecker::class)
73-
->args([
74-
service('security.access.decision_manager'),
75-
])
76-
->alias(UserAuthorizationCheckerInterface::class, 'security.user_authorization_checker')
77-
7870
->set('security.token_storage', UsageTrackingTokenStorage::class)
7971
->args([
8072
service('security.untracked_token_storage'),
@@ -93,7 +85,6 @@
9385
service_locator([
9486
'security.token_storage' => service('security.token_storage'),
9587
'security.authorization_checker' => service('security.authorization_checker'),
96-
'security.user_authorization_checker' => service('security.user_authorization_checker'),
9788
'security.authenticator.managers_locator' => service('security.authenticator.managers_locator')->ignoreOnInvalid(),
9889
'request_stack' => service('request_stack'),
9990
'security.firewall.map' => service('security.firewall.map'),

‎src/Symfony/Bundle/SecurityBundle/Resources/config/templating_twig.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
->args([
2727
service('security.authorization_checker')->ignoreOnInvalid(),
2828
service('security.impersonate_url_generator')->ignoreOnInvalid(),
29-
service('security.user_authorization_checker')->ignoreOnInvalid(),
3029
])
3130
->tag('twig.extension')
3231
;

‎src/Symfony/Bundle/SecurityBundle/Security.php

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
2020
use Symfony\Component\Security\Core\Authorization\AccessDecision;
2121
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
22-
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
2322
use Symfony\Component\Security\Core\Exception\LogicException;
2423
use Symfony\Component\Security\Core\Exception\LogoutException;
2524
use Symfony\Component\Security\Core\User\UserInterface;
@@ -39,7 +38,7 @@
3938
*
4039
* @final
4140
*/
42-
class Security implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface
41+
class Security implements AuthorizationCheckerInterface
4342
{
4443
public function __construct(
4544
private readonly ContainerInterface $container,
@@ -65,6 +64,17 @@ public function isGranted(mixed $attributes, mixed $subject = null, ?AccessDecis
6564
->isGranted($attributes, $subject, $accessDecision);
6665
}
6766

67+
/**
68+
* Checks if the attribute is granted against the user and optionally supplied subject.
69+
*
70+
* This should be used over isGranted() when checking permissions against a user that is not currently logged in or while in a CLI context.
71+
*/
72+
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
73+
{
74+
return $this->container->get('security.authorization_checker')
75+
->isGrantedForUser($user, $attribute, $subject, $accessDecision);
76+
}
77+
6878
public function getToken(): ?TokenInterface
6979
{
7080
return $this->container->get('security.token_storage')->getToken();
@@ -150,17 +160,6 @@ public function logout(bool $validateCsrfToken = true): ?Response
150160
return $logoutEvent->getResponse();
151161
}
152162

153-
/**
154-
* Checks if the attribute is granted against the user and optionally supplied subject.
155-
*
156-
* This should be used over isGranted() when checking permissions against a user that is not currently logged in or while in a CLI context.
157-
*/
158-
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
159-
{
160-
return $this->container->get('security.user_authorization_checker')
161-
->isGrantedForUser($user, $attribute, $subject, $accessDecision);
162-
}
163-
164163
private function getAuthenticator(?string $authenticatorName, string $firewallName): AuthenticatorInterface
165164
{
166165
if (!isset($this->authenticators[$firewallName])) {

‎src/Symfony/Component/Security/Core/Authentication/Token/UserAuthorizationCheckerToken.php

Lines changed: 0 additions & 31 deletions
This file was deleted.

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,11 @@
1111

1212
namespace Symfony\Component\Security\Core\Authorization;
1313

14+
use Symfony\Component\Security\Core\Authentication\Token\AbstractToken;
1415
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
16+
use Symfony\Component\Security\Core\Authentication\Token\OfflineTokenInterface;
1517
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
18+
use Symfony\Component\Security\Core\User\UserInterface;
1619

1720
/**
1821
* AuthorizationChecker is the main authorization point of the Security component.
@@ -48,4 +51,19 @@ final public function isGranted(mixed $attribute, mixed $subject = null, ?Access
4851
array_pop($this->accessDecisionStack);
4952
}
5053
}
54+
55+
final public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
56+
{
57+
$token = new class($user->getRoles()) extends AbstractToken implements OfflineTokenInterface {};
58+
$token->setUser($user);
59+
60+
$accessDecision ??= end($this->accessDecisionStack) ?: new AccessDecision();
61+
$this->accessDecisionStack[] = $accessDecision;
62+
63+
try {
64+
return $accessDecision->isGranted = $this->accessDecisionManager->decide($token, [$attribute], $subject, $accessDecision);
65+
} finally {
66+
array_pop($this->accessDecisionStack);
67+
}
68+
}
5169
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
* The AuthorizationCheckerInterface.
1616
*
1717
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
18+
*
19+
* @method bool isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null)
1820
*/
1921
interface AuthorizationCheckerInterface
2022
{

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

Lines changed: 0 additions & 31 deletions
This file was deleted.

0 commit comments

Comments
 (0)
0