Skip to content

Commit f70b38a

Browse files
derrabusderrabus
committed
Implement ADM strategies as dedicated classes
Signed-off-by: Alexander M. Turek <me@derrabus.de>
1 parent d859d89 commit f70b38a

24 files changed

+797
-169
lines changed

‎UPGRADE-5.4.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,3 +159,6 @@ Security
159159
}
160160
}
161161
```
162+
* Deprecate passing the strategy as string to `AccessDecisionManager`,
163+
pass an instance of `AccessDecisionStrategyInterface` instead
164+
* Flag `AccessDecisionManager` as `@final`

‎UPGRADE-6.0.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,8 @@ Security
394394
}
395395
}
396396
```
397+
* `AccessDecisionManager` does not accept strings as strategy anymore,
398+
pass an instance of `AccessDecisionStrategyInterface` instead
397399

398400
SecurityBundle
399401
--------------

‎src/Symfony/Bundle/SecurityBundle/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ CHANGELOG
1616
factories instead.
1717
* Deprecate the `always_authenticate_before_granting` option
1818
* Display the roles of the logged-in user in the Web Debug Toolbar
19+
* Add the `security.access_decision_manager.strategy_service` option
1920

2021
5.3
2122
---

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php‎

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
1919
use Symfony\Component\Config\Definition\ConfigurationInterface;
2020
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
21-
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
2221
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
2322
use Symfony\Component\Security\Http\Event\LogoutEvent;
2423
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy;
@@ -30,6 +29,15 @@
3029
*/
3130
class MainConfiguration implements ConfigurationInterface
3231
{
32+
/** @internal */
33+
public const STRATEGY_AFFIRMATIVE = 'affirmative';
34+
/** @internal */
35+
public const STRATEGY_CONSENSUS = 'consensus';
36+
/** @internal */
37+
public const STRATEGY_UNANIMOUS = 'unanimous';
38+
/** @internal */
39+
public const STRATEGY_PRIORITY = 'priority';
40+
3341
private $factories;
3442
private $userProviderFactories;
3543

@@ -65,14 +73,18 @@ public function getConfigTreeBuilder()
6573
return true;
6674
}
6775

68-
if (!isset($v['access_decision_manager']['strategy']) && !isset($v['access_decision_manager']['service'])) {
76+
if (!isset($v['access_decision_manager']['strategy'])
77+
&& !isset($v['access_decision_manager']['service'])
78+
&& !isset($v['access_decision_manager']['strategy_service'])
79+
&& !isset($v['access_decision_manager']['strategy-service'])
80+
) {
6981
return true;
7082
}
7183

7284
return false;
7385
})
7486
->then(function ($v) {
75-
$v['access_decision_manager']['strategy'] = AccessDecisionManager::STRATEGY_AFFIRMATIVE;
87+
$v['access_decision_manager']['strategy'] = self::STRATEGY_AFFIRMATIVE;
7688

7789
return $v;
7890
})
@@ -114,13 +126,22 @@ public function getConfigTreeBuilder()
114126
->values($this->getAccessDecisionStrategies())
115127
->end()
116128
->scalarNode('service')->end()
129+
->scalarNode('strategy_service')->end()
117130
->booleanNode('allow_if_all_abstain')->defaultFalse()->end()
118131
->booleanNode('allow_if_equal_granted_denied')->defaultTrue()->end()
119132
->end()
120133
->validate()
121-
->ifTrue(function ($v) { return isset($v['strategy']) && isset($v['service']); })
134+
->ifTrue(function ($v) { return isset($v['strategy'], $v['service']); })
122135
->thenInvalid('"strategy" and "service" cannot be used together.')
123136
->end()
137+
->validate()
138+
->ifTrue(function ($v) { return isset($v['strategy'], $v['strategy_service']); })
139+
->thenInvalid('"strategy" and "strategy_service" cannot be used together.')
140+
->end()
141+
->validate()
142+
->ifTrue(function ($v) { return isset($v['service'], $v['strategy_service']); })
143+
->thenInvalid('"service" and "strategy_service" cannot be used together.')
144+
->end()
124145
->end()
125146
->end()
126147
;
@@ -507,18 +528,13 @@ private function addPasswordHashersSection(ArrayNodeDefinition $rootNode)
507528
->end();
508529
}
509530

510-
private function getAccessDecisionStrategies()
531+
private function getAccessDecisionStrategies(): array
511532
{
512-
$strategies = [
513-
AccessDecisionManager::STRATEGY_AFFIRMATIVE,
514-
AccessDecisionManager::STRATEGY_CONSENSUS,
515-
AccessDecisionManager::STRATEGY_UNANIMOUS,
533+
return [
534+
self::STRATEGY_AFFIRMATIVE,
535+
self::STRATEGY_CONSENSUS,
536+
self::STRATEGY_UNANIMOUS,
537+
self::STRATEGY_PRIORITY,
516538
];
517-
518-
if (\defined(AccessDecisionManager::class.'::STRATEGY_PRIORITY')) {
519-
$strategies[] = AccessDecisionManager::STRATEGY_PRIORITY;
520-
}
521-
522-
return $strategies;
523539
}
524540
}

‎src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@
3939
use Symfony\Component\PasswordHasher\Hasher\Pbkdf2PasswordHasher;
4040
use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher;
4141
use Symfony\Component\PasswordHasher\Hasher\SodiumPasswordHasher;
42+
use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy;
43+
use Symfony\Component\Security\Core\Authorization\Strategy\ConsensusStrategy;
44+
use Symfony\Component\Security\Core\Authorization\Strategy\PriorityStrategy;
45+
use Symfony\Component\Security\Core\Authorization\Strategy\UnanimousStrategy;
4246
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
4347
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
4448
use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;
@@ -150,12 +154,18 @@ public function load(array $configs, ContainerBuilder $container)
150154

151155
if (isset($config['access_decision_manager']['service'])) {
152156
$container->setAlias('security.access.decision_manager', $config['access_decision_manager']['service']);
157+
} elseif (isset($config['access_decision_manager']['strategy_service'])) {
158+
$container
159+
->getDefinition('security.access.decision_manager')
160+
->addArgument(new Reference($config['access_decision_manager']['strategy_service']));
153161
} else {
154162
$container
155163
->getDefinition('security.access.decision_manager')
156-
->addArgument($config['access_decision_manager']['strategy'])
157-
->addArgument($config['access_decision_manager']['allow_if_all_abstain'])
158-
->addArgument($config['access_decision_manager']['allow_if_equal_granted_denied']);
164+
->addArgument($this->createStrategyDefinition(
165+
$config['access_decision_manager']['strategy'],
166+
$config['access_decision_manager']['allow_if_all_abstain'],
167+
$config['access_decision_manager']['allow_if_equal_granted_denied']
168+
));
159169
}
160170

161171
$container->setParameter('security.access.always_authenticate_before_granting', $config['always_authenticate_before_granting']);
@@ -196,6 +206,25 @@ public function load(array $configs, ContainerBuilder $container)
196206
->addTag('security.voter');
197207
}
198208

209+
/**
210+
* @throws \InvalidArgumentException if the $strategy is invalid
211+
*/
212+
private function createStrategyDefinition(string $strategy, bool $allowIfAllAbstainDecisions, bool $allowIfEqualGrantedDeniedDecisions): Definition
213+
{
214+
switch ($strategy) {
215+
case MainConfiguration::STRATEGY_AFFIRMATIVE:
216+
return new Definition(AffirmativeStrategy::class, [$allowIfAllAbstainDecisions]);
217+
case MainConfiguration::STRATEGY_CONSENSUS:
218+
return new Definition(ConsensusStrategy::class, [$allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions]);
219+
case MainConfiguration::STRATEGY_UNANIMOUS:
220+
return new Definition(UnanimousStrategy::class, [$allowIfAllAbstainDecisions]);
221+
case MainConfiguration::STRATEGY_PRIORITY:
222+
return new Definition(PriorityStrategy::class, [$allowIfAllAbstainDecisions]);
223+
}
224+
225+
throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategy));
226+
}
227+
199228
private function createRoleHierarchy(array $config, ContainerBuilder $container)
200229
{
201230
if (!isset($config['role_hierarchy']) || 0 === \count($config['role_hierarchy'])) {

‎src/Symfony/Bundle/SecurityBundle/Resources/config/schema/security-1.0.xsd‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
<xsd:complexType name="access_decision_manager">
6464
<xsd:attribute name="strategy" type="access_decision_manager_strategy" />
6565
<xsd:attribute name="service" type="xsd:string" />
66+
<xsd:attribute name="strategy-service" type="xsd:string" />
6667
<xsd:attribute name="allow-if-all-abstain" type="xsd:boolean" />
6768
<xsd:attribute name="allow-if-equal-granted-denied" type="xsd:boolean" />
6869
</xsd:complexType>

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@
1717
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
1818
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
1919
use Symfony\Component\DependencyInjection\ContainerBuilder;
20+
use Symfony\Component\DependencyInjection\Definition;
2021
use Symfony\Component\DependencyInjection\Reference;
2122
use Symfony\Component\PasswordHasher\Hasher\NativePasswordHasher;
2223
use Symfony\Component\PasswordHasher\Hasher\Pbkdf2PasswordHasher;
2324
use Symfony\Component\PasswordHasher\Hasher\PlaintextPasswordHasher;
2425
use Symfony\Component\PasswordHasher\Hasher\SodiumPasswordHasher;
2526
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
27+
use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy;
2628
use Symfony\Component\Security\Core\Encoder\NativePasswordEncoder;
2729
use Symfony\Component\Security\Core\Encoder\SodiumPasswordEncoder;
2830
use Symfony\Component\Security\Http\Authentication\AuthenticatorManager;
@@ -1046,7 +1048,7 @@ public function testDefaultAccessDecisionManagerStrategyIsAffirmative()
10461048
{
10471049
$container = $this->getContainer('access_decision_manager_default_strategy');
10481050

1049-
$this->assertSame(AccessDecisionManager::STRATEGY_AFFIRMATIVE, $container->getDefinition('security.access.decision_manager')->getArgument(1), 'Default vote strategy is affirmative');
1051+
$this->assertEquals((new Definition(AffirmativeStrategy::class, [false])), $container->getDefinition('security.access.decision_manager')->getArgument(1), 'Default vote strategy is affirmative');
10501052
}
10511053

10521054
public function testCustomAccessDecisionManagerService()
@@ -1069,9 +1071,17 @@ public function testAccessDecisionManagerOptionsAreNotOverriddenByImplicitStrate
10691071

10701072
$accessDecisionManagerDefinition = $container->getDefinition('security.access.decision_manager');
10711073

1072-
$this->assertSame(AccessDecisionManager::STRATEGY_AFFIRMATIVE, $accessDecisionManagerDefinition->getArgument(1));
1073-
$this->assertTrue($accessDecisionManagerDefinition->getArgument(2));
1074-
$this->assertFalse($accessDecisionManagerDefinition->getArgument(3));
1074+
$this->assertEquals((new Definition(AffirmativeStrategy::class, [true])), $accessDecisionManagerDefinition->getArgument(1));
1075+
}
1076+
1077+
public function testAccessDecisionManagerWithStrategyService()
1078+
{
1079+
$container = $this->getContainer('access_decision_manager_strategy_service');
1080+
1081+
$accessDecisionManagerDefinition = $container->getDefinition('security.access.decision_manager');
1082+
1083+
$this->assertEquals(AccessDecisionManager::class, $accessDecisionManagerDefinition->getClass());
1084+
$this->assertEquals(new Reference('app.custom_access_decision_strategy'), $accessDecisionManagerDefinition->getArgument(1));
10751085
}
10761086

10771087
public function testFirewallUndefinedUserProvider()

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/php/access_decision_manager_strategy_service.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
$container->loadFromExtension('security', [
4+
'enable_authenticator_manager' => true,
5+
'access_decision_manager' => [
6+
'strategy_service' => 'app.custom_access_decision_strategy',
7+
],
8+
'providers' => [
9+
'default' => [
10+
'memory' => [
11+
'users' => [
12+
'foo' => ['password' => 'foo', 'roles' => 'ROLE_USER'],
13+
],
14+
],
15+
],
16+
],
17+
'firewalls' => [
18+
'simple' => ['pattern' => '/login', 'security' => false],
19+
],
20+
]);

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/xml/access_decision_manager_strategy_service.xml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<srv:container xmlns="http://symfony.com/schema/dic/security"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xmlns:srv="http://symfony.com/schema/dic/services"
5+
xsi:schemaLocation="http://symfony.com/schema/dic/services
6+
https://symfony.com/schema/dic/services/services-1.0.xsd
7+
http://symfony.com/schema/dic/security
8+
https://symfony.com/schema/dic/security/security-1.0.xsd">
9+
10+
<config enable-authenticator-manager="true">
11+
<access-decision-manager strategy-service="app.custom_access_decision_strategy" />
12+
13+
<provider name="default">
14+
<memory>
15+
<user identifier="foo" password="foo" roles="ROLE_USER" />
16+
</memory>
17+
</provider>
18+
19+
<firewall name="simple" pattern="/login" security="false" />
20+
</config>
21+
</srv:container>

‎src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/Fixtures/yml/access_decision_manager_strategy_service.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
security:
2+
enable_authenticator_manager: true
3+
access_decision_manager:
4+
strategy_service: app.custom_access_decision_strategy
5+
providers:
6+
default:
7+
memory:
8+
users:
9+
foo: { password: foo, roles: ROLE_USER }
10+
firewalls:
11+
simple: { pattern: /login, security: false }

0 commit comments

Comments
 (0)