Skip to content

Commit 1524d5b

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

25 files changed

+776
-169
lines changed

‎UPGRADE-5.4.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,6 @@ Security
3939
* Deprecate `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods without replacement.
4040
Security tokens won't have an "authenticated" flag anymore, so they will always be considered authenticated
4141
* Deprecate `DeauthenticatedEvent`, use `TokenDeauthenticatedEvent` instead
42+
* Deprecate passing the strategy as string to `AccessDecisionManager`,
43+
pass an instance of `AccessDecisionStrategyInterface` instead
44+
* Flag `AccessDecisionManager` as `@final`

‎UPGRADE-6.0.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,8 @@ Security
325325
* Remove `TokenInterface:isAuthenticated()` and `setAuthenticated()` methods without replacement.
326326
Security tokens won't have an "authenticated" flag anymore, so they will always be considered authenticated
327327
* Remove `DeauthenticatedEvent`, use `TokenDeauthenticatedEvent` instead
328+
* `AccessDecisionManager` does not accept strings as strategy anymore,
329+
pass an instance of `AccessDecisionStrategyInterface` instead
328330

329331
SecurityBundle
330332
--------------

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ CHANGELOG
55
---
66

77
* Deprecate the `always_authenticate_before_granting` option
8+
* Add the `security.access_decision_manager.strategy_service` option
89

910
5.3
1011
---

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

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
1717
use Symfony\Component\Config\Definition\ConfigurationInterface;
1818
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
19-
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
2019
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
2120
use Symfony\Component\Security\Http\Event\LogoutEvent;
2221
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy;
@@ -28,6 +27,15 @@
2827
*/
2928
class MainConfiguration implements ConfigurationInterface
3029
{
30+
/** @internal */
31+
public const STRATEGY_AFFIRMATIVE = 'affirmative';
32+
/** @internal */
33+
public const STRATEGY_CONSENSUS = 'consensus';
34+
/** @internal */
35+
public const STRATEGY_UNANIMOUS = 'unanimous';
36+
/** @internal */
37+
public const STRATEGY_PRIORITY = 'priority';
38+
3139
private $factories;
3240
private $userProviderFactories;
3341

@@ -54,14 +62,18 @@ public function getConfigTreeBuilder()
5462
return true;
5563
}
5664

57-
if (!isset($v['access_decision_manager']['strategy']) && !isset($v['access_decision_manager']['service'])) {
65+
if (!isset($v['access_decision_manager']['strategy'])
66+
&& !isset($v['access_decision_manager']['service'])
67+
&& !isset($v['access_decision_manager']['strategy_service'])
68+
&& !isset($v['access_decision_manager']['strategy-service'])
69+
) {
5870
return true;
5971
}
6072

6173
return false;
6274
})
6375
->then(function ($v) {
64-
$v['access_decision_manager']['strategy'] = AccessDecisionManager::STRATEGY_AFFIRMATIVE;
76+
$v['access_decision_manager']['strategy'] = self::STRATEGY_AFFIRMATIVE;
6577

6678
return $v;
6779
})
@@ -103,13 +115,24 @@ public function getConfigTreeBuilder()
103115
->values($this->getAccessDecisionStrategies())
104116
->end()
105117
->scalarNode('service')->end()
118+
->scalarNode('strategy_service')->end()
106119
->booleanNode('allow_if_all_abstain')->defaultFalse()->end()
107120
->booleanNode('allow_if_equal_granted_denied')->defaultTrue()->end()
108121
->end()
109122
->validate()
110-
->ifTrue(function ($v) { return isset($v['strategy']) && isset($v['service']); })
123+
->ifTrue(function ($v) { return isset($v['strategy'], $v['service']); })
111124
->thenInvalid('"strategy" and "service" cannot be used together.')
112125
->end()
126+
->validate()
127+
->ifTrue(function ($v) {
128+
return isset($v['strategy'], $v['strategy_service']);
129+
})
130+
->thenInvalid('"strategy" and "strategy_service" cannot be used together.')
131+
->end()
132+
->validate()
133+
->ifTrue(function ($v) { return isset($v['service'], $v['strategy_service']); })
134+
->thenInvalid('"service" and "strategy_service" cannot be used together.')
135+
->end()
113136
->end()
114137
->end()
115138
;
@@ -498,18 +521,13 @@ private function addPasswordHashersSection(ArrayNodeDefinition $rootNode)
498521
->end();
499522
}
500523

501-
private function getAccessDecisionStrategies()
524+
private function getAccessDecisionStrategies(): array
502525
{
503-
$strategies = [
504-
AccessDecisionManager::STRATEGY_AFFIRMATIVE,
505-
AccessDecisionManager::STRATEGY_CONSENSUS,
506-
AccessDecisionManager::STRATEGY_UNANIMOUS,
526+
return [
527+
self::STRATEGY_AFFIRMATIVE,
528+
self::STRATEGY_CONSENSUS,
529+
self::STRATEGY_UNANIMOUS,
530+
self::STRATEGY_PRIORITY,
507531
];
508-
509-
if (\defined(AccessDecisionManager::class.'::STRATEGY_PRIORITY')) {
510-
$strategies[] = AccessDecisionManager::STRATEGY_PRIORITY;
511-
}
512-
513-
return $strategies;
514532
}
515533
}

‎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;
@@ -160,12 +164,18 @@ public function load(array $configs, ContainerBuilder $container)
160164

161165
if (isset($config['access_decision_manager']['service'])) {
162166
$container->setAlias('security.access.decision_manager', $config['access_decision_manager']['service']);
167+
} elseif (isset($config['access_decision_manager']['strategy_service'])) {
168+
$container
169+
->getDefinition('security.access.decision_manager')
170+
->addArgument(new Reference($config['access_decision_manager']['strategy_service']));
163171
} else {
164172
$container
165173
->getDefinition('security.access.decision_manager')
166-
->addArgument($config['access_decision_manager']['strategy'])
167-
->addArgument($config['access_decision_manager']['allow_if_all_abstain'])
168-
->addArgument($config['access_decision_manager']['allow_if_equal_granted_denied']);
174+
->addArgument($this->createStrategyDefinition(
175+
$config['access_decision_manager']['strategy'],
176+
$config['access_decision_manager']['allow_if_all_abstain'],
177+
$config['access_decision_manager']['allow_if_equal_granted_denied']
178+
));
169179
}
170180

171181
$container->setParameter('security.access.always_authenticate_before_granting', $config['always_authenticate_before_granting']);
@@ -206,6 +216,25 @@ public function load(array $configs, ContainerBuilder $container)
206216
->addTag('security.voter');
207217
}
208218

219+
/**
220+
* @throws \InvalidArgumentException if the $strategy is invalid
221+
*/
222+
private function createStrategyDefinition(string $strategy, bool $allowIfAllAbstainDecisions, bool $allowIfEqualGrantedDeniedDecisions): Definition
223+
{
224+
switch ($strategy) {
225+
case MainConfiguration::STRATEGY_AFFIRMATIVE:
226+
return new Definition(AffirmativeStrategy::class, [$allowIfAllAbstainDecisions]);
227+
case MainConfiguration::STRATEGY_CONSENSUS:
228+
return new Definition(ConsensusStrategy::class, [$allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions]);
229+
case MainConfiguration::STRATEGY_UNANIMOUS:
230+
return new Definition(UnanimousStrategy::class, [$allowIfAllAbstainDecisions]);
231+
case MainConfiguration::STRATEGY_PRIORITY:
232+
return new Definition(PriorityStrategy::class, [$allowIfAllAbstainDecisions]);
233+
}
234+
235+
throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $strategy));
236+
}
237+
209238
private function createRoleHierarchy(array $config, ContainerBuilder $container)
210239
{
211240
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)