Skip to content

Commit 4167684

Browse files
derrabusderrabus
committed
[Security] Remove deprecated access decision code
Signed-off-by: Alexander M. Turek <me@derrabus.de>
1 parent af3cae9 commit 4167684

File tree

8 files changed

+16
-167
lines changed

8 files changed

+16
-167
lines changed

‎Authorization/AccessDecisionManager.php

Lines changed: 8 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,6 @@
1414
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1515
use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface;
1616
use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy;
17-
use Symfony\Component\Security\Core\Authorization\Strategy\ConsensusStrategy;
18-
use Symfony\Component\Security\Core\Authorization\Strategy\PriorityStrategy;
19-
use Symfony\Component\Security\Core\Authorization\Strategy\UnanimousStrategy;
2017
use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface;
2118
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
2219
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
@@ -26,61 +23,26 @@
2623
* that use decision voters.
2724
*
2825
* @author Fabien Potencier <fabien@symfony.com>
29-
*
30-
* @final since Symfony 5.4
3126
*/
32-
class AccessDecisionManager implements AccessDecisionManagerInterface
27+
final class AccessDecisionManager implements AccessDecisionManagerInterface
3328
{
34-
/**
35-
* @deprecated use {@see AffirmativeStrategy} instead
36-
*/
37-
public const STRATEGY_AFFIRMATIVE = 'affirmative';
38-
39-
/**
40-
* @deprecated use {@see ConsensusStrategy} instead
41-
*/
42-
public const STRATEGY_CONSENSUS = 'consensus';
43-
44-
/**
45-
* @deprecated use {@see UnanimousStrategy} instead
46-
*/
47-
public const STRATEGY_UNANIMOUS = 'unanimous';
48-
49-
/**
50-
* @deprecated use {@see PriorityStrategy} instead
51-
*/
52-
public const STRATEGY_PRIORITY = 'priority';
53-
5429
private const VALID_VOTES = [
5530
VoterInterface::ACCESS_GRANTED => true,
5631
VoterInterface::ACCESS_DENIED => true,
5732
VoterInterface::ACCESS_ABSTAIN => true,
5833
];
5934

60-
private $voters;
61-
private $votersCacheAttributes;
62-
private $votersCacheObject;
63-
private $strategy;
35+
private iterable $voters;
36+
private array $votersCacheAttributes = [];
37+
private array $votersCacheObject = [];
38+
private AccessDecisionStrategyInterface $strategy;
6439

6540
/**
66-
* @param iterable<mixed, VoterInterface> $voters An array or an iterator of VoterInterface instances
67-
* @param AccessDecisionStrategyInterface|null $strategy The vote strategy
68-
*
69-
* @throws \InvalidArgumentException
41+
* @param iterable<mixed, VoterInterface> $voters An array or an iterator of VoterInterface instances
7042
*/
71-
public function __construct(iterable $voters = [], /* AccessDecisionStrategyInterface */ $strategy = null)
43+
public function __construct(iterable $voters = [], AccessDecisionStrategyInterface $strategy = null)
7244
{
7345
$this->voters = $voters;
74-
if (\is_string($strategy)) {
75-
trigger_deprecation('symfony/security-core', '5.4', 'Passing the access decision strategy as a string is deprecated, pass an instance of "%s" instead.', AccessDecisionStrategyInterface::class);
76-
$allowIfAllAbstainDecisions = 3 <= \func_num_args() && func_get_arg(2);
77-
$allowIfEqualGrantedDeniedDecisions = 4 > \func_num_args() || func_get_arg(3);
78-
79-
$strategy = $this->createStrategy($strategy, $allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions);
80-
} elseif (null !== $strategy && !$strategy instanceof AccessDecisionStrategyInterface) {
81-
throw new \TypeError(sprintf('"%s": Parameter #2 ($strategy) is expected to be an instance of "%s" or null, "%s" given.', __METHOD__, AccessDecisionStrategyInterface::class, get_debug_type($strategy)));
82-
}
83-
8446
$this->strategy = $strategy ?? new AffirmativeStrategy();
8547
}
8648

@@ -102,11 +64,9 @@ public function decide(TokenInterface $token, array $attributes, mixed $object =
10264
}
10365

10466
/**
105-
* @param mixed $object
106-
*
10767
* @return \Traversable<int, int>
10868
*/
109-
private function collectResults(TokenInterface $token, array $attributes, $object): \Traversable
69+
private function collectResults(TokenInterface $token, array $attributes, mixed $object): \Traversable
11070
{
11171
foreach ($this->getVoters($attributes, $object) as $voter) {
11272
$result = $voter->vote($token, $object, $attributes);
@@ -118,25 +78,6 @@ private function collectResults(TokenInterface $token, array $attributes, $objec
11878
}
11979
}
12080

121-
/**
122-
* @throws \InvalidArgumentException if the $strategy is invalid
123-
*/
124-
private function createStrategy(string $strategy, bool $allowIfAllAbstainDecisions, bool $allowIfEqualGrantedDeniedDecisions): AccessDecisionStrategyInterface
125-
{
126-
switch ($strategy) {
127-
case self::STRATEGY_AFFIRMATIVE:
128-
return new AffirmativeStrategy($allowIfAllAbstainDecisions);
129-
case self::STRATEGY_CONSENSUS:
130-
return new ConsensusStrategy($allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions);
131-
case self::STRATEGY_UNANIMOUS:
132-
return new UnanimousStrategy($allowIfAllAbstainDecisions);
133-
case self::STRATEGY_PRIORITY:
134-
return new PriorityStrategy($allowIfAllAbstainDecisions);
135-
}
136-
137-
throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategy));
138-
}
139-
14081
/**
14182
* @return iterable<mixed, VoterInterface>
14283
*/

‎Authorization/Strategy/AffirmativeStrategy.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,7 @@
2424
*/
2525
final class AffirmativeStrategy implements AccessDecisionStrategyInterface, \Stringable
2626
{
27-
/**
28-
* @var bool
29-
*/
30-
private $allowIfAllAbstainDecisions;
27+
private bool $allowIfAllAbstainDecisions;
3128

3229
public function __construct(bool $allowIfAllAbstainDecisions = false)
3330
{

‎Authorization/Strategy/ConsensusStrategy.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,8 @@
3232
*/
3333
final class ConsensusStrategy implements AccessDecisionStrategyInterface, \Stringable
3434
{
35-
private $allowIfAllAbstainDecisions;
36-
private $allowIfEqualGrantedDeniedDecisions;
35+
private bool $allowIfAllAbstainDecisions;
36+
private bool $allowIfEqualGrantedDeniedDecisions;
3737

3838
public function __construct(bool $allowIfAllAbstainDecisions = false, bool $allowIfEqualGrantedDeniedDecisions = true)
3939
{

‎Authorization/Strategy/PriorityStrategy.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
*/
2626
final class PriorityStrategy implements AccessDecisionStrategyInterface, \Stringable
2727
{
28-
private $allowIfAllAbstainDecisions;
28+
private bool $allowIfAllAbstainDecisions;
2929

3030
public function __construct(bool $allowIfAllAbstainDecisions = false)
3131
{

‎Authorization/Strategy/UnanimousStrategy.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
*/
2525
final class UnanimousStrategy implements AccessDecisionStrategyInterface, \Stringable
2626
{
27-
private $allowIfAllAbstainDecisions;
27+
private bool $allowIfAllAbstainDecisions;
2828

2929
public function __construct(bool $allowIfAllAbstainDecisions = false)
3030
{

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CHANGELOG
88
* Remove all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead
99
* Remove methods `getPassword()` and `getSalt()` from `UserInterface`, use `PasswordAuthenticatedUserInterface`
1010
or `LegacyPasswordAuthenticatedUserInterface` instead
11+
* `AccessDecisionManager` requires the strategy to be passed as in instance of `AccessDecisionStrategyInterface`
1112

1213
5.4
1314
---

‎Tests/Authorization/AccessDecisionManagerTest.php

Lines changed: 0 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313

1414
use PHPUnit\Framework\Assert;
1515
use PHPUnit\Framework\TestCase;
16-
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
1716
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1817
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
1918
use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface;
@@ -22,32 +21,6 @@
2221

2322
class AccessDecisionManagerTest extends TestCase
2423
{
25-
use ExpectDeprecationTrait;
26-
27-
/**
28-
* @group legacy
29-
*/
30-
public function testSetUnsupportedStrategy()
31-
{
32-
$this->expectException(\InvalidArgumentException::class);
33-
new AccessDecisionManager([$this->getVoter(VoterInterface::ACCESS_GRANTED)], 'fooBar');
34-
}
35-
36-
/**
37-
* @group legacy
38-
*
39-
* @dataProvider getStrategyTests
40-
*/
41-
public function testStrategies($strategy, $voters, $allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions, $expected)
42-
{
43-
$token = $this->createMock(TokenInterface::class);
44-
45-
$this->expectDeprecation('Since symfony/security-core 5.4: Passing the access decision strategy as a string is deprecated, pass an instance of "Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface" instead.');
46-
$manager = new AccessDecisionManager($voters, $strategy, $allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions);
47-
48-
$this->assertSame($expected, $manager->decide($token, ['ROLE_FOO']));
49-
}
50-
5124
public function provideBadVoterResults(): array
5225
{
5326
return [
@@ -92,66 +65,6 @@ public function decide(\Traversable $results): bool
9265
$this->assertTrue($manager->decide($token, ['ROLE_FOO']));
9366
}
9467

95-
public function getStrategyTests()
96-
{
97-
return [
98-
// affirmative
99-
[AccessDecisionManager::STRATEGY_AFFIRMATIVE, $this->getVoters(1, 0, 0), false, true, true],
100-
[AccessDecisionManager::STRATEGY_AFFIRMATIVE, $this->getVoters(1, 2, 0), false, true, true],
101-
[AccessDecisionManager::STRATEGY_AFFIRMATIVE, $this->getVoters(0, 1, 0), false, true, false],
102-
[AccessDecisionManager::STRATEGY_AFFIRMATIVE, $this->getVoters(0, 0, 1), false, true, false],
103-
[AccessDecisionManager::STRATEGY_AFFIRMATIVE, $this->getVoters(0, 0, 1), true, true, true],
104-
105-
// consensus
106-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(1, 0, 0), false, true, true],
107-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(1, 2, 0), false, true, false],
108-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(2, 1, 0), false, true, true],
109-
110-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(0, 0, 1), false, true, false],
111-
112-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(0, 0, 1), true, true, true],
113-
114-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(2, 2, 0), false, true, true],
115-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(2, 2, 1), false, true, true],
116-
117-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(2, 2, 0), false, false, false],
118-
[AccessDecisionManager::STRATEGY_CONSENSUS, $this->getVoters(2, 2, 1), false, false, false],
119-
120-
// unanimous
121-
[AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(1, 0, 0), false, true, true],
122-
[AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(1, 0, 1), false, true, true],
123-
[AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(1, 1, 0), false, true, false],
124-
125-
[AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(0, 0, 2), false, true, false],
126-
[AccessDecisionManager::STRATEGY_UNANIMOUS, $this->getVoters(0, 0, 2), true, true, true],
127-
128-
// priority
129-
[AccessDecisionManager::STRATEGY_PRIORITY, [
130-
$this->getVoter(VoterInterface::ACCESS_ABSTAIN),
131-
$this->getVoter(VoterInterface::ACCESS_GRANTED),
132-
$this->getVoter(VoterInterface::ACCESS_DENIED),
133-
$this->getVoter(VoterInterface::ACCESS_DENIED),
134-
], true, true, true],
135-
136-
[AccessDecisionManager::STRATEGY_PRIORITY, [
137-
$this->getVoter(VoterInterface::ACCESS_ABSTAIN),
138-
$this->getVoter(VoterInterface::ACCESS_DENIED),
139-
$this->getVoter(VoterInterface::ACCESS_GRANTED),
140-
$this->getVoter(VoterInterface::ACCESS_GRANTED),
141-
], true, true, false],
142-
143-
[AccessDecisionManager::STRATEGY_PRIORITY, [
144-
$this->getVoter(VoterInterface::ACCESS_ABSTAIN),
145-
$this->getVoter(VoterInterface::ACCESS_ABSTAIN),
146-
], false, true, false],
147-
148-
[AccessDecisionManager::STRATEGY_PRIORITY, [
149-
$this->getVoter(VoterInterface::ACCESS_ABSTAIN),
150-
$this->getVoter(VoterInterface::ACCESS_ABSTAIN),
151-
], true, true, true],
152-
];
153-
}
154-
15568
public function testCacheableVoters()
15669
{
15770
$token = $this->createMock(TokenInterface::class);

‎Tests/Authorization/TraceableAccessDecisionManagerTest.php

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1616
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
17+
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
1718
use Symfony\Component\Security\Core\Authorization\DebugAccessDecisionManager;
1819
use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager;
1920
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
@@ -26,11 +27,7 @@ class TraceableAccessDecisionManagerTest extends TestCase
2627
public function testDecideLog(array $expectedLog, array $attributes, $object, array $voterVotes, bool $result)
2728
{
2829
$token = $this->createMock(TokenInterface::class);
29-
30-
$admMock = $this
31-
->getMockBuilder(AccessDecisionManager::class)
32-
->setMethods(['decide'])
33-
->getMock();
30+
$admMock = $this->createMock(AccessDecisionManagerInterface::class);
3431

3532
$adm = new TraceableAccessDecisionManager($admMock);
3633

0 commit comments

Comments
 (0)