Merge pull request #47 from symfony-cmf/issue-45/default-voter

dbu · web-flow · commit 548e312ac678 · 2017-02-01T10:13:34.000+01:00
Added security configuration
diff --git a/src/DependencyInjection/CmfResourceRestExtension.php b/src/DependencyInjection/CmfResourceRestExtension.php
@@ -41,6 +41,18 @@ public function load(array $configs, ContainerBuilder $container)
         $loader->load('resource-rest.xml');
 
         $this->configurePayloadAliasRegistry($container, $config['payload_alias_map']);
+        $this->configureSecurityVoter($loader, $container, $config['security']);
+    }
+
+    private function configureSecurityVoter(XmlFileLoader $loader, ContainerBuilder $container, array $config)
+    {
+        if ([] === $config['access_control']) {
+            return;
+        }
+
+        $container->setParameter('cmf_resource_rest.security.access_map', $config['access_control']);
+
+        $loader->load('security.xml');
     }
 
     public function getNamespace()
diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -11,6 +11,7 @@
 
 namespace Symfony\Cmf\Bundle\ResourceRestBundle\DependencyInjection;
 
+use Symfony\Cmf\Bundle\ResourceRestBundle\Controller\ResourceController;
 use Symfony\Component\Config\Definition\Builder\TreeBuilder;
 use Symfony\Component\Config\Definition\ConfigurationInterface;
 
@@ -29,6 +30,39 @@ public function getConfigTreeBuilder()
             ->children()
                 ->integerNode('max_depth')->defaultValue(2)->end()
                 ->booleanNode('expose_payload')->defaultFalse()->end()
+
+                ->arrayNode('security')
+                    ->fixXmlConfig('rule', 'access_control')
+                    ->addDefaultsIfNotSet()
+                    ->children()
+                        ->arrayNode('access_control')
+                            ->defaultValue([])
+                            ->prototype('array')
+                                ->fixXmlConfig('attribute')
+                                ->children()
+                                    ->scalarNode('pattern')->defaultValue('^/')->end()
+                                    ->scalarNode('repository')->defaultNull()->end()
+                                    ->arrayNode('attributes')
+                                        ->defaultValue([ResourceController::ROLE_RESOURCE_READ, ResourceController::ROLE_RESOURCE_WRITE])
+                                        ->prototype('scalar')->end()
+                                    ->end()
+                                    ->arrayNode('require')
+                                        ->isRequired()
+                                        ->requiresAtLeastOneElement()
+                                        ->beforeNormalization()
+                                            ->ifString()
+                                            ->then(function ($v) {
+                                                return [$v];
+                                            })
+                                        ->end()
+                                        ->prototype('scalar')->end()
+                                    ->end() // roles
+                                ->end()
+                            ->end()
+                        ->end() // access_control
+                    ->end()
+                ->end() // security
+
                 ->arrayNode('payload_alias_map')
                     ->useAttributeAsKey('name')
                     ->prototype('array')
@@ -37,7 +71,7 @@ public function getConfigTreeBuilder()
                              ->scalarNode('type')->end()
                          ->end()
                     ->end()
-                ->end()
+                ->end() // payload_alias_map
             ->end();
 
         return $treeBuilder;
diff --git a/src/Resources/config/schema/resource-rest.xsd b/src/Resources/config/schema/resource-rest.xsd
@@ -9,6 +9,7 @@
     <xsd:complexType name="config">
         <xsd:sequence>
             <xsd:element name="payload-alias" type="payload_alias" minOccurs="0" />
+            <xsd:element name="rule" type="rule" minOccurs="0" />
         </xsd:sequence>
 
         <xsd:attribute name="max-depth" type="xsd:integer" default="2" />
@@ -19,5 +20,17 @@
         <xsd:attribute name="type" type="xsd:string" />
     </xsd:complexType>
 
+    <xsd:complexType name="rule">
+        <xsd:sequence>
+            <xsd:element name="require" type="xsd:string" minOccurs="0" />
+            <xsd:element name="attribute" type="xsd:string" minOccurs="0" />
+        </xsd:sequence>
+
+        <xsd:attribute name="pattern" type="xsd:string" />
+        <xsd:attribute name="repository" type="xsd:string" />
+        <xsd:attribute name="require" type="xsd:string" />
+        <xsd:attribute name="attribute" type="xsd:string" />
+    </xsd:complexType>
+
 </xsd:schema>
 
diff --git a/src/Resources/config/security.xml b/src/Resources/config/security.xml
@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+
+    <services>
+        <service id="cmf_resource_rest.security.resource_path_voter" class="Symfony\Cmf\Bundle\ResourceRestBundle\Security\ResourcePathVoter" public="false">
+            <argument type="service" id="security.access.decision_manager" />
+            <argument>%cmf_resource_rest.security.access_map%</argument>
+
+            <tag name="security.voter"/>
+        </service>
+    </services>
+</container>
diff --git a/src/Security/ResourcePathVoter.php b/src/Security/ResourcePathVoter.php
@@ -0,0 +1,76 @@
+<?php
+
+/*
+ * This file is part of the Symfony CMF package.
+ *
+ * (c) 2011-2017 Symfony CMF
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Cmf\Bundle\ResourceRestBundle\Security;
+
+use Symfony\Cmf\Bundle\ResourceRestBundle\Controller\ResourceController;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter;
+
+/**
+ * @author Wouter de Jong <wouter@wouterj.nl>
+ */
+class ResourcePathVoter extends Voter
+{
+    private $accessDecisionManager;
+    private $accessMap;
+
+    public function __construct(AccessDecisionManagerInterface $accessDecisionManager, array $accessMap)
+    {
+        $this->accessDecisionManager = $accessDecisionManager;
+        $this->accessMap = $accessMap;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function supports($attribute, $subject)
+    {
+        return in_array($attribute, [ResourceController::ROLE_RESOURCE_READ, ResourceController::ROLE_RESOURCE_WRITE])
+            && is_array($subject) && isset($subject['repository_name']) && isset($subject['path']);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
+    {
+        foreach ($this->accessMap as $rule) {
+            if (!$this->ruleMatches($rule, $attribute, $subject)) {
+                continue;
+            }
+
+            if ($this->accessDecisionManager->decide($token, $rule['require'])) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private function ruleMatches($rule, $attribute, $subject)
+    {
+        if (!in_array($attribute, $rule['attributes'])) {
+            return false;
+        }
+
+        if (null !== $rule['repository'] && $rule['repository'] !== $subject['repository_name']) {
+            return false;
+        }
+
+        if (!preg_match('{'.$rule['pattern'].'}', $subject['path'])) {
+            return false;
+        }
+
+        return true;
+    }
+}
diff --git a/tests/Features/nesting.feature b/tests/Features/nesting.feature
@@ -11,6 +11,11 @@ Feature: Nesting resources
                     default:
                         type: doctrine/phpcr-odm
                         basepath: /tests/cmf/articles
+
+            cmf_resource_rest:
+                security:
+                    access_control:
+                        - { pattern: '^/', require: IS_AUTHENTICATED_ANONYMOUSLY }
             """
         And there exists an "Article" document at "/cmf/articles/foo":
             | title | Article 1          |
diff --git a/tests/Features/resource_api_phpcr.feature b/tests/Features/resource_api_phpcr.feature
@@ -15,6 +15,9 @@ Feature: PHPCR resource repository
 
             cmf_resource_rest:
                 expose_payload: true
+                security:
+                    access_control:
+                        - { pattern: '^/', require: IS_AUTHENTICATED_ANONYMOUSLY }
             """
 
 
diff --git a/tests/Features/resource_api_phpcr_odm.feature b/tests/Features/resource_api_phpcr_odm.feature
@@ -18,6 +18,9 @@ Feature: PHPCR-ODM resource repository
                     article:
                         repository: doctrine/phpcr-odm
                         type: "Symfony\\Cmf\\Bundle\\ResourceRestBundle\\Tests\\Resources\\TestBundle\\Document\\Article"
+                security:
+                    access_control:
+                        - { pattern: '^/', require: IS_AUTHENTICATED_ANONYMOUSLY }
             """
 
 
diff --git a/tests/Features/security.feature b/tests/Features/security.feature
@@ -11,6 +11,11 @@ Feature: Security
                     security:
                         type: phpcr/phpcr
                         basepath: /tests/cmf/articles
+
+            cmf_resource_rest:
+                security:
+                    access_control:
+                        - { pattern: '^/tests/cmf/articles/private', repository: security, require: ROLE_ADMIN }
             """
         And there exists an "Article" document at "/private/foo":
             | title | Article 1          |
diff --git a/tests/Resources/app/config/config.php b/tests/Resources/app/config/config.php
@@ -10,7 +10,6 @@
  */
 
 use Symfony\Cmf\Bundle\ResourceRestBundle\Tests\Resources\TestBundle\Description\DummyEnhancer;
-use Symfony\Cmf\Bundle\ResourceRestBundle\Tests\Resources\TestBundle\Security\ResourceVoter;
 
 $container->setParameter('cmf_testing.bundle_fqn', 'Symfony\Cmf\Bundle\ResourceRestBundle');
 $loader->import(CMF_TEST_CONFIG_DIR.'/dist/parameters.yml');
@@ -20,8 +19,5 @@
 $loader->import(CMF_TEST_CONFIG_DIR.'/dist/security.yml');
 $loader->import(CMF_TEST_CONFIG_DIR.'/phpcr_odm.php');
 
-$container->register('app.resource_voter', ResourceVoter::class)
-    ->addTag('security.voter');
-
 $container->register('app.dummy_enhancer', DummyEnhancer::class)
     ->addTag('cmf_resource.description.enhancer', ['alias' => 'dummy']);
diff --git a/tests/Unit/DependencyInjection/ConfigurationTest.php b/tests/Unit/DependencyInjection/ConfigurationTest.php
@@ -49,6 +49,13 @@ public function testConfig($source)
             ],
             'max_depth' => 2,
             'expose_payload' => false,
+            'security' => [
+                'access_control' => [
+                    ['pattern' => '^/cms/public', 'attributes' => ['CMF_RESOURCE_READ'], 'require' => ['IS_AUTHENTICATED_ANONYMOUSLY'], 'repository' => null],
+                    ['pattern' => '^/cms/members-only', 'attributes' => ['CMF_RESOURCE_READ'], 'require' => ['ROLE_USER'], 'repository' => null],
+                    ['pattern' => '^/', 'attributes' => ['CMF_RESOURCE_WRITE'], 'require' => ['ROLE_ADMIN'], 'repository' => null],
+                ],
+            ],
         ], [$source]);
     }
 }
diff --git a/tests/Unit/DependencyInjection/fixtures/config.xml b/tests/Unit/DependencyInjection/fixtures/config.xml
@@ -1,9 +1,24 @@
 <container xmlns="http://symfony.com/schema/dic/services"
     xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-    xmlns:cmf_resource_rest="http://cmf.symfony.com/schema/dic/cmf_resource_rest"
+    
     xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd
         http://symfony.com/schema/dic/symfony http://symfony.com/schema/dic/symfony/symfony-1.0.xsd">
-    <cmf_resource_rest:config>
-        <cmf_resource_rest:payload_alias name="article" type="Namespace\Article" repository="doctrine_phpcr_odm" />
-    </cmf_resource_rest:config>
+
+    <config xmlns="http://cmf.symfony.com/schema/dic/cmf_resource_rest">
+
+        <payload-alias name="article" type="Namespace\Article" repository="doctrine_phpcr_odm" />
+
+        <security>
+            <rule pattern="^/cms/public" require="IS_AUTHENTICATED_ANONYMOUSLY">
+                <attribute>CMF_RESOURCE_READ</attribute>
+            </rule>
+
+            <rule pattern="^/cms/members-only" attribute="CMF_RESOURCE_READ">
+                <require>ROLE_USER</require>
+            </rule>
+
+            <rule pattern="^/" attribute="CMF_RESOURCE_WRITE" require="ROLE_ADMIN" />
+        </security>
+    </config>
+
 </container>
diff --git a/tests/Unit/DependencyInjection/fixtures/config.yml b/tests/Unit/DependencyInjection/fixtures/config.yml
@@ -3,3 +3,9 @@ cmf_resource_rest:
         article:
             repository: doctrine_phpcr_odm
             type: Namespace\Article
+
+    security:
+        access_control:
+            - { pattern: ^/cms/public, attributes: [CMF_RESOURCE_READ], require: IS_AUTHENTICATED_ANONYMOUSLY }
+            - { pattern: ^/cms/members-only, attributes: [CMF_RESOURCE_READ], require: ROLE_USER }
+            - { pattern: ^/, attributes: [CMF_RESOURCE_WRITE], require: ROLE_ADMIN }
diff --git a/tests/Unit/Security/ResourcePathVoterTest.php b/tests/Unit/Security/ResourcePathVoterTest.php
@@ -0,0 +1,81 @@
+<?php
+
+/*
+ * This file is part of the Symfony CMF package.
+ *
+ * (c) 2011-2017 Symfony CMF
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Cmf\Bundle\ResourceRestBundle\Tests\Security;
+
+use Symfony\Cmf\Bundle\ResourceRestBundle\Security\ResourcePathVoter;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
+use Symfony\Component\Security\Core\Authorization\Voter\Voter as V;
+
+class ResourcePathVoterTest extends \PHPUnit_Framework_TestCase
+{
+    private $accessDecisionManager;
+
+    protected function setUp()
+    {
+        $this->accessDecisionManager = $this->prophesize(AccessDecisionManagerInterface::class);
+    }
+
+    /**
+     * @dataProvider provideVoteData
+     */
+    public function testVote($rules, $subject, array $attributes, $result)
+    {
+        $token = $this->prophesize(TokenInterface::class)->reveal();
+
+        $this->accessDecisionManager->decide($token, ['ROLE_USER'])->willReturn(true);
+        $this->accessDecisionManager->decide($token, ['ROLE_ADMIN'])->willReturn(false);
+
+        $voter = new ResourcePathVoter($this->accessDecisionManager->reveal(), $rules);
+
+        $this->assertSame($result, $voter->vote($token, $subject, $attributes));
+    }
+
+    public function provideVoteData()
+    {
+        $ruleSet1 = [
+            $this->buildRule('^/', ['ROLE_USER'], ['CMF_RESOURCE_READ']),
+            $this->buildRule('^/cms/private', ['ROLE_ADMIN'], ['CMF_RESOURCE_WRITE']),
+        ];
+
+        return [
+            // Basic behaviour
+            [[$this->buildRule('^/')], $this->buildSubject('/cms/articles/foo'), ['CMF_RESOURCE_READ'], V::ACCESS_GRANTED],
+            [[$this->buildRule('^/')], $this->buildSubject('/cms/articles/foo'), ['CMF_RESOURCE_WRITE'], V::ACCESS_GRANTED],
+            [[$this->buildRule('^/', ['ROLE_ADMIN'])], $this->buildSubject('/cms/articles/foo'), ['CMF_RESOURCE_READ'], V::ACCESS_DENIED],
+
+            // Multiple rules
+            [$ruleSet1, $this->buildSubject('/cms/private/admin'), ['CMF_RESOURCE_READ'], V::ACCESS_GRANTED],
+            [$ruleSet1, $this->buildSubject('/cms/private/admin'), ['CMF_RESOURCE_WRITE'], V::ACCESS_DENIED],
+            [$ruleSet1, $this->buildSubject('/cms/public'), ['CMF_RESOURCE_READ', 'CMF_RESOURCE_WRITE'], V::ACCESS_GRANTED],
+
+            // Unsupported attributes or subjects
+            [[], $this->buildSubject('/cms/articles'), ['CMF_RESOURCE_READ'], V::ACCESS_DENIED],
+            [[$this->buildRule('^/')], $this->buildSubject('/cms/articles'), ['ROLE_USER'], V::ACCESS_ABSTAIN],
+            [[$this->buildRule('^/')], new \stdClass(), ['CMF_RESOURCE_READ'], V::ACCESS_ABSTAIN],
+
+            // Repository name matching
+            [[$this->buildRule('^/')], $this->buildSubject('/cms/articles', 'other_repo'), ['CMF_RESOURCE_READ'], V::ACCESS_DENIED],
+            [[$this->buildRule('^/', ['ROLE_USER'], ['CMF_RESOURCE_READ'], 'other_repo')], $this->buildSubject('/cms/articles'), ['CMF_RESOURCE_READ'], V::ACCESS_DENIED],
+        ];
+    }
+
+    private function buildRule($pattern, $require = ['ROLE_USER'], $attributes = ['CMF_RESOURCE_READ', 'CMF_RESOURCE_WRITE'], $repository = 'default')
+    {
+        return ['pattern' => $pattern, 'attributes' => $attributes, 'require' => $require, 'repository' => $repository];
+    }
+
+    private function buildSubject($path, $repository = 'default')
+    {
+        return ['path' => $path, 'repository_name' => $repository];
+    }
+}
