feature #23324 [Security] remove support for defining voters that don…

…'t implement VoterInterface. (hhamon) This PR was merged into the 4.0-dev branch. Discussion ---------- [Security] remove support for defining voters that don't implement VoterInterface. | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | yes | Deprecations? | no | Tests pass? | yes | Fixed tickets | ~ | License | MIT | Doc PR | ~ Commits ------- f527790 [Security] remove support for defining voters that don't implement the VoterInterface interface.
symfony · Jun 29, 2017 · 1c106da · 1c106da
2 parents 0932183 + f527790
commit 1c106da
Show file tree

Hide file tree

Showing 8 changed files with 15 additions and 129 deletions.
diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md
@@ -455,6 +455,8 @@ Security
  * The `AccessDecisionManager::setVoters()` method has been removed. Pass the
    voters to the constructor instead.
 
+ * Support for defining voters that don't implement the `VoterInterface` has been removed.
+
 SecurityBundle
 --------------
 

diff --git a/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md b/src/Symfony/Bundle/SecurityBundle/CHANGELOG.md
@@ -8,6 +8,7 @@ CHANGELOG
  * made `FirewallMap::$container` and `::$map` private
  * made the first `UserPasswordEncoderCommand::_construct()` argument mandatory
  * `UserPasswordEncoderCommand` does not extend `ContainerAwareCommand` anymore
+ * removed support for voters that don't implement the `VoterInterface`
 
 3.4.0
 -----

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSecurityVotersPass.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Compiler/AddSecurityVotersPass.php
@@ -38,19 +38,14 @@ public function process(ContainerBuilder $container)
 
         $voters = $this->findAndSortTaggedServices('security.voter', $container);
         if (!$voters) {
-            throw new LogicException('No security voters found. You need to tag at least one with "security.voter"');
+            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));
+                throw new LogicException(sprintf('%s must implement the %s when used as a voter.', $class, VoterInterface::class));
             }
         }
 

diff --git a/...ny/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSecurityVotersPassTest.php b/...ny/Bundle/SecurityBundle/Tests/DependencyInjection/Compiler/AddSecurityVotersPassTest.php
@@ -14,16 +14,15 @@
 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
 {
     /**
      * @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException
+     * @expectedExceptionMessage No security voters found. You need to tag at least one with "security.voter".
      */
     public function testNoVoters()
     {
@@ -71,8 +70,8 @@ public function testThatSecurityVotersAreProcessedInPriorityOrder()
     }
 
     /**
-     * @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.
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\LogicException
+     * @expectedExceptionMessage stdClass must implement the Symfony\Component\Security\Core\Authorization\Voter\VoterInterface when used as a voter.
      */
     public function testVoterMissingInterface()
     {
@@ -82,40 +81,7 @@ public function testVoterMissingInterface()
             ->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')
+            ->register('without_interface', 'stdClass')
             ->addTag('security.voter')
         ;
         $compilerPass = new AddSecurityVotersPass();

diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md
@@ -8,7 +8,8 @@ CHANGELOG
    You should implement this method yourself in your concrete authenticator.
  * removed the `AccessDecisionManager::setVoters()` method
  * removed the `RoleInterface`
- * added a sixth `string $context` argument to`LogoutUrlGenerator::registerListener()`
+ * removed support for voters that don't implement the `VoterInterface`
+ * added a sixth `string $context` argument to `LogoutUrlGenerator::registerListener()`
 
 3.4.0
 -----

diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php
@@ -13,7 +13,6 @@
 
 use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
-use Symfony\Component\Security\Core\Exception\LogicException;
 
 /**
  * AccessDecisionManager is the base class for all access decision managers
@@ -33,7 +32,7 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
     private $allowIfEqualGrantedDeniedDecisions;
 
     /**
-     * @param iterable|VoterInterface[] $voters                             An iterator of VoterInterface instances
+     * @param iterable|VoterInterface[] $voters                             An array or an iterator of VoterInterface instances
      * @param string                    $strategy                           The vote strategy
      * @param bool                      $allowIfAllAbstainDecisions         Whether to grant access if all voters abstained or not
      * @param bool                      $allowIfEqualGrantedDeniedDecisions Whether to grant access if result are equals
@@ -71,7 +70,7 @@ private function decideAffirmative(TokenInterface $token, array $attributes, $ob
     {
         $deny = 0;
         foreach ($this->voters as $voter) {
-            $result = $this->vote($voter, $token, $object, $attributes);
+            $result = $voter->vote($token, $object, $attributes);
             switch ($result) {
                 case VoterInterface::ACCESS_GRANTED:
                     return true;
@@ -112,7 +111,7 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje
         $grant = 0;
         $deny = 0;
         foreach ($this->voters as $voter) {
-            $result = $this->vote($voter, $token, $object, $attributes);
+            $result = $voter->vote($token, $object, $attributes);
 
             switch ($result) {
                 case VoterInterface::ACCESS_GRANTED:
@@ -153,7 +152,7 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje
         $grant = 0;
         foreach ($this->voters as $voter) {
             foreach ($attributes as $attribute) {
-                $result = $this->vote($voter, $token, $object, array($attribute));
+                $result = $voter->vote($token, $object, array($attribute));
 
                 switch ($result) {
                     case VoterInterface::ACCESS_GRANTED:
@@ -177,27 +176,4 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje
 
         return $this->allowIfAllAbstainDecisions;
     }
-
-    /**
-     * TokenInterface vote proxy method.
-     *
-     * Acts as a BC layer when the VoterInterface is not implemented on the voter.
-     *
-     * @deprecated as of 3.4 and will be removed in 4.0. Call the voter directly as the instance will always be a VoterInterface
-     */
-    private function vote($voter, TokenInterface $token, $subject, $attributes)
-    {
-        if ($voter instanceof VoterInterface) {
-            return $voter->vote($token, $subject, $attributes);
-        }
-
-        if (method_exists($voter, 'vote')) {
-            @trigger_error(sprintf('Calling vote() on an voter without %1$s is deprecated as of 3.4 and will be removed in 4.0. Implement the %1$s on your voter.', VoterInterface::class), E_USER_DEPRECATED);
-
-            // making the assumption that the signature matches
-            return $voter->vote($token, $subject, $attributes);
-        }
-
-        throw new LogicException(sprintf('%s should implement the %s interface when used as voter.', get_class($voter), VoterInterface::class));
-    }
 }
diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php
@@ -12,11 +12,8 @@
 namespace Symfony\Component\Security\Core\Tests\Authorization;
 
 use PHPUnit\Framework\TestCase;
-use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
 use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
-use Symfony\Component\Security\Core\Exception\LogicException;
-use Symfony\Component\Security\Core\Tests\Authorization\Stub\VoterWithoutInterface;
 
 class AccessDecisionManagerTest extends TestCase
 {
@@ -141,34 +138,4 @@ protected function getVoter($vote)
 
         return $voter;
     }
-
-    public function testVotingWrongTypeNoVoteMethod()
-    {
-        $exception = LogicException::class;
-        $message = sprintf('stdClass should implement the %s interface when used as voter.', VoterInterface::class);
-
-        if (method_exists($this, 'expectException')) {
-            $this->expectException($exception);
-            $this->expectExceptionMessage($message);
-        } else {
-            $this->setExpectedException($exception, $message);
-        }
-
-        $adm = new AccessDecisionManager(array(new \stdClass()));
-        $token = $this->getMockBuilder(TokenInterface::class)->getMock();
-
-        $adm->decide($token, array('TEST'));
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation Calling vote() on an voter without 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 on your voter.
-     */
-    public function testVotingWrongTypeWithVote()
-    {
-        $adm = new AccessDecisionManager(array(new VoterWithoutInterface()));
-        $token = $this->getMockBuilder(TokenInterface::class)->getMock();
-
-        $adm->decide($token, array('TEST'));
-    }
 }
diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Stub/VoterWithoutInterface.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Stub/VoterWithoutInterface.php