[SecurityBundle] added compiler pass for checking voters validity

symfony · pjarmalavicius · Aug 11, 2017 · Aug 11, 2017 · Aug 11, 2017 · Aug 21, 2017
commit d0cdefc6c4971fc0d04b51959382c380578e583f
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSecurityVotersPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSecurityVotersPass.php
@@ -40,20 +40,6 @@ public function process(ContainerBuilder $container)
        if (!$voters) {
            throw new LogicException('No security voters found. You need to tag at least one with "security.voter"');
        }
-
-        foreach ($voters as $voter) {
-            $class = $container->getDefinition((string) $voter)->getClass();
-
-            if (!is_a($class, VoterInterface::class, true)) {
-                @trigger_error(sprintf('Using a security.voter tag on a class without implementing the %1$s is deprecated as of 3.4 and will be removed in 4.0. Implement the %1$s instead.', VoterInterface::class), E_USER_DEPRECATED);
-            }
-
-            if (!method_exists($class, 'vote')) {
-                // in case the vote method is completely missing, to prevent exceptions when voting
-                throw new LogicException(sprintf('%s should implement the %s interface when used as voter.', $class, VoterInterface::class));
-            }
-        }
-
        $adm = $container->getDefinition('security.access.decision_manager');
        $adm->replaceArgument(0, new IteratorArgument($voters));
    }

diff --git a/...ny/Bundle/SecurityBundle/DependencyInjection/Compiler/CheckSecurityVotersValidityPass.php b/...ny/Bundle/SecurityBundle/DependencyInjection/Compiler/CheckSecurityVotersValidityPass.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\LogicException;
+use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
+
+/**
+ * Checks if all configured security voters implements VoterInterface.
+ *
+ * @author Paulius Jarmalavičius <paulius.jarmalavicius@gmail.com>
+ */
+class CheckSecurityVotersValidityPass implements CompilerPassInterface
+{
+    use PriorityTaggedServiceTrait;
+
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container)
+    {
+        $voters = $this->findAndSortTaggedServices('security.voter', $container);
+        foreach ($voters as $voter) {
+            $class = $container->getDefinition((string) $voter)->getClass();
+
+            if (!is_a($class, VoterInterface::class, true)) {
+                @trigger_error(
+                    sprintf(
+                        'Using a security.voter tag on a class without implementing the %1$s is deprecated as of 3.4 and will be removed in 4.0. Implement the %1$s instead.',
+                        VoterInterface::class
+                    ),
+                    E_USER_DEPRECATED
+                );
+            }
+
+            if (!method_exists($class, 'vote')) {
+                // in case the vote method is completely missing, to prevent exceptions when voting
+                throw new LogicException(
+                    sprintf('%s should implement the %s interface when used as voter.', $class, VoterInterface::class)
+                );
+            }
+        }
+    }
+}
diff --git a/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php b/src/Symfony/Bundle/SecurityBundle/SecurityBundle.php
@@ -11,8 +11,10 @@

 namespace Symfony\Bundle\SecurityBundle;

+use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\CheckSecurityVotersValidityPass;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Security\Factory\JsonLoginFactory;
 use Symfony\Component\Console\Application;
+use Symfony\Component\DependencyInjection\Compiler\PassConfig;
 use Symfony\Component\HttpKernel\Bundle\Bundle;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
@@ -58,6 +60,7 @@ public function build(ContainerBuilder $container)
        $extension->addUserProviderFactory(new InMemoryFactory());
        $extension->addUserProviderFactory(new LdapFactory());
        $container->addCompilerPass(new AddSecurityVotersPass());
+        $container->addCompilerPass(new CheckSecurityVotersValidityPass(), PassConfig::TYPE_OPTIMIZE);
    }

    public function registerCommands(Application $application)

diff --git a/...ny/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSecurityVotersPassTest.php b/...ny/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSecurityVotersPassTest.php
@@ -14,11 +14,9 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\Exception\LogicException;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
 use Symfony\Component\Security\Core\Authorization\Voter\Voter;
-use Symfony\Component\Security\Core\Tests\Authorization\Stub\VoterWithoutInterface;

 class AddSecurityVotersPassTest extends TestCase
 {
@@ -69,56 +67,4 @@ public function testThatSecurityVotersAreProcessedInPriorityOrder()
        $this->assertEquals(new Reference('lowest_prio_service'), $refs[1]);
        $this->assertCount(4, $refs);
    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Using a security.voter tag on a class without implementing the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface is deprecated as of 3.4 and will be removed in 4.0. Implement the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface instead.
-     */
-    public function testVoterMissingInterface()
-    {
-        $container = new ContainerBuilder();
-        $container
-            ->register('security.access.decision_manager', AccessDecisionManager::class)
-            ->addArgument(array())
-        ;
-        $container
-            ->register('without_interface', VoterWithoutInterface::class)
-            ->addTag('security.voter')
-        ;
-        $compilerPass = new AddSecurityVotersPass();
-        $compilerPass->process($container);
-
-        $argument = $container->getDefinition('security.access.decision_manager')->getArgument(0);
-        $refs = $argument->getValues();
-        $this->assertEquals(new Reference('without_interface'), $refs[0]);
-        $this->assertCount(1, $refs);
-    }
-
-    /**
-     * @group legacy
-     */
-    public function testVoterMissingInterfaceAndMethod()
-    {
-        $exception = LogicException::class;
-        $message = 'stdClass should implement the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface interface when used as voter.';
-
-        if (method_exists($this, 'expectException')) {
-            $this->expectException($exception);
-            $this->expectExceptionMessage($message);
-        } else {
-            $this->setExpectedException($exception, $message);
-        }
-
-        $container = new ContainerBuilder();
-        $container
-            ->register('security.access.decision_manager', AccessDecisionManager::class)
-            ->addArgument(array())
-        ;
-        $container
-            ->register('without_method', 'stdClass')
-            ->addTag('security.voter')
-        ;
-        $compilerPass = new AddSecurityVotersPass();
-        $compilerPass->process($container);
-    }
 }
diff --git a/...SecurityBundle/Tests/DependencyInjection/Compiler/CheckSecurityVotersValidityPassTest.php b/...SecurityBundle/Tests/DependencyInjection/Compiler/CheckSecurityVotersValidityPassTest.php
@@ -0,0 +1,78 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\SecurityBundle\Tests\DependencyInjection\Compiler;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\AddSecurityVotersPass;
+use Symfony\Bundle\SecurityBundle\DependencyInjection\Compiler\CheckSecurityVotersValidityPass;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Exception\LogicException;
+use Symfony\Component\DependencyInjection\Reference;
+use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
+use Symfony\Component\Security\Core\Tests\Authorization\Stub\VoterWithoutInterface;
+
+class CheckSecurityVotersValidityPassTest extends TestCase
+{
+    /**
+     * @group legacy
+     * @expectedDeprecation Using a security.voter tag on a class without implementing the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface is deprecated as of 3.4 and will be removed in 4.0. Implement the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface instead.
+     */
+    public function testVoterMissingInterface()
+    {
+        $container = new ContainerBuilder();
+        $container
+            ->register('security.access.decision_manager', AccessDecisionManager::class)
+            ->addArgument(array())
+        ;
+        $container
+            ->register('without_interface', VoterWithoutInterface::class)
+            ->addTag('security.voter')
+        ;
+        $addCompilerPass = new AddSecurityVotersPass();
+        $addCompilerPass->process($container);
+        $checkCompilerPass = new CheckSecurityVotersValidityPass();
+        $checkCompilerPass->process($container);
+
+        $argument = $container->getDefinition('security.access.decision_manager')->getArgument(0);
+        $refs = $argument->getValues();
+        $this->assertEquals(new Reference('without_interface'), $refs[0]);
+        $this->assertCount(1, $refs);
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testVoterMissingInterfaceAndMethod()
+    {
+        $exception = LogicException::class;
+        $message = 'stdClass should implement the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface interface when used as voter.';
+
+        if (method_exists($this, 'expectException')) {
+            $this->expectException($exception);
+            $this->expectExceptionMessage($message);
+        } else {
+            $this->setExpectedException($exception, $message);
+        }
+
+        $container = new ContainerBuilder();
+        $container
+            ->register('security.access.decision_manager', AccessDecisionManager::class)
+            ->addArgument(array())
+        ;
+        $container
+            ->register('without_method', 'stdClass')
+            ->addTag('security.voter')
+        ;
+        $compilerPass = new CheckSecurityVotersValidityPass();
+        $compilerPass->process($container);
+    }
+}