Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d74e07a

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[Security] Improve DX of recent additions
1 parent ca6f399 commit d74e07a
Copy full SHA for d74e07a

22 files changed

+338
-378
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Twig/Extension/SecurityExtension.php
+8-5
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ final class SecurityExtension extends AbstractExtension
3131
public function __construct(
3232
private ?AuthorizationCheckerInterface $securityChecker = null,
3333
private ?ImpersonateUrlGenerator $impersonateUrlGenerator = null,
34-
private ?UserAuthorizationCheckerInterface $userSecurityChecker = null,
3534
) {
3635
}
3736

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

5958
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null, ?AccessDecision $accessDecision = null): bool
6059
{
61-
if (!$this->userSecurityChecker) {
62-
throw new \LogicException(\sprintf('An instance of "%s" must be provided to use "%s()".', UserAuthorizationCheckerInterface::class, __METHOD__));
60+
if (null === $this->securityChecker) {
61+
return false;
62+
}
63+
64+
if (!$this->securityChecker instanceof UserAuthorizationCheckerInterface) {
65+
throw new \LogicException(\sprintf('You cannot use "%s()" if the authorization checker doesn\'t implement "%s".%s', __METHOD__, UserAuthorizationCheckerInterface::class, interface_exists(UserAuthorizationCheckerInterface::class) ? ' Try upgrading the "symfony/security-core" package to v7.3 minimum.' : ''));
6366
}
6467

6568
if (null !== $field) {
@@ -71,7 +74,7 @@ public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $s
7174
}
7275

7376
try {
74-
return $this->userSecurityChecker->isGrantedForUser($user, $attribute, $subject, $accessDecision);
77+
return $this->securityChecker->isGrantedForUser($user, $attribute, $subject, $accessDecision);
7578
} catch (AuthenticationCredentialsNotFoundException) {
7679
return false;
7780
}
@@ -123,7 +126,7 @@ public function getFunctions(): array
123126
new TwigFunction('impersonation_path', $this->getImpersonatePath(...)),
124127
];
125128

126-
if ($this->userSecurityChecker) {
129+
if ($this->securityChecker instanceof UserAuthorizationCheckerInterface) {
127130
$functions[] = new TwigFunction('is_granted_for_user', $this->isGrantedForUser(...));
128131
}
129132

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Twig/Tests/Extension/SecurityExtensionTest.php
+61-26
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;
1819
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
1920
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 (!interface_exists(UserAuthorizationCheckerInterface::class)) {
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,49 +70,74 @@ public function testIsGrantedThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist
6070
*/
6171
public function testIsGrantedForUserCreatesFieldVoteObjectWhenFieldNotNull($object, $field, $expectedSubject)
6272
{
63-
if (!class_exists(UserAuthorizationCheckerInterface::class)) {
73+
if (!interface_exists(UserAuthorizationCheckerInterface::class)) {
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 = $this->createMockAuthorizationChecker();
7479

75-
$securityExtension = new SecurityExtension(null, null, $userSecurityChecker);
80+
$securityExtension = new SecurityExtension($securityChecker);
7681
$this->assertTrue($securityExtension->isGrantedForUser($user, 'ROLE', $object, $field));
82+
$this->assertSame($user, $securityChecker->user);
83+
$this->assertSame('ROLE', $securityChecker->attribute);
84+
85+
if (null === $field) {
86+
$this->assertSame($object, $securityChecker->subject);
87+
} else {
88+
$this->assertEquals($expectedSubject, $securityChecker->subject);
89+
}
90+
}
91+
92+
public static function provideObjectFieldAclCases()
93+
{
94+
return [
95+
[null, null, null],
96+
['object', null, 'object'],
97+
['object', false, new FieldVote('object', false)],
98+
['object', 0, new FieldVote('object', 0)],
99+
['object', '', new FieldVote('object', '')],
100+
['object', 'field', new FieldVote('object', 'field')],
101+
];
77102
}
78103

79104
public function testIsGrantedForUserThrowsWhenFieldNotNullAndFieldVoteClassDoesNotExist()
80105
{
81-
if (!class_exists(UserAuthorizationCheckerInterface::class)) {
106+
if (!interface_exists(UserAuthorizationCheckerInterface::class)) {
82107
$this->markTestSkipped('This test requires symfony/security-core 7.3 or superior.');
83108
}
84109

85-
$securityChecker = $this->createMock(UserAuthorizationCheckerInterface::class);
110+
$securityChecker = $this->createMockAuthorizationChecker();
86111

87-
ClassExistsMock::register(SecurityExtension::class);
88112
ClassExistsMock::withMockedClasses([FieldVote::class => false]);
89113

90114
$this->expectException(\LogicException::class);
91-
$this->expectExceptionMessageMatches('Passing a $field to the "is_granted_for_user()" function requires symfony/acl.');
115+
$this->expectExceptionMessage('Passing a $field to the "is_granted_for_user()" function requires symfony/acl.');
92116

93-
$securityExtension = new SecurityExtension(null, null, $securityChecker);
94-
$securityExtension->isGrantedForUser($this->createMock(UserInterface::class), 'object', 'bar');
117+
$securityExtension = new SecurityExtension($securityChecker);
118+
$securityExtension->isGrantedForUser($this->createMock(UserInterface::class), 'ROLE', 'object', 'bar');
95119
}
96120

97-
public static function provideObjectFieldAclCases()
121+
private function createMockAuthorizationChecker(): AuthorizationCheckerInterface&UserAuthorizationCheckerInterface
98122
{
99-
return [
100-
[null, null, null],
101-
['object', null, 'object'],
102-
['object', false, new FieldVote('object', false)],
103-
['object', 0, new FieldVote('object', 0)],
104-
['object', '', new FieldVote('object', '')],
105-
['object', 'field', new FieldVote('object', 'field')],
106-
];
123+
return new class implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface {
124+
public UserInterface $user;
125+
public mixed $attribute;
126+
public mixed $subject;
127+
128+
public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
129+
{
130+
throw new \BadMethodCallException('This method should not be called.');
131+
}
132+
133+
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
134+
{
135+
$this->user = $user;
136+
$this->attribute = $attribute;
137+
$this->subject = $subject;
138+
139+
return true;
140+
}
141+
};
107142
}
108143
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/security.php
+3-10
Original file line numberDiff line numberDiff line change
@@ -31,7 +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;
3534
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
3635
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
3736
use Symfony\Component\Security\Core\Authorization\Voter\ClosureVoter;
@@ -69,12 +68,7 @@
6968
service('security.access.decision_manager'),
7069
])
7170
->alias(AuthorizationCheckerInterface::class, 'security.authorization_checker')
72-
73-
->set('security.user_authorization_checker', UserAuthorizationChecker::class)
74-
->args([
75-
service('security.access.decision_manager'),
76-
])
77-
->alias(UserAuthorizationCheckerInterface::class, 'security.user_authorization_checker')
71+
->alias(UserAuthorizationCheckerInterface::class, 'security.authorization_checker')
7872

7973
->set('security.token_storage', UsageTrackingTokenStorage::class)
8074
->args([
@@ -94,7 +88,7 @@
9488
service_locator([
9589
'security.token_storage' => service('security.token_storage'),
9690
'security.authorization_checker' => service('security.authorization_checker'),
97-
'security.user_authorization_checker' => service('security.user_authorization_checker'),
91+
'security.user_authorization_checker' => service('security.authorization_checker'),
9892
'security.authenticator.managers_locator' => service('security.authenticator.managers_locator')->ignoreOnInvalid(),
9993
'request_stack' => service('request_stack'),
10094
'security.firewall.map' => service('security.firewall.map'),
@@ -174,8 +168,7 @@
174168

175169
->set('security.access.closure_voter', ClosureVoter::class)
176170
->args([
177-
service('security.access.decision_manager'),
178-
service('security.authentication.trust_resolver'),
171+
service('security.authorization_checker'),
179172
])
180173
->tag('security.voter', ['priority' => 245])
181174

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/templating_twig.php
-1
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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Security.php
+11-11
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,17 @@ public function isGranted(mixed $attributes, mixed $subject = null, ?AccessDecis
6565
->isGranted($attributes, $subject, $accessDecision);
6666
}
6767

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

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-
164164
private function getAuthenticator(?string $authenticatorName, string $firewallName): AuthenticatorInterface
165165
{
166166
if (!isset($this->authenticators[$firewallName])) {

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authentication/Token/UserAuthorizationCheckerToken.php
-31
This file was deleted.

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php
+19-2
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.
@@ -22,8 +25,9 @@
2225
* @author Fabien Potencier <fabien@symfony.com>
2326
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
2427
*/
25-
class AuthorizationChecker implements AuthorizationCheckerInterface
28+
class AuthorizationChecker implements AuthorizationCheckerInterface, UserAuthorizationCheckerInterface
2629
{
30+
private array $tokenStack = [];
2731
private array $accessDecisionStack = [];
2832

2933
public function __construct(
@@ -34,7 +38,7 @@ public function __construct(
3438

3539
final public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
3640
{
37-
$token = $this->tokenStorage->getToken();
41+
$token = end($this->tokenStack) ?: $this->tokenStorage->getToken();
3842

3943
if (!$token || !$token->getUser()) {
4044
$token = new NullToken();
@@ -48,4 +52,17 @@ final public function isGranted(mixed $attribute, mixed $subject = null, ?Access
4852
array_pop($this->accessDecisionStack);
4953
}
5054
}
55+
56+
final public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
57+
{
58+
$token = new class($user->getRoles()) extends AbstractToken implements OfflineTokenInterface {};
59+
$token->setUser($user);
60+
$this->tokenStack[] = $token;
61+
62+
try {
63+
return $this->isGranted($attribute, $subject, $accessDecision);
64+
} finally {
65+
array_pop($this->tokenStack);
66+
}
67+
}
5168
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authorization/UserAuthorizationChecker.php
-31
This file was deleted.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.