feature #19034 [Security] make it possible to configure a custom acce…

…ss decision manager service (xabbuh) This PR was merged into the 3.4 branch. Discussion ---------- [Security] make it possible to configure a custom access decision manager service | Q | A | | --- | --- | | Branch? | 3.4 | | Bug fix? | no | | New feature? | yes | | BC breaks? | no | | Deprecations? | no | | Tests pass? | yes | | Fixed tickets | #942, #14049, #15295, #16828, #16843, | | License | MIT | | Doc PR | TODO | These changes will make it possible to let users define their own voting strategies without the need for custom compiler passes that replace the built-in `AccessDecisionManager` (see linked issues in the PR table for some use cases). Commits ------- e0913a2 add option to define the access decision manager
symfony · Jul 12, 2017 · afaf299 · afaf299
2 parents 954e9f1 + e0913a2
commit afaf299
Show file tree

Hide file tree

Showing 12 changed files with 196 additions and 7 deletions.
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/MainConfiguration.php
@@ -59,6 +59,26 @@ public function getConfigTreeBuilder()
         $rootNode = $tb->root('security');
 
         $rootNode
+            ->beforeNormalization()
+                ->ifTrue(function ($v) {
+                    if (!isset($v['access_decision_manager'])) {
+                        return true;
+                    }
+
+                    if (!isset($v['access_decision_manager']['strategy']) && !isset($v['access_decision_manager']['service'])) {
+                        return true;
+                    }
+
+                    return false;
+                })
+                ->then(function ($v) {
+                    $v['access_decision_manager'] = array(
+                        'strategy' => AccessDecisionManager::STRATEGY_AFFIRMATIVE,
+                    );
+
+                    return $v;
+                })
+            ->end()
             ->children()
                 ->scalarNode('access_denied_url')->defaultNull()->example('/foo/error403')->end()
                 ->enumNode('session_fixation_strategy')
@@ -73,11 +93,15 @@ public function getConfigTreeBuilder()
                     ->children()
                         ->enumNode('strategy')
                             ->values(array(AccessDecisionManager::STRATEGY_AFFIRMATIVE, AccessDecisionManager::STRATEGY_CONSENSUS, AccessDecisionManager::STRATEGY_UNANIMOUS))
-                            ->defaultValue(AccessDecisionManager::STRATEGY_AFFIRMATIVE)
                         ->end()
+                        ->scalarNode('service')->end()
                         ->booleanNode('allow_if_all_abstain')->defaultFalse()->end()
                         ->booleanNode('allow_if_equal_granted_denied')->defaultTrue()->end()
                     ->end()
+                    ->validate()
+                        ->ifTrue(function ($v) { return isset($v['strategy']) && isset($v['service']); })
+                        ->thenInvalid('"strategy" and "service" cannot be used together.')
+                    ->end()
                 ->end()
             ->end()
         ;

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
@@ -83,12 +83,17 @@ public function load(array $configs, ContainerBuilder $container)
         $container->setParameter('security.access.denied_url', $config['access_denied_url']);
         $container->setParameter('security.authentication.manager.erase_credentials', $config['erase_credentials']);
         $container->setParameter('security.authentication.session_strategy.strategy', $config['session_fixation_strategy']);
-        $container
-            ->getDefinition('security.access.decision_manager')
-            ->addArgument($config['access_decision_manager']['strategy'])
-            ->addArgument($config['access_decision_manager']['allow_if_all_abstain'])
-            ->addArgument($config['access_decision_manager']['allow_if_equal_granted_denied'])
-        ;
+
+        if (isset($config['access_decision_manager']['service'])) {
+            $container->setAlias('security.access.decision_manager', $config['access_decision_manager']['service']);
+        } else {
+            $container
+                ->getDefinition('security.access.decision_manager')
+                ->addArgument($config['access_decision_manager']['strategy'])
+                ->addArgument($config['access_decision_manager']['allow_if_all_abstain'])
+                ->addArgument($config['access_decision_manager']['allow_if_equal_granted_denied']);
+        }
+
         $container->setParameter('security.access.always_authenticate_before_granting', $config['always_authenticate_before_granting']);
         $container->setParameter('security.authentication.hide_user_not_found', $config['hide_user_not_found']);
 

diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DependencyInjection/CompleteConfigurationTest.php
@@ -17,6 +17,7 @@
 use Symfony\Bundle\SecurityBundle\SecurityBundle;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\SecurityExtension;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
 
 abstract class CompleteConfigurationTest extends TestCase
 {
@@ -357,6 +358,29 @@ public function testUserPasswordEncoderCommandIsRegistered()
         $this->assertTrue($this->getContainer('remember_me_options')->has('security.console.user_password_encoder_command'));
     }
 
+    public function testDefaultAccessDecisionManagerStrategyIsAffirmative()
+    {
+        $container = $this->getContainer('access_decision_manager_default_strategy');
+
+        $this->assertSame(AccessDecisionManager::STRATEGY_AFFIRMATIVE, $container->getDefinition('security.access.decision_manager')->getArgument(1), 'Default vote strategy is affirmative');
+    }
+
+    public function testCustomAccessDecisionManagerService()
+    {
+        $container = $this->getContainer('access_decision_manager_service');
+
+        $this->assertSame('app.access_decision_manager', (string) $container->getAlias('security.access.decision_manager'), 'The custom access decision manager service is aliased');
+    }
+
+    /**
+     * @expectedException \Symfony\Component\Config\Definition\Exception\InvalidConfigurationException
+     * @expectedExceptionMessage "strategy" and "service" cannot be used together.
+     */
+    public function testAccessDecisionManagerServiceAndStrategyCannotBeUsedAtTheSameTime()
+    {
+        $container = $this->getContainer('access_decision_manager_service_and_strategy');
+    }
+
     protected function getContainer($file)
     {
         $file = $file.'.'.$this->getFileExtension();

diff --git a/...undle/Tests/DependencyInjection/Fixtures/php/access_decision_manager_default_strategy.php b/...undle/Tests/DependencyInjection/Fixtures/php/access_decision_manager_default_strategy.php
@@ -0,0 +1,16 @@
+<?php
+
+$container->loadFromExtension('security', array(
+    'providers' => array(
+        'default' => array(
+            'memory' => array(
+                'users' => array(
+                    'foo' => array('password' => 'foo', 'roles' => 'ROLE_USER'),
+                ),
+            ),
+        ),
+    ),
+    'firewalls' => array(
+        'simple' => array('pattern' => '/login', 'security' => false),
+    ),
+));
diff --git a/...SecurityBundle/Tests/DependencyInjection/Fixtures/php/access_decision_manager_service.php b/...SecurityBundle/Tests/DependencyInjection/Fixtures/php/access_decision_manager_service.php
@@ -0,0 +1,19 @@
+<?php
+
+$container->loadFromExtension('security', array(
+    'access_decision_manager' => array(
+        'service' => 'app.access_decision_manager',
+    ),
+    'providers' => array(
+        'default' => array(
+            'memory' => array(
+                'users' => array(
+                    'foo' => array('password' => 'foo', 'roles' => 'ROLE_USER'),
+                ),
+            ),
+        ),
+    ),
+    'firewalls' => array(
+        'simple' => array('pattern' => '/login', 'security' => false),
+    ),
+));
diff --git a/...e/Tests/DependencyInjection/Fixtures/php/access_decision_manager_service_and_strategy.php b/...e/Tests/DependencyInjection/Fixtures/php/access_decision_manager_service_and_strategy.php
@@ -0,0 +1,20 @@
+<?php
+
+$container->loadFromExtension('security', array(
+    'access_decision_manager' => array(
+        'service' => 'app.access_decision_manager',
+        'strategy' => 'affirmative',
+    ),
+    'providers' => array(
+        'default' => array(
+            'memory' => array(
+                'users' => array(
+                    'foo' => array('password' => 'foo', 'roles' => 'ROLE_USER'),
+                ),
+            ),
+        ),
+    ),
+    'firewalls' => array(
+        'simple' => array('pattern' => '/login', 'security' => false),
+    ),
+));
diff --git a/...undle/Tests/DependencyInjection/Fixtures/xml/access_decision_manager_default_strategy.xml b/...undle/Tests/DependencyInjection/Fixtures/xml/access_decision_manager_default_strategy.xml
@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<srv:container xmlns="http://symfony.com/schema/dic/security"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns:srv="http://symfony.com/schema/dic/services"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <config>
+        <provider name="default">
+            <memory>
+                <user name="foo" password="foo" roles="ROLE_USER" />
+            </memory>
+        </provider>
+
+        <firewall name="simple" pattern="/login" security="false" />
+    </config>
+</srv:container>
diff --git a/...SecurityBundle/Tests/DependencyInjection/Fixtures/xml/access_decision_manager_service.xml b/...SecurityBundle/Tests/DependencyInjection/Fixtures/xml/access_decision_manager_service.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<srv:container xmlns="http://symfony.com/schema/dic/security"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns:srv="http://symfony.com/schema/dic/services"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <config>
+        <access-decision-manager service="app.access_decision_manager" />
+
+        <provider name="default">
+            <memory>
+                <user name="foo" password="foo" roles="ROLE_USER" />
+            </memory>
+        </provider>
+
+        <firewall name="simple" pattern="/login" security="false" />
+    </config>
+</srv:container>
diff --git a/...e/Tests/DependencyInjection/Fixtures/xml/access_decision_manager_service_and_strategy.xml b/...e/Tests/DependencyInjection/Fixtures/xml/access_decision_manager_service_and_strategy.xml
@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<srv:container xmlns="http://symfony.com/schema/dic/security"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xmlns:srv="http://symfony.com/schema/dic/services"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <config>
+        <access-decision-manager service="app.access_decision_manager" strategy="affirmative" />
+
+        <provider name="default">
+            <memory>
+                <user name="foo" password="foo" roles="ROLE_USER" />
+            </memory>
+        </provider>
+
+        <firewall name="simple" pattern="/login" security="false" />
+    </config>
+</srv:container>
diff --git a/...undle/Tests/DependencyInjection/Fixtures/yml/access_decision_manager_default_strategy.yml b/...undle/Tests/DependencyInjection/Fixtures/yml/access_decision_manager_default_strategy.yml
@@ -0,0 +1,8 @@
+security:
+    providers:
+        default:
+            memory:
+                users:
+                    foo: { password: foo, roles: ROLE_USER }
+    firewalls:
+        simple: { pattern: /login, security: false }
diff --git a/...SecurityBundle/Tests/DependencyInjection/Fixtures/yml/access_decision_manager_service.yml b/...SecurityBundle/Tests/DependencyInjection/Fixtures/yml/access_decision_manager_service.yml
@@ -0,0 +1,10 @@
+security:
+    access_decision_manager:
+        service: app.access_decision_manager
+    providers:
+        default:
+            memory:
+                users:
+                    foo: { password: foo, roles: ROLE_USER }
+    firewalls:
+        simple: { pattern: /login, security: false }
diff --git a/...e/Tests/DependencyInjection/Fixtures/yml/access_decision_manager_service_and_strategy.yml b/...e/Tests/DependencyInjection/Fixtures/yml/access_decision_manager_service_and_strategy.yml
@@ -0,0 +1,11 @@
+security:
+    access_decision_manager:
+        service: app.access_decision_manager
+        strategy: affirmative
+    providers:
+        default:
+            memory:
+                users:
+                    foo: { password: foo, roles: ROLE_USER }
+    firewalls:
+        simple: { pattern: /login, security: false }