diff --git a/UPGRADE-5.3.md b/UPGRADE-5.3.md index 69532a9397..1685d36ac2 100644 --- a/UPGRADE-5.3.md +++ b/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. diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 3eda1d0f68..450a3a0a10 100644 --- a/UPGRADE-6.0.md +++ b/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 ---------- diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index 566625a7c1..2f7c94deed 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/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 ----- diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 8356c38bb9..7c4cbd7288 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/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; diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index 0e3c62c5bd..2f7ce5e9f6 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/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 = [];