diff --git a/src/DependencyInjection/CmfResourceRestExtension.php b/src/DependencyInjection/CmfResourceRestExtension.php index 2a71cff..a3c5e02 100644 --- 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 index fec86a1..5866494 100644 --- 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 index 54e4731..eefc9b3 100644 --- a/src/Resources/config/schema/resource-rest.xsd +++ b/src/Resources/config/schema/resource-rest.xsd @@ -9,6 +9,7 @@ + @@ -19,5 +20,17 @@ + + + + + + + + + + + + diff --git a/src/Resources/config/security.xml b/src/Resources/config/security.xml new file mode 100644 index 0000000..6144698 --- /dev/null +++ b/src/Resources/config/security.xml @@ -0,0 +1,15 @@ + + + + + + + + %cmf_resource_rest.security.access_map% + + + + + diff --git a/src/Security/ResourcePathVoter.php b/src/Security/ResourcePathVoter.php new file mode 100644 index 0000000..c43979e --- /dev/null +++ b/src/Security/ResourcePathVoter.php @@ -0,0 +1,76 @@ + + */ +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 index 7ade054..c89c01e 100644 --- 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 index 0393640..45797c8 100644 --- 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 index d96807b..b850643 100644 --- 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 index 5781389..537b106 100644 --- 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 index a799694..5f55871 100644 --- 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 index fa99a31..b8aab6b 100644 --- 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 index f0f78b0..e97e43b 100644 --- a/tests/Unit/DependencyInjection/fixtures/config.xml +++ b/tests/Unit/DependencyInjection/fixtures/config.xml @@ -1,9 +1,24 @@ - - - + + + + + + + + CMF_RESOURCE_READ + + + + ROLE_USER + + + + + + diff --git a/tests/Unit/DependencyInjection/fixtures/config.yml b/tests/Unit/DependencyInjection/fixtures/config.yml index 33ffc0a..4c6949e 100644 --- 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 new file mode 100644 index 0000000..a332357 --- /dev/null +++ b/tests/Unit/Security/ResourcePathVoterTest.php @@ -0,0 +1,81 @@ +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]; + } +}