8000 Assert voter returns valid decision · symfony/symfony@ffdfb4f · GitHub
[go: up one dir, main page]
Skip to content

Commit ffdfb4f

Browse files
jderussejderusse
committed
Assert voter returns valid decision
1 parent 8143f8a commit ffdfb4f

File tree

5 files changed

+46
-0
lines changed

5 files changed

+46
-0
lines changed

‎UPGRADE-5.3.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,8 @@ Form
1212
* Deprecated passing an array as the first argument of the `CheckboxListMapper::mapFormsToData()` method, pass `\Traversable` instead.
1313
* Deprecated passing an array as the second argument of the `RadioListMapper::mapDataToForms()` method, pass `\Traversable` instead.
1414
* Deprecated passing an array as the first argument of the `RadioListMapper::mapFormsToData()` method, pass `\Traversable` instead.
15+
16+
Security
17+
--------
18+
19+
* Deprecated voters that do not return a valid decision when calling the `vote` method.
< 10000 h3 id="heading-:R1dlab:" class="DiffFileHeader-module__file-name--mY1O5">‎UPGRADE-6.0.md
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ Security
159159
in `PreAuthenticatedToken`, `RememberMeToken`, `SwitchUserToken`, `UsernamePasswordToken`,
160160
`DefaultAuthenticationSuccessHandler`.
161161
* Removed the `AbstractRememberMeServices::$providerKey` property in favor of `AbstractRememberMeServices::$firewallName`
162+
* `AccessDecisionManager` now throw an exception when a voter does not return a valid decision.
162163

163164
TwigBundle
164165
----------

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
5.3.0
5+
-----
6+
7+
* Deprecated voters that do not return a valid decision when calling the `vote` method.
8+
49
5.2.0
510
-----
611

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ private function decideAffirmative(TokenInterface $token, array $attributes, $ob
8989

9090
if (VoterInterface::ACCESS_DENIED === $result) {
9191
++$deny;
92+
} elseif (VoterInterface::ACCESS_ABSTAIN !== $result) {
93+
trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class);
9294
}
9395
}
9496

@@ -124,6 +126,8 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje
124126
++$grant;
125127
} elseif (VoterInterface::ACCESS_DENIED === $result) {
126128
++$deny;
129+
} elseif (VoterInterface::ACCESS_ABSTAIN !== $result) {
130+
trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class);
127131
}
128132
}
129133

@@ -161,6 +165,8 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje
161165

162166
if (VoterInterface::ACCESS_GRANTED === $result) {
163167
++$grant;
168+
} elseif (VoterInterface::ACCESS_ABSTAIN !== $result) {
169+
trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class);
164170
}
165171
}
166172
}
@@ -192,6 +198,10 @@ private function decidePriority(TokenInterface $token, array $attributes, $objec
192198
if (VoterInterface::ACCESS_DENIED === $result) {
193199
return false;
194200
}
201+
202+
if (VoterInterface::ACCESS_ABSTAIN !== $result) {
203+
trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class);
204+
}
195205
}
196206

197207
return $this->allowIfAllAbstainDecisions;

‎src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,14 @@
1212
namespace Symfony\Component\Security\Core\Tests\Authorization;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
1516
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
1617
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
1718

1819
class AccessDecisionManagerTest extends TestCase
1920
{
21+
use ExpectDeprecationTrait;
22+
2023
public function testSetUnsupportedStrategy()
2124
{
2225
$this->expectException('InvalidArgumentException');
@@ -34,6 +37,20 @@ public function testStrategies($strategy, $voters, $allowIfAllAbstainDecisions,
3437
$this->assertSame($expected, $manager->decide($token, ['ROLE_FOO']));
3538
}
3639

40+
/**
41+
* @dataProvider provideStrategies
42+
* @group legacy
43+
*/
44+
public function testDeprecatedVoter($strategy)
45+
{
46+
$token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
47+
$manager = new AccessDecisionManager([$this->getVoter(3)], $strategy);
48+
49+
$this->expectDeprecation('Since symfony/security-core 5.3: Returning "3" in "%s::vote()" is deprecated, return one of "Symfony\Component\Security\Core\Authorization\Voter\VoterInterface" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".');
50+
51+
$manager->decide($token, ['ROLE_FOO']);
52+
}
53+
3754
public function getStrategyTests()
3855
{
3956
return [
@@ -94,6 +111,14 @@ public function getStrategyTests()
94111
];
95112
}
96113

114+
public function provideStrategies()
115+
{
116+
yield [AccessDecisionManager::STRATEGY_AFFIRMATIVE];
117+
yield [AccessDecisionManager::STRATEGY_CONSENSUS];
118+
yield [AccessDecisionManager::STRATEGY_UNANIMOUS];
119+
yield [AccessDecisionManager::STRATEGY_PRIORITY];
120+
}
121+
97122
protected function getVoters($grants, $denies, $abstains)
98123
{
99124
$voters = [];

0 commit comments

Comments
 (0)
0