Assert voter returns valid decision

UPGRADE-5.3.md

@@ -12,3 +12,8 @@ Form
  * Deprecated passing an array as the first argument of the `CheckboxListMapper::mapFormsToData()` method, pass `\Traversable` instead.
  * Deprecated passing an array as the second argument of the `RadioListMapper::mapDataToForms()` method, pass `\Traversable` instead.
  * Deprecated passing an array as the first argument of the `RadioListMapper::mapFormsToData()` method, pass `\Traversable` instead.
+
+Security
+--------
+
+ * Deprecated voters that do not return a valid decision when calling the `vote` method.

UPGRADE-6.0.md

@@ -159,6 +159,7 @@ Security
    in `PreAuthenticatedToken`, `RememberMeToken`, `SwitchUserToken`, `UsernamePasswordToken`,
    `DefaultAuthenticationSuccessHandler`.
  * Removed the `AbstractRememberMeServices::$providerKey` property in favor of `AbstractRememberMeServices::$firewallName`
+ * `AccessDecisionManager` now throw an exception when a voter does not return a valid decision.
 
 TwigBundle
 ----------

src/Symfony/Component/Security/CHANGELOG.md

@@ -1,6 +1,11 @@
 CHANGELOG
 =========
 
+5.3.0
+-----
+
+* Deprecated voters that do not return a valid decision when calling the `vote` method.
+
 5.2.0
 -----
 

src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php

@@ -89,6 +89,8 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
 
             if (VoterInterface::ACCESS_DENIED === $result) {
                 ++$deny;
+            } elseif (VoterInterface::ACCESS_ABSTAIN !== $result) {
+                trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class);
             }
         }
 
@@ -124,6 +126,8 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
                 ++$grant;
             } elseif (VoterInterface::ACCESS_DENIED === $result) {
                 ++$deny;
+            } elseif (VoterInterface::ACCESS_ABSTAIN !== $result) {
+                trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class);
             }
         }
 
@@ -161,6 +165,8 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
 
                 if (VoterInterface::ACCESS_GRANTED === $result) {
                     ++$grant;
+                } elseif (VoterInterface::ACCESS_ABSTAIN !== $result) {
+                    trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class);
                 }
             }
         }
@@ -192,6 +198,10 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
             if (VoterInterface::ACCESS_DENIED === $result) {
                 return false;
             }
+
+            if (VoterInterface::ACCESS_ABSTAIN !== $result) {
+                trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class);
+            }
         }
 
         return $this->allowIfAllAbstainDecisions;

src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php

@@ -12,11 +12,14 @@
 namespace Symfony\Component\Security\Core\Tests\Authorization;
 
 use PHPUnit\Framework\TestCase;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
 use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
 
 class AccessDecisionManagerTest extends TestCase
 {
+    use ExpectDeprecationTrait;
+
     public function testSetUnsupportedStrategy()
     {
         $this->expectException('InvalidArgumentException');
@@ -34,6 +37,20 @@ class AccessDecisionManagerTest extends TestCase
         $this->assertSame($expected, $manager->decide($token, ['ROLE_FOO']));
     }
 
+    /**
+     * @dataProvider provideStrategies
+     * @group legacy
+     */
+    public function testDeprecatedVoter($strategy)
+    {
+        $token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
+        $manager = new AccessDecisionManager([$this->getVoter(3)], $strategy);
+
+        $this->expectDeprecation('Since symfony/security-core 5.3: Returning "3" in "%s::vote()" is deprecated, return one of "Symfony\Component\Security\Core\Authorization\Voter\VoterInterface" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".');
+
+        $manager->decide($token, ['ROLE_FOO']);
+    }
+
     public function getStrategyTests()
     {
         return [
@@ -94,6 +111,14 @@ class AccessDecisionManagerTest extends TestCase
         ];
     }
 
+    public function provideStrategies()
+    {
+        yield [AccessDecisionManager::STRATEGY_AFFIRMATIVE];
+        yield [AccessDecisionManager::STRATEGY_CONSENSUS];
+        yield [AccessDecisionManager::STRATEGY_UNANIMOUS];
+        yield [AccessDecisionManager::STRATEGY_PRIORITY];
+    }
+
     protected function getVoters($grants, $denies, $abstains)
     {
         $voters = [];