Skip to content

Commit 3d81940

Browse files
HypeMCHypeMC
committed
[Security] Allow using expressions with the #[IsGranted] attribute
1 parent 9d7ff0c commit 3d81940

File tree

10 files changed

+182
-36
lines changed

10 files changed

+182
-36
lines changed

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddExpressionLanguageProvidersPass.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public function process(ContainerBuilder $container)
3636

3737
if (!$container->hasDefinition('cache.system')) {
3838
$container->removeDefinition('cache.security_expression_language');
39+
$container->removeDefinition('cache.security_is_granted_attribute_expression_language');
3940
}
4041
}
4142
}

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ public function load(array $configs, ContainerBuilder $container)
112112
if (!$container::willBeAvailable('symfony/expression-language', ExpressionLanguage::class, ['symfony/security-bundle'])) {
113113
$container->removeDefinition('security.expression_language');
114114
$container->removeDefinition('security.access.expression_voter');
115+
$container->removeDefinition('security.is_granted_attribute_expression_language');
115116
}
116117

117118
// set some global scalars

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Bundle\SecurityBundle\Security\FirewallMap;
1919
use Symfony\Bundle\SecurityBundle\Security\LazyFirewallContext;
2020
use Symfony\Bundle\SecurityBundle\Security\Security;
21+
use Symfony\Component\ExpressionLanguage\ExpressionLanguage as BaseExpressionLanguage;
2122
use Symfony\Component\Ldap\Security\LdapUserProvider;
2223
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver;
2324
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
@@ -275,7 +276,17 @@
275276
->tag('kernel.cache_warmer')
276277

277278
->set('controller.is_granted_attribute_listener', IsGrantedAttributeListener::class)
278-
->args([service('security.authorization_checker')])
279+
->args([
280+
service('security.authorization_checker'),
281+
service('security.is_granted_attribute_expression_language')->nullOnInvalid(),
282+
])
279283
->tag('kernel.event_subscriber')
284+
285+
->set('security.is_granted_attribute_expression_language', BaseExpressionLanguage::class)
286+
->args([service('cache.security_is_granted_attribute_expression_language')->nullOnInvalid()])
287+
288+
->set('cache.security_is_granted_attribute_expression_language')
289+
->parent('cache.system')
290+
->tag('cache.pool')
280291
;
281292
};

‎src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

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

14+
use Symfony\Component\ExpressionLanguage\Expression;
15+
1416
/**
1517
* AccessDeniedException is thrown when the account has not the required role.
1618
*
@@ -31,9 +33,9 @@ public function getAttributes(): array
3133
return $this->attributes;
3234
}
3335

34-
public function setAttributes(array|string $attributes)
36+
public function setAttributes(array|string|Expression $attributes)
3537
{
36-
$this->attributes = (array) $attributes;
38+
$this->attributes = $attributes instanceof Expression ? [$attributes] : (array) $attributes;
3739
}
3840

3941
public function getSubject(): mixed

‎src/Symfony/Component/Security/Http/Attribute/IsGranted.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
namespace Symfony\Component\Security\Http\Attribute;
1313

14+
use Symfony\Component\ExpressionLanguage\Expression;
15+
1416
/**
1517
* @author Ryan Weaver <ryan@knpuniversity.com>
1618
*/
@@ -21,12 +23,12 @@ public function __construct(
2123
/**
2224
* Sets the first argument that will be passed to isGranted().
2325
*/
24-
public array|string|null $attributes = null,
26+
public array|string|Expression|null $attributes = null,
2527

2628
/**
2729
* Sets the second argument passed to isGranted().
2830
*/
29-
public array|string|null $subject = null,
31+
public array|string|Expression|null $subject = null,
3032

3133
/**
3234
* The message of the exception - has a nice default if not set.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ CHANGELOG
99
* Deprecate empty username or password when using when using `JsonLoginAuthenticator`
1010
* Set custom lifetime for login link
1111
* Add `$lifetime` parameter to `LoginLinkHandlerInterface::createLoginLink()`
12+
* Allow using expressions as `#[IsGranted()]` attribute and subject
1213

1314
6.0
1415
---

‎src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
namespace Symfony\Component\Security\Http\EventListener;
1313

1414
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
15+
use Symfony\Component\ExpressionLanguage\Expression;
16+
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
1517
use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
1618
use Symfony\Component\HttpKernel\Exception\HttpException;
1719
use Symfony\Component\HttpKernel\KernelEvents;
@@ -28,7 +30,8 @@
2830
class IsGrantedAttributeListener implements EventSubscriberInterface
2931
{
3032
public function __construct(
31-
private AuthorizationCheckerInterface $authChecker,
33+
private readonly AuthorizationCheckerInterface $authChecker,
34+
private ?ExpressionLanguage $expressionLanguage = null,
3235
) {
3336
}
3437

@@ -42,21 +45,15 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event)
4245
$arguments = $event->getNamedArguments();
4346

4447
foreach ($attributes as $attribute) {
45-
$subjectRef = $attribute->subject;
4648
$subject = null;
4749

48-
if ($subjectRef) {
50+
if ($subjectRef = $attribute->subject) {
4951
if (\is_array($subjectRef)) {
50-
foreach ($subjectRef as $ref) {
51-
if (!\array_key_exists($ref, $arguments)) {
52-
throw new RuntimeException(sprintf('Could not find the subject "%s" for the #[IsGranted] attribute. Try adding a "$%s" argument to your controller method.', $ref, $ref));
53-
}
54-
$subject[$ref] = $arguments[$ref];
52+
foreach ($subjectRef as $refKey => $ref) {
53+
$subject[\is_string($refKey) ? $refKey : (string) $ref] = $this->getIsGrantedSubject($ref, $arguments);
5554
}
56-
} elseif (!\array_key_exists($subjectRef, $arguments)) {
57-
throw new RuntimeException(sprintf('Could not find the subject "%s" for the #[IsGranted] attribute. Try adding a "$%s" argument to your controller method.', $subjectRef, $subjectRef));
5855
} else {
59-
$subject = $arguments[$subjectRef];
56+
$subject = $this->getIsGrantedSubject($subjectRef, $arguments);
6057
}
6158
}
6259

@@ -81,13 +78,40 @@ public static function getSubscribedEvents(): array
8178
return [KernelEvents::CONTROLLER_ARGUMENTS => ['onKernelControllerArguments', 10]];
8279
}
8380

81+
private function getIsGrantedSubject(string|Expression $subjectRef, array $arguments): mixed
82+
{
83+
if ($subjectRef instanceof Expression) {
84+
$this->expressionLanguage ??= new ExpressionLanguage();
85+
86+
return $this->expressionLanguage->evaluate($subjectRef, [
87+
'args' => $arguments,
88+
]);
89+
}
90+
91+
if (!\array_key_exists($subjectRef, $arguments)) {
92+
throw new RuntimeException(sprintf('Could not find the subject "%s" for the #[IsGranted] attribute. Try adding a "$%s" argument to your controller method.', $subjectRef, $subjectRef));
93+
}
94+
95+
return $arguments[$subjectRef];
96+
}
97+
8498
private function getIsGrantedString(IsGranted $isGranted): string
8599
{
86-
$attributes = array_map(fn ($attribute) => '"'.$attribute.'"', (array) $isGranted->attributes);
100+
$processValues = fn ($values) => array_map(
101+
fn ($value) => sprintf($value instanceof Expression ? 'new Expression("%s")' : '"%s"', $value),
102+
$values instanceof Expression ? [$values] : (array) $values
103+
);
104+
105+
$attributes = $processValues($isGranted->attributes);
87106
$argsString = 1 === \count($attributes) ? reset($attributes) : '['.implode(', ', $attributes).']';
88107

89-
if (null !== $isGranted->subject) {
90-
$argsString .= ', "'.implode('", "', (array) $isGranted->subject).'"';
108+
if (null !== $subject = $isGranted->subject) {
109+
$subject = $processValues($subject);
110+
$subject = array_map(
111+
fn ($key, $value) => \is_string($key) ? sprintf('"%s" => %s', $key, $value) : $value,
112+
array_keys($subject), $subject
113+
);
114+
$argsString .= ', '.(1 === \count($subject) ? reset($subject) : '['.implode(', ', $subject).']');
91115
}
92116

93117
return $argsString;

‎src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php‎

Lines changed: 101 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,13 @@
1212
namespace Symfony\Component\Security\Http\Tests\EventListener;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\ExpressionLanguage\Expression;
1516
use Symfony\Component\HttpFoundation\Request;
1617
use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
1718
use Symfony\Component\HttpKernel\Exception\HttpException;
1819
use Symfony\Component\HttpKernel\HttpKernelInterface;
1920
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
21+
use Symfony\Component\Security\Core\Authorization\ExpressionLanguage;
2022
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
2123
use Symfony\Component\Security\Http\EventListener\IsGrantedAttributeListener;
2224
use Symfony\Component\Security\Http\Tests\Fixtures\IsGrantedAttributeController;
@@ -42,7 +44,7 @@ public function testAttribute()
4244
$listener = new IsGrantedAttributeListener($authChecker);
4345
$listener->onKernelControllerArguments($event);
4446

45-
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
47+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
4648
$authChecker->expects($this->once())
4749
->method('isGranted')
4850
->willReturn(true);
@@ -61,7 +63,7 @@ public function testAttribute()
6163

6264
public function testNothingHappensWithNoConfig()
6365
{
64-
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
66+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
6567
$authChecker->expects($this->never())
6668
->method('isGranted');
6769

@@ -79,7 +81,7 @@ public function testNothingHappensWithNoConfig()
7981

8082
public function testIsGrantedCalledCorrectly()
8183
{
82-
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
84+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
8385
$authChecker->expects($this->once())
8486
->method('isGranted')
8587
->with('ROLE_ADMIN')
@@ -99,7 +101,7 @@ public function testIsGrantedCalledCorrectly()
99101

100102
public function testIsGrantedSubjectFromArguments()
101103
{
102-
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
104+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
103105
$authChecker->expects($this->once())
104106
->method('isGranted')
105107
// the subject => arg2name will eventually resolve to the 2nd argument, which has this value
@@ -146,7 +148,7 @@ public function testIsGrantedSubjectFromArgumentsWithArray()
146148

147149
public function testIsGrantedNullSubjectFromArguments()
148150
{
149-
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
151+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
150152
$authChecker->expects($this->once())
151153
->method('isGranted')
152154
->with('ROLE_ADMIN', null)
@@ -166,7 +168,7 @@ public function testIsGrantedNullSubjectFromArguments()
166168

167169
public function testIsGrantedArrayWithNullValueSubjectFromArguments()
168170
{
169-
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
171+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
170172
$authChecker->expects($this->once())
171173
->method('isGranted')
172174
->with('ROLE_ADMIN', [
@@ -191,7 +193,7 @@ public function testExceptionWhenMissingSubjectAttribute()
191193
{
192194
$this->expectException(\RuntimeException::class);
193195

194-
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
196+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
195197

196198
$event = new ControllerArgumentsEvent(
197199
$this->createMock(HttpKernelInterface::class),
@@ -208,20 +210,22 @@ public function testExceptionWhenMissingSubjectAttribute()
208210
/**
209211
* @dataProvider getAccessDeniedMessageTests
210212
*/
211-
public function testAccessDeniedMessages(array $attributes, ?string $subject, string $method, string $expectedMessage)
213+
public function testAccessDeniedMessages(array $attributes, string|array|null $subject, string $method, string $expectedMessage)
212214
{
213-
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
215+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
214216
$authChecker->expects($this->any())
215217
->method('isGranted')
216218
->willReturn(false);
217219

220+
$expressionLanguage = $this->createMock(ExpressionLanguage::class);
221+
$expressionLanguage->expects($this->any())
222+
->method('evaluate')
223+
->willReturn('bar');
224+
218225
// avoid the error of the subject not being found in the request attributes
219-
$arguments = [];
220-
if (null !== $subject) {
221-
$arguments[] = 'bar';
222-
}
226+
$arguments = array_fill(0, \count((array) $subject), 'bar');
223227

224-
$listener = new IsGrantedAttributeListener($authChecker);
228+
$listener = new IsGrantedAttributeListener($authChecker, $expressionLanguage);
225229

226230
$event = new ControllerArgumentsEvent(
227231
$this->createMock(HttpKernelInterface::class),
@@ -236,9 +240,9 @@ public function testAccessDeniedMessages(array $attributes, ?string $subject, st
236240
$this->fail();
237241
} catch (AccessDeniedException $e) {
238242
$this->assertSame($expectedMessage, $e->getMessage());
239-
$this->assertSame($attributes, $e->getAttributes());
243+
$this->assertEquals($attributes, $e->getAttributes());
240244
if (null !== $subject) {
241-
$this->assertSame('bar', $e->getSubject());
245+
$this->assertSame($subject, $e->getSubject());
242246
} else {
243247
$this->assertNull($e->getSubject());
244248
}
@@ -249,7 +253,11 @@ public function getAccessDeniedMessageTests()
249253
{
250254
yield [['ROLE_ADMIN'], null, 'admin', 'Access Denied by #[IsGranted("ROLE_ADMIN")] on controller'];
251255
yield [['ROLE_ADMIN', 'ROLE_USER'], null, 'adminOrUser', 'Access Denied by #[IsGranted(["ROLE_ADMIN", "ROLE_USER"])] on controller'];
252-
yield [['ROLE_ADMIN', 'ROLE_USER'], 'product', 'adminOrUserWithSubject', 'Access Denied by #[IsGranted(["ROLE_ADMIN", "ROLE_USER"], "product")] on controller'];
256+
yield [['ROLE_ADMIN', 'ROLE_USER'], 'bar', 'adminOrUserWithSubject', 'Access Denied by #[IsGranted(["ROLE_ADMIN", "ROLE_USER"], "product")] on controller'];
257+
yield [['ROLE_ADMIN'], ['arg1Name' => 'bar', 'arg2Name' => 'bar'], 'withSubjectArray', 'Access Denied by #[IsGranted("ROLE_ADMIN", ["arg1Name", "arg2Name"])] on controller'];
258+
yield [[new Expression('"ROLE_ADMIN" in role_names or is_granted("POST_VIEW", subject)')], 'bar', 'withExpressionInAttribute', 'Access Denied by #[IsGranted(new Expression(""ROLE_ADMIN" in role_names or is_granted("POST_VIEW", subject)"), "post")] on controller'];
259+
yield [[new Expression('user === subject')], 'bar', 'withExpressionInSubject', 'Access Denied by #[IsGranted(new Expression("user === subject"), new Expression("args["post"].getAuthor()"))] on controller'];
260+
yield [[new Expression('user === subject["author"]')], ['author' => 'bar', 'alias' => 'bar'], 'withNestedExpressionInSubject', 'Access Denied by #[IsGranted(new Expression("user === subject["author"]"), ["author" => new Expression("args["post"].getAuthor()"), "alias" => "arg2Name"])] on controller'];
253261
}
254262

255263
public function testNotFoundHttpException()
@@ -273,4 +281,80 @@ public function testNotFoundHttpException()
273281
$listener = new IsGrantedAttributeListener($authChecker);
274282
$listener->onKernelControllerArguments($event);
275283
}
284+
285+
public function testIsGrantedwithExpressionInAttribute()
286+
{
287+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
288+
$authChecker->expects($this->once())
289+
->method('isGranted')
290+
->with(new Expression('"ROLE_ADMIN" in role_names or is_granted("POST_VIEW", subject)'), 'postVal')
291+
->willReturn(true);
292+
293+
$event = new ControllerArgumentsEvent(
294+
$this->createMock(HttpKernelInterface::class),
295+
[new IsGrantedAttributeMethodsController(), 'withExpressionInAttribute'],
296+
['postVal'],
297+
new Request(),
298+
null
299+
);
300+
301+
$listener = new IsGrantedAttributeListener($authChecker);
302+
$listener->onKernelControllerArguments($event);
303+
}
304+
305+
public function testIsGrantedwithExpressionInSubject()
306+
{
307+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
308+
$authChecker->expects($this->once())
309+
->method('isGranted')
310+
->with(new Expression('user === subject'), 'author')
311+
->willReturn(true);
312+
313+
$expressionLanguage = $this->createMock(ExpressionLanguage::class);
314+
$expressionLanguage->expects($this->once())
315+
->method('evaluate')
316+
->with(new Expression('args["post"].getAuthor()'), [
317+
'args' => ['post' => 'postVal'],
318+
])
319+
->willReturn('author');
320+
321+
$event = new ControllerArgumentsEvent(
322+
$this->createMock(HttpKernelInterface::class),
323+
[new IsGrantedAttributeMethodsController(), 'withExpressionInSubject'],
324+
['postVal'],
325+
new Request(),
326+
null
327+
);
328+
329+
$listener = new IsGrantedAttributeListener($authChecker, $expressionLanguage);
330+
$listener->onKernelControllerArguments($event);
331+
}
332+
333+
public function testIsGrantedwithNestedExpressionInSubject()
334+
{
335+
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
336+
$authChecker->expects($this->once())
337+
->method('isGranted')
338+
->with(new Expression('user === subject["author"]'), ['author' => 'author', 'alias' => 'arg2Val'])
339+
->willReturn(true);
340+
341+
$expressionLanguage = $this->createMock(ExpressionLanguage::class);
342+
$expressionLanguage->expects($this->once())
343+
->method('evaluate')
344+
->with(new Expression('args["post"].getAuthor()'), [
345+
'args' => ['post' => 'postVal', 'arg2Name' => 'arg2Val'],
346+
])
347+
->willReturn('author');
348+
349+
$event = new ControllerArgumentsEvent(
350+
$this->createMock(HttpKernelInterface::class),
351+
[new IsGrantedAttributeMethodsController(), 'withNestedExpressionInSubject'],
352+
['postVal', 'arg2Val'],
353+
new Request(),
354+
null
355+
);
356+
357+
$listener = new IsGrantedAttributeListener($authChecker, $expressionLanguage);
358+
$listener->onKernelControllerArguments($event);
359+
}
276360
}

0 commit comments

Comments
 (0)