Skip to content

Navigation Menu

Sign in
Appearance settings

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 2435013

Browse filesBrowse files
fabpotfabpot
committed
bug #37325 Fix the supports() method argument type of the security voter (francoispluchino)
This PR was submitted for the master branch but it was merged into the 5.0 branch instead. Discussion ---------- Fix the supports() method argument type of the security voter | Q | A | ------------- | --- | Branch? | 5.0 and 5.1 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | ~ | License | MIT | Doc PR | ~ Since adding types to method arguments in the version 5.0 (and therefore also 5.1), there is a type mismatch on the first argument of the `supports()` method of the abstract class `Symfony\Component\Security\Core\Authorization\Voter\Voter`. Indeed, the `supports()` method had in previous versions (4.x), the phpdoc indicating that the argument `$attribute` must be a `string`, but this one is not compatible with the `isGranted()` method of the interface `Symfony\Component\Security\Core\AuthorizationAuthorizationCheckerInterface` whose the `$attribute` argument is of type `mixed`. The problem arises when you have voters extending the abstract class `Voter` positioned before a vote with an attribute of a type other than `string`. Apart from Voters created by third parties, there is the voter `ExpressionVoter` which waits in attribute, an instance of the class `Symfony\Component\ExpressionLanguage\Expression` (you can see the [doc](https://symfony.com/doc/current/security/expressions.html) for an example). Just add a voter extending the abstract class `Voter` with a higher priority than the voter `ExpressionVoter` to get the error: ``` Argument 1 passed to FooVoter::supports() must be of the type string, object given ``` To avoid removing the type of the `$attribute` argument from the method `Symfony\Component\Security\Core\Authorization\Voter\Voter::supports(string $attribute, $subject)`, which can break the backward compatibility, you just have to test in the `vote()` method if the attribute is not a `string` and continue before calling the `supports()` method. Commits ------- b8192ee Fix the 'supports' method argument type of the security voter
2 parents 2caadf1 + b8192ee commit 2435013
Copy full SHA for 2435013

File tree

2 files changed

+76
-14
lines changed
Filter options

2 files changed

+76
-14
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php
+15-2Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,21 @@ public function vote(TokenInterface $token, $subject, array $attributes)
3030
$vote = self::ACCESS_ABSTAIN;
3131

3232
foreach ($attributes as $attribute) {
33-
if (!$this->supports($attribute, $subject)) {
34-
continue;
33+
try {
34+
if (!$this->supports($attribute, $subject)) {
35+
continue;
36+
}
37+
} catch (\TypeError $e) {
38+
if (\PHP_VERSION_ID < 80000) {
39+
if (0 === strpos($e->getMessage(), 'Argument 1 passed to')
40+
&& false !== strpos($e->getMessage(), '::supports() must be of the type string')) {
41+
continue;
42+
}
43+
} elseif (false !== strpos($e->getMessage(), 'supports(): Argument #1')) {
44+
continue;
45+
}
46+
47+
throw $e;
3548
}
3649

3750
// as soon as at least one attribute is supported, default is to deny access

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php
+61-12Lines changed: 61 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,34 +27,49 @@ protected function setUp(): void
2727

2828
public function getTests()
2929
{
30+
$voter = new VoterTest_Voter();
31+
$integerVoter = new IntegerVoterTest_Voter();
32+
3033
return [
31-
[['EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'],
32-
[['CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access'],
34+
[$voter, ['EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'],
35+
[$voter, ['CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access'],
36+
37+
[$voter, ['DELETE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute is supported and grants access'],
38+
[$voter, ['DELETE', 'CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if one attribute is supported and denies access'],
39+
40+
[$voter, ['CREATE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute grants access'],
3341

34-
[['DELETE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute is supported and grants access'],
35-
[['DELETE', 'CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if one attribute is supported and denies access'],
42+
[$voter, ['DELETE'], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'],
3643

37-
[['CREATE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute grants access'],
44+
[$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, $this, 'ACCESS_ABSTAIN if class is not supported'],
3845

39-
[['DELETE'], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'],
46+
[$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, null, 'ACCESS_ABSTAIN if object is null'],
4047

41-
[['EDIT'], VoterInterface::ACCESS_ABSTAIN, $this, 'ACCESS_ABSTAIN if class is not supported'],
48+
[$voter, [], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attributes were provided'],
4249

43-
[['EDIT'], VoterInterface::ACCESS_ABSTAIN, null, 'ACCESS_ABSTAIN if object is null'],
50+
[$voter, [new StringableAttribute()], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'],
4451

45-
[[], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attributes were provided'],
52+
[$voter, [new \stdClass()], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if attributes were not strings'],
53+
54+
[$integerVoter, [42], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute is an integer'],
4655
];
4756
}
4857

4958
/**
5059
* @dataProvider getTests
5160
*/
52-
public function testVote(array $attributes, $expectedVote, $object, $message)
61+
public function testVote(VoterInterface $voter, array $attributes, $expectedVote, $object, $message)
5362
{
54-
$voter = new VoterTest_Voter();
55-
5663
$this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message);
5764
}
65+
66+
public function testVoteWithTypeError()
67+
{
68+
$this->expectException('TypeError');
69+
$this->expectExceptionMessage('Should error');
70+
$voter = new TypeErrorVoterTest_Voter();
71+
$voter->vote($this->token, new \stdClass(), ['EDIT']);
72+
}
5873
}
5974

6075
class VoterTest_Voter extends Voter
@@ -69,3 +84,37 @@ protected function supports(string $attribute, $object): bool
6984
return $object instanceof \stdClass && \in_array($attribute, ['EDIT', 'CREATE']);
7085
}
7186
}
87+
88+
class IntegerVoterTest_Voter extends Voter
89+
{
90+
protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool
91+
{
92+
return 42 === $attribute;
93+
}
94+
95+
protected function supports($attribute, $object): bool
96+
{
97+
return $object instanceof \stdClass && \is_int($attribute);
98+
}
99+
}
100+
101+
class TypeErrorVoterTest_Voter extends Voter
102+
{
103+
protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool
104+
{
105+
return false;
106+
}
107+
108+
protected function supports($attribute, $object): bool
109+
{
110+
throw new \TypeError('Should error');
111+
}
112+
}
113+
114+
class StringableAttribute
115+
{
116+
public function __toString(): string
117+
{
118+
return 'EDIT';
119+
}
120+
}

0 commit comments

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