diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #37325 Fix the supports() method argument type of the security voter (francoispluchino)

Browse Source 
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
-------

b8192eecab Fix the 'supports' method argument type of the security voter
This commit is contained in:
Fabien Potencier 2020-06-25 11:02:02 +02:00
commit 24350138b9
2 changed files with 76 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php
View File

@ -30,8 +30,21 @@ abstract class Voter implements VoterInterface
$vote = self::ACCESS_ABSTAIN;
foreach ($attributes as $attribute) {
if (!$this->supports($attribute, $subject)) {
continue;
try {
if (!$this->supports($attribute, $subject)) {
continue;
}
} catch (\TypeError $e) {
if (\PHP_VERSION_ID < 80000) {
if (0 === strpos($e->getMessage(), 'Argument 1 passed to')
&& false !== strpos($e->getMessage(), '::supports() must be of the type string')) {
continue;
}
} elseif (false !== strpos($e->getMessage(), 'supports(): Argument #1')) {
continue;
}
throw $e;
}
// as soon as at least one attribute is supported, default is to deny access

73
src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php
View File

@ -27,34 +27,49 @@ class VoterTest extends TestCase
public function getTests()
{
$voter = new VoterTest_Voter();
$integerVoter = new IntegerVoterTest_Voter();
return [
[['EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'],
[['CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access'],
[$voter, ['EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'],
[$voter, ['CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access'],
[['DELETE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute is supported and grants access'],
[['DELETE', 'CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if one attribute is supported and denies access'],
[$voter, ['DELETE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute is supported and grants access'],
[$voter, ['DELETE', 'CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if one attribute is supported and denies access'],
[['CREATE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute grants access'],
[$voter, ['CREATE', 'EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if one attribute grants access'],
[['DELETE'], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'],
[$voter, ['DELETE'], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'],
[['EDIT'], VoterInterface::ACCESS_ABSTAIN, $this, 'ACCESS_ABSTAIN if class is not supported'],
[$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, $this, 'ACCESS_ABSTAIN if class is not supported'],
[['EDIT'], VoterInterface::ACCESS_ABSTAIN, null, 'ACCESS_ABSTAIN if object is null'],
[$voter, ['EDIT'], VoterInterface::ACCESS_ABSTAIN, null, 'ACCESS_ABSTAIN if object is null'],
[[], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attributes were provided'],
[$voter, [], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if no attributes were provided'],
[$voter, [new StringableAttribute()], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'],
[$voter, [new \stdClass()], VoterInterface::ACCESS_ABSTAIN, new \stdClass(), 'ACCESS_ABSTAIN if attributes were not strings'],
[$integerVoter, [42], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute is an integer'],
];
}
/**
* @dataProvider getTests
*/
public function testVote(array $attributes, $expectedVote, $object, $message)
public function testVote(VoterInterface $voter, array $attributes, $expectedVote, $object, $message)
{
$voter = new VoterTest_Voter();
$this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message);
}
public function testVoteWithTypeError()
{
$this->expectException('TypeError');
$this->expectExceptionMessage('Should error');
$voter = new TypeErrorVoterTest_Voter();
$voter->vote($this->token, new \stdClass(), ['EDIT']);
}
}
class VoterTest_Voter extends Voter
@ -69,3 +84,37 @@ class VoterTest_Voter extends Voter
return $object instanceof \stdClass && \in_array($attribute, ['EDIT', 'CREATE']);
}
}
class IntegerVoterTest_Voter extends Voter
{
protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool
{
return 42 === $attribute;
}
protected function supports($attribute, $object): bool
{
return $object instanceof \stdClass && \is_int($attribute);
}
}
class TypeErrorVoterTest_Voter extends Voter
{
protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool
{
return false;
}
protected function supports($attribute, $object): bool
{
throw new \TypeError('Should error');
}
}
class StringableAttribute
{
public function __toString(): string
{
return 'EDIT';
}
}