rebase on master / adapt the TraceableAccessDecisionManager and the s…

…ecurity web profiler
symfony · fancyweb · Jan 6, 2017 · Jan 6, 2017 · Jan 6, 2017 · Jan 6, 2017
commit 50155a1f1cc535cd13eda3036db0034a09fdd9c1
diff --git a/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php b/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php
@@ -127,14 +127,14 @@ public function collect(Request $request, Response $response, \Exception $except
                 return $decision;
             }, $this->accessDecisionManager->getDecisionLog());
 
-            $this->data['voter_strategy'] = $this->accessDecisionManager->getStrategy();
+            $this->data['voter_default_strategy'] = $this->accessDecisionManager->getDefaultStrategy();
 
             foreach ($this->accessDecisionManager->getVoters() as $voter) {
                 $this->data['voters'][] = get_class($voter);
             }
         } else {
             $this->data['access_decision_log'] = array();
-            $this->data['voter_strategy'] = 'unknown';
+            $this->data['voter_default_strategy'] = 'unknown';
             $this->data['voters'] = array();
         }
 
@@ -263,13 +263,13 @@ public function getVoters()
     }
 
     /**
-     * Returns the strategy configured for the security voters.
+     * Returns the default strategy configured for the security voters.
      *
      * @return string
      */
-    public function getVoterStrategy()
+    public function getVoterDefaultStrategy()
     {
-        return $this->data['voter_strategy'];
+        return $this->data['voter_default_strategy'];
     }
 
     /**

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddStrategyResolversPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddStrategyResolversPass.php
@@ -11,12 +11,15 @@
 
 namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;
 
-use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\Security\Core\Authorization\StrategyResolverInterface;
 
 class AddStrategyResolversPass implements CompilerPassInterface
 {
+    use PriorityTaggedServiceTrait;
+
     /**
      * {@inheritdoc}
      */
@@ -26,21 +29,15 @@ public function process(ContainerBuilder $container)
             return;
         }
 
-        $strategyResolvers = new \SplPriorityQueue();
-        foreach ($container->findTaggedServiceIds('security.strategy_resolver') as $id => $attributes) {
-            $class = $container->getDefinition($id)->getClass();
-            $interface = 'Symfony\Component\Security\Core\Authorization\StrategyResolverInterface';
-            if (!is_subclass_of($class, $interface)) {
-                throw new \InvalidArgumentException(sprintf('Service "%s" must implement interface "%s".', $id, $interface));
+        $strategyResolverReferences = $this->findAndSortTaggedServices('security.strategy_resolver', $container);
+        foreach ($strategyResolverReferences as $strategyResolverReference) {
+            $strategyResolverServiceId = $strategyResolverReference->__toString();
+            $class = $container->getDefinition($strategyResolverServiceId)->getClass();
+            if (!is_subclass_of($class, StrategyResolverInterface::class)) {
+                throw new \InvalidArgumentException(sprintf('Service "%s" must implement interface "%s".', $strategyResolverServiceId, StrategyResolverInterface::class));
             }
-
-            $priority = isset($attributes[0]['priority']) ? $attributes[0]['priority'] : 0;
-            $strategyResolvers->insert(new Reference($id), $priority);
         }
 
-        $strategyResolvers = iterator_to_array($strategyResolvers);
-        ksort($strategyResolvers);
-
-        $container->getDefinition('security.access.decision_manager')->replaceArgument(4, array_values($strategyResolvers));
+        $container->getDefinition('security.access.decision_manager')->replaceArgument(4, $strategyResolverReferences);
     }
 }
diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig
@@ -205,8 +205,8 @@
 
         <div class="metrics">
             <div class="metric">
-                <span class="value">{{ collector.voterStrategy|default('unknown') }}</span>
-                <span class="label">Strategy</span>
+                <span class="value">{{ collector.voterDefaultStrategy|default('unknown') }}</span>
+                <span class="label">Default strategy</span>
             </div>
         </div>
 
@@ -235,13 +235,15 @@
         <table class="decision-log">
             <col style="width: 30px">
             <col style="width: 120px">
+            <col style="width: 120px">
             <col style="width: 25%">
-            <col style="width: 60%">
+            <col style="width: 50%">
 
             <thead>
                 <tr>
                     <th>#</th>
                     <th>Result</th>
+                    <th>Strategy</th>
                     <th>Attributes</th>
                     <th>Object</th>
                 </tr>
@@ -257,6 +259,7 @@
                                 : '<span class="label status-error same-width">DENIED</span>'
                             }}
                         </td>
+                        <td>{{ decision.strategy }}</td>
                         <td>{{ decision.attributes|length == 1 ? decision.attributes|first : profiler_dump(decision.attributes) }}</td>
                         <td>{{ profiler_dump(decision.object) }}</td>
                     </tr>

diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php
@@ -27,7 +27,7 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
     const STRATEGY_UNANIMOUS = 'unanimous';
 
     private $voters;
-    private $defaultStrategyMethod;
+    private $defaultStrategy;
     private $allowIfAllAbstainDecisions;
     private $allowIfEqualGrantedDeniedDecisions;
 
@@ -49,15 +49,19 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
      */
     public function __construct(array $voters = array(), $defaultStrategy = self::STRATEGY_AFFIRMATIVE, $allowIfAllAbstainDecisions = false, $allowIfEqualGrantedDeniedDecisions = true, array $strategyResolvers = array())
     {
-        $defaultStrategyMethod = $this->getStrategyMethod($defaultStrategy);
-        if (!is_callable(array($this, $defaultStrategyMethod))) {
+        if (!in_array($defaultStrategy, array(
+            self::STRATEGY_AFFIRMATIVE,
+            self::STRATEGY_CONSENSUS,
+            self::STRATEGY_UNANIMOUS
+        ))) {
             throw new \InvalidArgumentException(sprintf('The strategy "%s" is not supported.', $defaultStrategy));
         }
 
         $this->voters = $voters;
-        $this->defaultStrategyMethod = $defaultStrategyMethod;
+        $this->defaultStrategy = $defaultStrategy;
         $this->allowIfAllAbstainDecisions = (bool) $allowIfAllAbstainDecisions;
         $this->allowIfEqualGrantedDeniedDecisions = (bool) $allowIfEqualGrantedDeniedDecisions;
+        $this->strategyResolvers = $strategyResolvers;
     }
 
     /**
@@ -75,27 +79,40 @@ public function setVoters(array $voters)
      */
     public function decide(TokenInterface $token, array $attributes, $object = null)
     {
-        $strategyMethod = $this->defaultStrategyMethod;
+        $strategyMethod = 'decide' . ucfirst($this->getStrategy($token, $attributes, $object));
+
+        return $this->{$strategyMethod}($token, $attributes, $object);
+    }
+
+    /**
+     * @param TokenInterface $token
+     * @param array $attributes
+     * @param mixed $object
+     *
+     * @return string
+     */
+    public function getStrategy(TokenInterface $token, array $attributes, $object = null)
+    {
+        $strategy = $this->defaultStrategy;
         /* @var $strategyResolver StrategyResolverInterface */
         foreach ($this->strategyResolvers as $strategyResolver) {
             if ($strategyResolver->supports($token, $attributes, $object)) {
                 $resolvedStrategy = $strategyResolver->getStrategy($token, $attributes, $object);
-                if (!is_string($resolvedStrategy)) {
-                    continue;
-                }
-
-                $resolvedStrategyMethod = $this->getStrategyMethod($resolvedStrategy);
-                if (!is_callable(array($this, $resolvedStrategyMethod))) {
+                if (!in_array($resolvedStrategy, array(
+                    self::STRATEGY_AFFIRMATIVE,
+                    self::STRATEGY_CONSENSUS,
+                    self::STRATEGY_UNANIMOUS
+                ))) {
                     continue;
                 }
 
-                $strategyMethod = $resolvedStrategyMethod;
+                $strategy = $resolvedStrategy;
 
                 break;
             }
         }
 
-        return $this->{$strategyMethod}($token, $attributes, $object);
+        return $strategy;
     }
 
     /**
@@ -214,14 +231,4 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje
 
         return $this->allowIfAllAbstainDecisions;
     }
-
-    /**
-     * @param string $strategy
-     *
-     * @return string
-     */
-    private function getStrategyMethod($strategy)
-    {
-        return 'decide' . ucfirst($strategy);
-    }
 }
diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php
@@ -24,7 +24,7 @@
 class TraceableAccessDecisionManager implements AccessDecisionManagerInterface
 {
     private $manager;
-    private $strategy;
+    private $defaultStrategy;
     private $voters = array();
     private $decisionLog = array();
 
@@ -33,10 +33,10 @@ public function __construct(AccessDecisionManagerInterface $manager)
         $this->manager = $manager;
 
         if ($this->manager instanceof AccessDecisionManager) {
-            // The strategy is stored in a private property of the decorated service
-            $reflection = new \ReflectionProperty(AccessDecisionManager::class, 'strategy');
+            // The default strategy is stored in a private property of the decorated service
+            $reflection = new \ReflectionProperty(AccessDecisionManager::class, 'defaultStrategy');
             $reflection->setAccessible(true);
-            $this->strategy = $reflection->getValue($manager);
+            $this->defaultStrategy = $reflection->getValue($manager);
         }
     }
 
@@ -47,10 +47,16 @@ public function decide(TokenInterface $token, array $attributes, $object = null)
     {
         $result = $this->manager->decide($token, $attributes, $object);
 
+        $strategy = 'unknown';
+        if ($this->manager instanceof AccessDecisionManager) {
+            $strategy = $this->manager->getStrategy($token, $attributes, $object);
+        }
+
         $this->decisionLog[] = array(
             'attributes' => $attributes,
             'object' => $object,
             'result' => $result,
+            'strategy' => $strategy,
         );
 
         return $result;
@@ -72,12 +78,9 @@ public function setVoters(array $voters)
     /**
      * @return string
      */
-    public function getStrategy()
+    public function getDefaultStrategy()
     {
-        // The $strategy property is misleading because it stores the name of its
-        // method (e.g. 'decideAffirmative') instead of the original strategy name
-        // (e.g. 'affirmative')
-        return null === $this->strategy ? '-' : strtolower(substr($this->strategy, 6));
+        return null === $this->defaultStrategy ? 'unknown' : $this->defaultStrategy;
     }
 
     /**