Skip to content

Commit 445f0f1

Browse files
nicolas-grekasnicolas-grekas
committed
minor #47144 [Security] Remove using multiple attributes with #[IsGranted] (HypeMC)
This PR was merged into the 6.2 branch. Discussion ---------- [Security] Remove using multiple attributes with #[IsGranted] | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #46978 (comment) | License | MIT | Doc PR | - Passing multiple attributes to `isGranted()` has been removed in #33584, so the following doesn't work any more: ```php #[IsGranted(attributes: ['ROLE_ADMIN'])] public function index(Post $post) { } #[IsGranted(attributes: ['ROLE_USER', 'ROLE_ADMIN'])] public function index(Post $post) { } ``` As mentioned in sensiolabs/SensioFrameworkExtraBundle#648 , expressions should be used instead, see #46978 . This PR removes the possibility of using multiple attributes with `#[IsGranted]`. Also, it's currently possible to use `#[IsGranted()]` with no attributes (`null`). Since this doesn't seem to work either, nor can I find a reason why it even should, this PR removes that option as well. If I'm wrong about this one, please let me know. Commits ------- 663dc3e [Security] Remove using multiple attributes with #[IsGranted]
2 parents 3e1f1fe + 663dc3e commit 445f0f1

File tree

5 files changed

+23
-39
lines changed

5 files changed

+23
-39
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public function __construct(
2121
/**
2222
* Sets the first argument that will be passed to isGranted().
2323
*/
24-
public array|string|null $attributes = null,
24+
public string $attribute,
2525

2626
/**
2727
* Sets the second argument passed to isGranted().

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,15 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event)
6060
}
6161
}
6262

63-
if (!$this->authChecker->isGranted($attribute->attributes, $subject)) {
63+
if (!$this->authChecker->isGranted($attribute->attribute, $subject)) {
6464
$message = $attribute->message ?: sprintf('Access Denied by #[IsGranted(%s)] on controller', $this->getIsGrantedString($attribute));
6565

6666
if ($statusCode = $attribute->statusCode) {
6767
throw new HttpException($statusCode, $message);
6868
}
6969

7070
$accessDeniedException = new AccessDeniedException($message);
71-
$accessDeniedException->setAttributes($attribute->attributes);
71+
$accessDeniedException->setAttributes($attribute->attribute);
7272
$accessDeniedException->setSubject($subject);
7373

7474
throw $accessDeniedException;
@@ -83,11 +83,13 @@ public static function getSubscribedEvents(): array
8383

8484
private function getIsGrantedString(IsGranted $isGranted): string
8585
{
86-
$attributes = array_map(fn ($attribute) => '"'.$attribute.'"', (array) $isGranted->attributes);
87-
$argsString = 1 === \count($attributes) ? reset($attributes) : '['.implode(', ', $attributes).']';
86+
$processValue = fn ($value) => sprintf('"%s"', $value);
8887

89-
if (null !== $isGranted->subject) {
90-
$argsString .= ', "'.implode('", "', (array) $isGranted->subject).'"';
88+
$argsString = $processValue($isGranted->attribute);
89+
90+
if (null !== $subject = $isGranted->subject) {
91+
$subject = array_map($processValue, (array) $subject);
92+
$argsString .= ', '.(1 === \count($subject) ? reset($subject) : '['.implode(', ', $subject).']');
9193
}
9294

9395
return $argsString;

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

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -208,18 +208,15 @@ public function testExceptionWhenMissingSubjectAttribute()
208208
/**
209209
* @dataProvider getAccessDeniedMessageTests
210210
*/
211-
public function testAccessDeniedMessages(array $attributes, ?string $subject, string $method, string $expectedMessage)
211+
public function testAccessDeniedMessages(string $attribute, string|array|null $subject, string $method, int $numOfArguments, string $expectedMessage)
212212
{
213213
$authChecker = $this->getMockBuilder(AuthorizationCheckerInterface::class)->getMock();
214214
$authChecker->expects($this->any())
215215
->method('isGranted')
216216
->willReturn(false);
217217

218218
// avoid the error of the subject not being found in the request attributes
219-
$arguments = [];
220-
if (null !== $subject) {
221-
$arguments[] = 'bar';
222-
}
219+
$arguments = array_fill(0, $numOfArguments, 'bar');
223220

224221
$listener = new IsGrantedAttributeListener($authChecker);
225222

@@ -236,9 +233,9 @@ public function testAccessDeniedMessages(array $attributes, ?string $subject, st
236233
$this->fail();
237234
} catch (AccessDeniedException $e) {
238235
$this->assertSame($expectedMessage, $e->getMessage());
239-
$this->assertSame($attributes, $e->getAttributes());
236+
$this->assertSame([$attribute], $e->getAttributes());
240237
if (null !== $subject) {
241-
$this->assertSame('bar', $e->getSubject());
238+
$this->assertSame($subject, $e->getSubject());
242239
} else {
243240
$this->assertNull($e->getSubject());
244241
}
@@ -247,9 +244,9 @@ public function testAccessDeniedMessages(array $attributes, ?string $subject, st
247244

248245
public function getAccessDeniedMessageTests()
249246
{
250-
yield [['ROLE_ADMIN'], null, 'admin', 'Access Denied by #[IsGranted("ROLE_ADMIN")] on controller'];
251-
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'];
247+
yield ['ROLE_ADMIN', null, 'admin', 0, 'Access Denied by #[IsGranted("ROLE_ADMIN")] on controller'];
248+
yield ['ROLE_ADMIN', 'bar', 'withSubject', 2, 'Access Denied by #[IsGranted("ROLE_ADMIN", "arg2Name")] on controller'];
249+
yield ['ROLE_ADMIN', ['arg1Name' => 'bar', 'arg2Name' => 'bar'], 'withSubjectArray', 2, 'Access Denied by #[IsGranted("ROLE_ADMIN", ["arg1Name", "arg2Name"])] on controller'];
253250
}
254251

255252
public function testNotFoundHttpException()

‎src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeController.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@
1313

1414
use Symfony\Component\Security\Http\Attribute\IsGranted;
1515

16-
#[IsGranted(attributes: ['ROLE_ADMIN', 'ROLE_USER'])]
16+
#[IsGranted(attribute: 'ROLE_USER')]
1717
class IsGrantedAttributeController
1818
{
19-
#[IsGranted(attributes: ['ROLE_ADMIN'])]
19+
#[IsGranted(attribute: 'ROLE_ADMIN')]
2020
public function foo()
2121
{
2222
}

‎src/Symfony/Component/Security/Http/Tests/Fixtures/IsGrantedAttributeMethodsController.php‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,42 +19,27 @@ public function noAttribute()
1919
{
2020
}
2121

22-
#[IsGranted()]
23-
public function emptyAttribute()
24-
{
25-
}
26-
27-
#[IsGranted(attributes: 'ROLE_ADMIN')]
22+
#[IsGranted(attribute: 'ROLE_ADMIN')]
2823
public function admin()
2924
{
3025
}
3126

32-
#[IsGranted(attributes: ['ROLE_ADMIN', 'ROLE_USER'])]
33-
public function adminOrUser()
34-
{
35-
}
36-
37-
#[IsGranted(attributes: ['ROLE_ADMIN', 'ROLE_USER'], subject: 'product')]
38-
public function adminOrUserWithSubject($product)
39-
{
40-
}
41-
42-
#[IsGranted(attributes: 'ROLE_ADMIN', subject: 'arg2Name')]
27+
#[IsGranted(attribute: 'ROLE_ADMIN', subject: 'arg2Name')]
4328
public function withSubject($arg1Name, $arg2Name)
4429
{
4530
}
4631

47-
#[IsGranted(attributes: 'ROLE_ADMIN', subject: ['arg1Name', 'arg2Name'])]
32+
#[IsGranted(attribute: 'ROLE_ADMIN', subject: ['arg1Name', 'arg2Name'])]
4833
public function withSubjectArray($arg1Name, $arg2Name)
4934
{
5035
}
5136

52-
#[IsGranted(attributes: 'ROLE_ADMIN', subject: 'non_existent')]
37+
#[IsGranted(attribute: 'ROLE_ADMIN', subject: 'non_existent')]
5338
public function withMissingSubject()
5439
{
5540
}
5641

57-
#[IsGranted(attributes: 'ROLE_ADMIN', statusCode: 404, message: 'Not found')]
42+
#[IsGranted(attribute: 'ROLE_ADMIN', message: 'Not found', statusCode: 404)]
5843
public function notFound()
5944
{
6045
}

0 commit comments

Comments
 (0)