ConfigDriver enhancements

.gitignore

@@ -1,3 +1,59 @@
-phpunit.xml
+###
+# .gitignore
+###
+
+###
+# Common pollution
+###
+*~
+*.DS_Store
+*.settings
+*Thumb.db
+
+###
+# Temporary, cache, backup and buffer files
+###
+*.tmp
+*.swp
+*.lock
+~*
+.*.sw[a-z]
+.Trashes
+.sass-cache
+*.swp
+composer.lock
+
+###
+# Development environement common files
+###
+*nbproject
+*.project
+*.idea
+*atlassian-ide-plugin.xml
+*catalog.xml
+*.sublime*
+.Spotlight-V100
+Thumb.db
+Vagrantfile
+.vagrant*
+nbproject/
+
+###
+# Generated files and directories
+###
+coverage/
 vendor/
+cache/
+log/
+app/cache/*
+app/logs/*
+clover.xml
+web/bundles/nelmioapidoc
+
+###
+# Excluded executables
+###
+bin/
+composer.phar
+
 

DependencyInjection/Configuration.php

@@ -69,9 +69,51 @@ public function getConfigTreeBuilder()
                         ->end()
                     ->end()
                     ->arrayNode('method_access_control')
+                        ->beforeNormalization()
+                            ->always(function ($node) {
+                                /** This is a backward compatibility layer */
+                                if (is_array($node)) {
+                                    foreach ($node as $key => $value) {
+                                        if (is_string($value)) {
+                                            $node[$key] = array(
+                                                'pre_authorize' => $value
+                                            );
+                                        }
+                                    }
+                                }
+
+                                return $node;
+                            })
+                        ->end()
                         ->useAttributeAsKey('pattern')
-                        ->prototype('scalar')->isRequired()->cannotBeEmpty()->end()
-                    ->end()
+                        ->prototype('array')
+                            ->children()
+                                ->scalarNode('pattern')->end()
+                                ->scalarNode('pre_authorize')->cannotBeEmpty()->end()
+                                ->arrayNode('secure')
+                                    ->fixXmlConfig('role')
+                                    ->children()
+                                        ->arrayNode('roles')->prototype('scalar')->end()
+                                    ->end()
+                                ->arrayNode('secure_param')
+                                    ->fixXmlConfig('permission')
+                                    ->children()
+                                        ->scalarNode('name')->end()
+                                        ->arrayNode('permissions')->prototype('scalar')->end()
+                                    ->end()
+                                ->arrayNode('secure_return')
+                                    ->fixXmlConfig('permission')
+                                    ->children()
+                                        ->arrayNode('permissions')->prototype('scalar')->end()
+                                    ->end()
+                                ->arrayNode('run_as')
+                                    ->fixXmlConfig('role')
+                                    ->children()
+                                        ->arrayNode('roles')->prototype('scalar')->end()
+                                    ->end()
+                                ->booleanNode('satisfies_parent_security_policy')->end()
+                            ->end()
+                        ->end()
                     ->arrayNode('util')
                         ->addDefaultsIfNotSet()
                         ->children()

Metadata/ClassMetadata.php

@@ -20,7 +20,7 @@
 
 use JMS\SecurityExtraBundle\Exception\RuntimeException;
 use JMS\SecurityExtraBundle\Exception\InvalidArgumentException;
-use Metadata\MethodMetadata;
+use Metadata\MethodMetadata as PlainMethodMetadata;
 use Metadata\MergeableInterface;
 use Metadata\MergeableClassMetadata;
 
@@ -31,7 +31,7 @@
  */
 class ClassMetadata extends MergeableClassMetadata
 {
-    public function addMethodMetadata(MethodMetadata $metadata)
+    public function addMethodMetadata(PlainMethodMetadata $metadata)
     {
         if ($this->reflection->isFinal()) {
             throw new RuntimeException(sprintf('Class "%s" is declared final, and cannot be secured.', $reflection->name));

Metadata/Driver/AnnotationDriver.php

@@ -132,4 +132,4 @@ private function convertMethodAnnotations(\ReflectionMethod $method, array $anno
 
         return $hasSecurityMetadata ? $methodMetadata : null;
     }
-}
+}

Metadata/Driver/ConfigDriver.php

@@ -17,62 +17,137 @@ class ConfigDriver implements DriverInterface
     private $bundles;
     private $config;
 
+    /**
+     * @param array $bundles A list of used bundles indexed by name.
+     * @param array $config  Metadata configuration
+     */
     public function __construct(array $bundles, array $config)
     {
-        uasort($bundles, function($a, $b) {
-            return strlen($b) - strlen($a);
+        uasort($bundles, function($operandA, $operandB) {
+            return strlen($operandB) - strlen($operandA);
         });
 
         foreach ($bundles as $name => $namespace) {
             $bundles[$name] = substr($namespace, 0, strrpos($namespace, '\\'));
         }
 
         $this->bundles = $bundles;
-        $this->config = $config;
+        $this->setConfigs($config);
     }
 
+    /**
+     * {@inheritDoc}
+     */
     public function loadMetadataForClass(\ReflectionClass $class)
     {
         $metadata = new ClassMetadata($class->name);
 
-        foreach ($class->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $method) {
+        $methods = $class->getMethods(
+            \ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED
+        );
+
+        foreach ($methods as $method) {
             if ($method->getDeclaringClass()->name !== $class->name) {
                 continue;
             }
 
-            $expression = null;
-            if (null !== $notation = $this->getControllerNotation($method)) {
-                $expression = $this->getExpressionForSignature($notation);
-            }
+            $methodMetadataConfig = $this->getMethodScopeMetadata($method);
 
-            if (null === $expression && null === $expression =
-                    $this->getExpressionForSignature($method->class.'::'.$method->name)) {
-                continue;
+            $methodMetadata = $this->fromMetadataConfig($method, $methodMetadataConfig);
+
+            if ($methodMetadata) {
+                $metadata->addMethodMetadata($methodMetadata);
             }
+        }
 
-            $methodMetadata = new MethodMetadata($method->class, $method->name);
-            $methodMetadata->roles = array(new Expression($expression));
-            $metadata->addMethodMetadata($methodMetadata);
+        return $this->metadataPostTreatment($metadata);
+    }
+
+    protected function getMethodScopeMetadata(\ReflectionMethod $method)
+    {
+        $configurationFound = null;
+
+        if (null !== $notation = $this->getControllerNotation($method)) {
+            $configurationFound = $this->getConfigForSignature($notation);
         }
 
-        if (!$metadata->methodMetadata) {
-            return null;
+        if (null === $configurationFound) {
+            $configurationFound = $this->getConfigForSignature($method->class.'::'.$method->name);
         }
 
-        return $metadata;
+        return $configurationFound ? $configurationFound : array();
+    }
+
+    protected function fromMetadataConfig(\ReflectionMethod $method, array $configs)
+    {
+        $parameters = array();
+        foreach ($method->getParameters() as $index => $parameter) {
+            $parameters[$parameter->getName()] = $index;
+        }
+
+        $methodMetadata = new MethodMetadata($method->class, $method->name);
+
+        $hasSecurityMetadata = false;
+
+        foreach ($configs as $name => $config) {
+            switch ($name) {
+                case "pre_authorize":
+                    $methodMetadata->roles = array(new Expression($config));
+                    $hasSecurityMetadata = true;
+                    break;
+                case "secure":
+                    $methodMetadata->roles = $config['roles'];
+                    $hasSecurityMetadata =  true;
+                    break;
+                case "secure_param":
+                    $this->assertParamExistsForMethod($parameters, $config['name'], $method->name);
+                    $methodMetadata->addParamPermissions(
+                        $parameters[$config['name']], $config['permissions']
+                    );
+                    $hasSecurityMetadata = true;
+                    break;
+                case "secure_return":
+                    $methodMetadata->returnPermissions = $config['permissions'];
+                    $hasSecurityMetadata = true;
+                    break;
+                case "run_as":
+                    $methodMetadata->runAsRoles = $config['roles'];
+                    $hasSecurityMetadata = true;
+                    break;
+                case "satisfies_parent_security_policy":
+                    $methodMetadata->satisfiesParentSecurityPolicy = true;
+                    $hasSecurityMetadata = true;
+                    break;
+            }
+        }
+
+        return $hasSecurityMetadata ? $methodMetadata : null;
     }
 
-    private function getExpressionForSignature($signature)
+    protected function getConfigForSignature($signature)
     {
-        foreach ($this->config as $pattern => $expr) {
+        $configurationFound = null;
+
+        foreach ($this->config as $pattern => $config) {
             if (!preg_match('#'.$pattern.'#i', $signature)) {
                 continue;
             }
 
-            return $expr;
+            $configurationFound = $config;
+
+            break;
         }
 
-        return null;
+        return $configurationFound;
+    }
+
+    protected function metadataPostTreatment(ClassMetadata $metadata)
+    {
+        if (!$metadata->methodMetadata) {
+            $metadata = null;
+        }
+
+        return $metadata;
     }
 
     // TODO: Is it feasible to reverse-engineer the notation for service controllers?
@@ -81,7 +156,13 @@ private function getControllerNotation(\ReflectionMethod $method)
         $signature = $method->class.'::'.$method->name;
 
         // check if class is a controller
-        if (0 === preg_match('#\\\\Controller\\\\([^\\\\]+)Controller::(.+)Action$#', $signature, $match)) {
+        $matched = preg_match(
+            '#\\\\Controller\\\\([^\\\\]+)Controller::(.+)Action$#',
+            $signature,
+            $match
+        );
+
+        if (!$matched) {
             return null;
         }
 
@@ -96,4 +177,31 @@ private function getControllerNotation(\ReflectionMethod $method)
 
         return null;
     }
-}
+
+    private function assertParamExistsForMethod(array $params, $name, $method)
+    {
+        if (!isset($params[$name])) {
+            throw new \InvalidArgumentException(
+                sprintf(
+                    'The parameter "%s" does not exist for method "%s".',
+                    $name,
+                    $method
+                )
+            );
+        }
+    }
+
+    private function setConfigs(array $config)
+    {
+        $this->config = array();
+        foreach ($config as $key => $value) {
+            if (is_string($value)) {
+                $this->config[$key] = array(
+                    'pre_authorize' => $value
+                );
+            } else {
+                $this->config[$key] = $value;
+            }
+        }
+    }
+}

Security/Authorization/Expression/Compiler/Func/ServiceCallbackFunctionCompiler.php

@@ -54,4 +54,4 @@ public function getName()
     {
         return $this->functionName;
     }
-}
+}

Tests/DependencyInjection/JMSSecurityExtraExtensionTest.php

@@ -60,7 +60,12 @@ public function testConfigLoadWithMethodAccessControl()
             )
         )), $container = $this->getContainer());
 
-        $this->assertEquals(array(':login$' => 'hasRole("FOO")'),
+        $this->assertEquals(
+            array(
+                ':login$' => array(
+                    'pre_authorize' => 'hasRole("FOO")',
+                )
+            ),
             $container->getParameter('security.access.method_access_control'));
     }
 
@@ -71,7 +76,7 @@ public function testConfigLoadThrowsExceptionWhenMethodAccessControlWithoutExpre
     {
         $this->extension->load(array(array(
             'expressions' => false,
-            'method_access_control' => array('foo' => 'bar'),
+            'method_access_control' => array('foo' => 'bar', 'FOO' => 'BAR'),
         )), $this->getContainer());
     }