From ef682050d980d2c754252b7ec6f9ed2d9c5eec8b Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Thu, 26 May 2022 18:29:40 +0200 Subject: [PATCH 1/4] [Security] Add the ability for voter to return decision reason --- .../Controller/AbstractController.php | 15 +- .../EventListener/VoteListener.php | 2 +- .../views/Collector/security.html.twig | 9 +- .../Tests/EventListener/VoteListenerTest.php | 26 ++- .../Core/Authorization/AccessDecision.php | 118 ++++++++++ .../Authorization/AccessDecisionManager.php | 53 ++++- .../AccessDecisionManagerInterface.php | 4 + .../Authorization/AuthorizationChecker.php | 13 +- .../AuthorizationCheckerInterface.php | 2 + .../AccessDecisionStrategyInterface.php | 6 + .../Strategy/AffirmativeStrategy.php | 32 +++ .../Strategy/ConsensusStrategy.php | 48 +++++ .../Strategy/PriorityStrategy.php | 27 +++ .../Strategy/UnanimousStrategy.php | 33 +++ .../TraceableAccessDecisionManager.php | 36 +++- .../Voter/AuthenticatedVoter.php | 25 ++- .../Authorization/Voter/ExpressionVoter.php | 15 +- .../Core/Authorization/Voter/RoleVoter.php | 15 +- .../Authorization/Voter/TraceableVoter.php | 20 +- .../Core/Authorization/Voter/Vote.php | 91 ++++++++ .../Core/Authorization/Voter/Voter.php | 53 ++++- .../Authorization/Voter/VoterInterface.php | 6 +- .../Security/Core/Event/VoteEvent.php | 21 +- .../Core/Exception/AccessDeniedException.php | 32 +++ .../Component/Security/Core/Security.php | 17 +- .../Test/AccessDecisionStrategyTestCase.php | 20 +- .../AccessDecisionManagerTest.php | 201 +++++++++++++----- .../AuthorizationCheckerTest.php | 89 +++++++- .../Strategy/AffirmativeStrategyTest.php | 24 ++- .../Strategy/ConsensusStrategyTest.php | 58 ++++- .../Strategy/PriorityStrategyTest.php | 42 ++-- .../Strategy/UnanimousStrategyTest.php | 30 ++- .../TraceableAccessDecisionManagerTest.php | 138 ++++++------ .../Voter/AuthenticatedVoterTest.php | 42 +++- .../Voter/ExpressionVoterTest.php | 35 ++- .../Voter/RoleHierarchyVoterTest.php | 25 ++- .../Authorization/Voter/RoleVoterTest.php | 34 ++- .../Voter/TraceableVoterTest.php | 38 ++++ .../Tests/Authorization/Voter/VoterTest.php | 102 ++++++++- .../Exception/AccessDeniedExceptionTest.php | 64 ++++++ .../Security/Http/Firewall/AccessListener.php | 14 +- .../Http/Firewall/SwitchUserListener.php | 10 +- .../Tests/Firewall/AccessListenerTest.php | 86 ++++++-- .../Tests/Firewall/SwitchUserListenerTest.php | 62 ++++-- 44 files changed, 1577 insertions(+), 256 deletions(-) create mode 100644 src/Symfony/Component/Security/Core/Authorization/AccessDecision.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php create mode 100644 src/Symfony/Component/Security/Core/Tests/Exception/AccessDeniedExceptionTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index 9bcc34606135c..ff811f86a2c85 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php @@ -35,6 +35,7 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\RouterInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\User\UserInterface; @@ -213,10 +214,22 @@ protected function isGranted(mixed $attribute, mixed $subject = null): bool */ protected function denyAccessUnlessGranted(mixed $attribute, mixed $subject = null, string $message = 'Access Denied.'): void { - if (!$this->isGranted($attribute, $subject)) { + if (!$this->container->has('security.authorization_checker')) { + throw new \LogicException('The SecurityBundle is not registered in your application. Try running "composer require symfony/security-bundle".'); + } + + $checker = $this->container->get('security.authorization_checker'); + if (method_exists($checker, 'getDecision')) { + $decision = $checker->getDecision($attribute, $subject); + } else { + $decision = $checker->isGranted($attribute, $subject) ? AccessDecision::createGranted() : AccessDecision::createDenied(); + } + + if (!$decision->isGranted()) { $exception = $this->createAccessDeniedException($message); $exception->setAttributes([$attribute]); $exception->setSubject($subject); + $exception->setAccessDecision($decision); throw $exception; } diff --git a/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php b/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php index 34ca91c3a7735..34ef8ae2088e1 100644 --- a/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php +++ b/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php @@ -33,7 +33,7 @@ public function __construct(TraceableAccessDecisionManager $traceableAccessDecis public function onVoterVote(VoteEvent $event): void { - $this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $event->getVote()); + $this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $event->getVoteDecision()); } public static function getSubscribedEvents(): array diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig index 48e6c95998c7a..1cb84ae4064c7 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig @@ -445,16 +445,17 @@ attribute {{ voter_detail['attributes'][0] }} {% endif %} - {% if voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_GRANTED') %} + {% if voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_GRANTED') %} ACCESS GRANTED - {% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_ABSTAIN') %} + {% elseif voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_ABSTAIN') %} ACCESS ABSTAIN - {% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %} + {% elseif voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %} ACCESS DENIED {% else %} - unknown ({{ voter_detail['vote'] }}) + unknown ({{ voter_detail['vote'].access }}) {% endif %} + {{ voter_detail['vote'].message|default('~') }} {% endfor %} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php index d262effae29ac..5ea42ee02fd3e 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Bundle\SecurityBundle\EventListener\VoteListener; use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Event\VoteEvent; @@ -29,10 +30,33 @@ public function testOnVoterVote() ->onlyMethods(['addVoterVote']) ->getMock(); + $vote = Vote::createGranted(); $traceableAccessDecisionManager ->expects($this->once()) ->method('addVoterVote') - ->with($voter, ['myattr1', 'myattr2'], VoterInterface::ACCESS_GRANTED); + ->with($voter, ['myattr1', 'myattr2'], $vote); + + $sut = new VoteListener($traceableAccessDecisionManager); + $sut->onVoterVote(new VoteEvent($voter, 'mysubject', ['myattr1', 'myattr2'], $vote)); + } + + /** + * @group legacy + */ + public function testOnVoterVoteLegacy() + { + $voter = $this->createMock(VoterInterface::class); + + $traceableAccessDecisionManager = $this + ->getMockBuilder(TraceableAccessDecisionManager::class) + ->disableOriginalConstructor() + ->setMethods(['addVoterVote']) + ->getMock(); + + $traceableAccessDecisionManager + ->expects($this->once()) + ->method('addVoterVote') + ->with($voter, ['myattr1', 'myattr2'], Vote::createGranted()); $sut = new VoteListener($traceableAccessDecisionManager); $sut->onVoterVote(new VoteEvent($voter, 'mysubject', ['myattr1', 'myattr2'], VoterInterface::ACCESS_GRANTED)); diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php new file mode 100644 index 0000000000000..0f1ea42c62976 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -0,0 +1,118 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authorization; + +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\Voter; +use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; + +/** + * An AccessDecision is returned by an AccessDecisionManager and contains the access verdict and all the related votes. + * + * @author Dany Maillard + */ +final class AccessDecision +{ + /** @var int One of the VoterInterface::ACCESS_* constants */ + private int $access; + + /** @var Vote[] */ + private array $votes = []; + + /** + * @param int $access One of the VoterInterface::ACCESS_* constants + * @param Vote[] $votes + */ + private function __construct(int $access, array $votes = []) + { + $this->access = $access; + $this->votes = $votes; + } + + public function getAccess(): int + { + return $this->access; + } + + public function isGranted(): bool + { + return VoterInterface::ACCESS_GRANTED === $this->access; + } + + public function isAbstain(): bool + { + return VoterInterface::ACCESS_ABSTAIN === $this->access; + } + + public function isDenied(): bool + { + return VoterInterface::ACCESS_DENIED === $this->access; + } + + /** + * @param Vote[] $votes + */ + public static function createGranted(array $votes = []): self + { + return new self(VoterInterface::ACCESS_GRANTED, $votes); + } + + /** + * @param Vote[] $votes + */ + public static function createDenied(array $votes = []): self + { + return new self(VoterInterface::ACCESS_DENIED, $votes); + } + + /** + * @return Vote[] + */ + public function getVotes(): array + { + return $this->votes; + } + + /** + * @return Vote[] + */ + public function getGrantedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_GRANTED); + } + + /** + * @return Vote[] + */ + public function getAbstainedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_ABSTAIN); + } + + /** + * @return Vote[] + */ + public function getDeniedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_DENIED); + } + + /** + * @return Vote[] + */ + private function getVotesByAccess(int $access): array + { + return array_filter($this->votes, static function (Vote $vote) use ($access): bool { + return $vote->getAccess() === $access; + }); + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 025f6827bdfd1..cdc6e48a5d8b9 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -15,6 +15,7 @@ use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; @@ -46,11 +47,35 @@ public function __construct(iterable $voters = [], AccessDecisionStrategyInterfa $this->strategy = $strategy ?? new AffirmativeStrategy(); } + public function getDecision(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): AccessDecision + { + // Special case for AccessListener, do not remove the right side of the condition before 6.0 + if (\count($attributes) > 1 && !$allowMultipleAttributes) { + throw new InvalidArgumentException(sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__)); + } + + if (method_exists($this->strategy, 'getDecision')) { + $decision = $this->strategy->getDecision( + $this->collectVotes($token, $attributes, $object) + ); + } else { + $decision = $this->strategy->decide( + $this->collectResults($token, $attributes, $object) + ) ? AccessDecision::createGranted() : AccessDecision::createDenied(); + } + + return $decision; + } + /** * @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array + * + * @deprecated since Symfony 6.2, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + // Special case for AccessListener, do not remove the right side of the condition before 6.0 if (\count($attributes) > 1 && !$allowMultipleAttributes) { throw new InvalidArgumentException(sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__)); @@ -62,17 +87,33 @@ public function decide(TokenInterface $token, array $attributes, mixed $object = } /** - * @return \Traversable + * @return \Traversable */ - private function collectResults(TokenInterface $token, array $attributes, mixed $object): \Traversable + private function collectVotes(TokenInterface $token, array $attributes, mixed $object): \Traversable { foreach ($this->getVoters($attributes, $object) as $voter) { - $result = $voter->vote($token, $object, $attributes); - if (!\is_int($result) || !(self::VALID_VOTES[$result] ?? false)) { - throw new \LogicException(sprintf('"%s::vote()" must return one of "%s" constants ("ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN"), "%s" returned.', get_debug_type($voter), VoterInterface::class, var_export($result, true))); + if (method_exists($voter, 'getVote')) { + yield $voter->getVote($token, $object, $attributes); + } else { + $result = $voter->vote($token, $object, $attributes); + yield match ($result) { + VoterInterface::ACCESS_GRANTED => Vote::createGranted(), + VoterInterface::ACCESS_DENIED => Vote::createDenied(), + VoterInterface::ACCESS_ABSTAIN => Vote::createAbstain(), + default => throw new \LogicException(sprintf('"%s::vote()" must return one of "%s" constants ("ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN"), "%s" returned.', get_debug_type($voter), VoterInterface::class, var_export($result, true))), + }; } + } + } - yield $result; + /** + * @return \Traversable + */ + private function collectResults(TokenInterface $token, array $attributes, mixed $object): \Traversable + { + /** @var Vote $vote */ + foreach ($this->collectVotes($token, $attributes, $object) as $vote) { + yield $vote->getAccess(); } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index f25c7e1bef9b3..6c9629babae7a 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -17,6 +17,8 @@ * AccessDecisionManagerInterface makes authorization decisions. * * @author Fabien Potencier + * + * @method AccessDecision getDecision(TokenInterface $token, array $attributes, mixed $object = null) */ interface AccessDecisionManagerInterface { @@ -25,6 +27,8 @@ interface AccessDecisionManagerInterface * * @param array $attributes An array of attributes associated with the method being invoked * @param mixed $object The object to secure + * + * @deprecated since Symfony 6.2, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, mixed $object = null): bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index 3827f8b91ee38..32c0e96e4bbcc 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -38,6 +38,11 @@ public function __construct(TokenStorageInterface $tokenStorage, AccessDecisionM } final public function isGranted(mixed $attribute, mixed $subject = null): bool + { + return $this->getDecision($attribute, $subject)->isGranted(); + } + + final public function getDecision($attribute, $subject = null): AccessDecision { $token = $this->tokenStorage->getToken(); @@ -45,6 +50,12 @@ final public function isGranted(mixed $attribute, mixed $subject = null): bool $token = new NullToken(); } - return $this->accessDecisionManager->decide($token, [$attribute], $subject); + if (!method_exists($this->accessDecisionManager, 'getDecision')) { + trigger_deprecation('symfony/security-core', '6.2', 'Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.', \get_class($this->accessDecisionManager)); + + return $this->accessDecisionManager->decide($token, [$attribute], $subject) ? AccessDecision::createGranted() : AccessDecision::createDenied(); + } + + return $this->accessDecisionManager->getDecision($token, [$attribute], $subject); } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php index 6f5a6022178ba..0089b2b1dbf6e 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php @@ -15,6 +15,8 @@ * The AuthorizationCheckerInterface. * * @author Johannes M. Schmitt + * + * @method AccessDecision getDecision(mixed $attribute, mixed $subject = null) */ interface AuthorizationCheckerInterface { diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php index 00238378a30fa..a1bbe8fdab82c 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php @@ -11,15 +11,21 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; + /** * A strategy for turning a stream of votes into a final decision. * * @author Alexander M. Turek + * + * @method AccessDecision getDecision(\Traversable $votes) */ interface AccessDecisionStrategyInterface { /** * @param \Traversable $results + * + * @deprecated since Symfony 6.2, use {@see getDecision()} instead. */ public function decide(\Traversable $results): bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php index ecd74b208616c..0686a74f62dab 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -21,6 +23,7 @@ * * @author Fabien Potencier * @author Alexander M. Turek + * @author Antoine Lamirault */ final class AffirmativeStrategy implements AccessDecisionStrategyInterface, \Stringable { @@ -31,8 +34,37 @@ public function __construct(bool $allowIfAllAbstainDecisions = false) $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; } + public function getDecision(\Traversable $votes): AccessDecision + { + $currentVotes = []; + $deny = 0; + + /** @var Vote $vote */ + foreach ($votes as $vote) { + $currentVotes[] = $vote; + if ($vote->isGranted()) { + return AccessDecision::createGranted($currentVotes); + } + + if ($vote->isDenied()) { + ++$deny; + } + } + + if ($deny > 0) { + return AccessDecision::createDenied($currentVotes); + } + + return $this->allowIfAllAbstainDecisions + ? AccessDecision::createGranted($currentVotes) + : AccessDecision::createDenied($currentVotes) + ; + } + public function decide(\Traversable $results): bool { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + $deny = 0; foreach ($results as $result) { if (VoterInterface::ACCESS_GRANTED === $result) { diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php index 489b34287b520..8e20ed6df8d4f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -29,6 +31,7 @@ * * @author Fabien Potencier * @author Alexander M. Turek + * @author Antoine Lamirault */ final class ConsensusStrategy implements AccessDecisionStrategyInterface, \Stringable { @@ -41,8 +44,53 @@ public function __construct(bool $allowIfAllAbstainDecisions = false, bool $allo $this->allowIfEqualGrantedDeniedDecisions = $allowIfEqualGrantedDeniedDecisions; } + /** + * {@inheritdoc} + */ + public function getDecision(\Traversable $votes): AccessDecision + { + $currentVotes = []; + + /** @var Vote $vote */ + $grant = 0; + $deny = 0; + foreach ($votes as $vote) { + $currentVotes[] = $vote; + if ($vote->isGranted()) { + ++$grant; + } elseif ($vote->isDenied()) { + ++$deny; + } + } + + if ($grant > $deny) { + return AccessDecision::createGranted($currentVotes); + } + + if ($deny > $grant) { + return AccessDecision::createDenied($currentVotes); + } + + if ($grant > 0) { + return $this->allowIfEqualGrantedDeniedDecisions + ? AccessDecision::createGranted($currentVotes) + : AccessDecision::createDenied($currentVotes) + ; + } + + return $this->allowIfAllAbstainDecisions + ? AccessDecision::createGranted($currentVotes) + : AccessDecision::createDenied($currentVotes) + ; + } + + /** + * {@inheritdoc} + */ public function decide(\Traversable $results): bool { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + $grant = 0; $deny = 0; foreach ($results as $result) { diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php index 9599950c7fdbb..b8674b5f3de45 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -22,6 +24,7 @@ * * @author Fabien Potencier * @author Alexander M. Turek + * @author Antoine Lamirault */ final class PriorityStrategy implements AccessDecisionStrategyInterface, \Stringable { @@ -32,8 +35,32 @@ public function __construct(bool $allowIfAllAbstainDecisions = false) $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; } + public function getDecision(\Traversable $votes): AccessDecision + { + $currentVotes = []; + + /** @var Vote $vote */ + foreach ($votes as $vote) { + $currentVotes[] = $vote; + if ($vote->isGranted()) { + return AccessDecision::createGranted($currentVotes); + } + + if ($vote->isDenied()) { + return AccessDecision::createDenied($currentVotes); + } + } + + return $this->allowIfAllAbstainDecisions + ? AccessDecision::createGranted($currentVotes) + : AccessDecision::createDenied($currentVotes) + ; + } + public function decide(\Traversable $results): bool { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + foreach ($results as $result) { if (VoterInterface::ACCESS_GRANTED === $result) { return true; diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php index 1f3b85c548fbf..3cf62aac99608 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -21,6 +23,7 @@ * * @author Fabien Potencier * @author Alexander M. Turek + * @author Antoine Lamirault */ final class UnanimousStrategy implements AccessDecisionStrategyInterface, \Stringable { @@ -31,8 +34,38 @@ public function __construct(bool $allowIfAllAbstainDecisions = false) $this->allowIfAllAbstainDecisions = $allowIfAllAbstainDecisions; } + public function getDecision(\Traversable $votes): AccessDecision + { + $currentVotes = []; + $grant = 0; + + /** @var Vote $vote */ + foreach ($votes as $vote) { + $currentVotes[] = $vote; + if ($vote->isDenied()) { + return AccessDecision::createDenied($currentVotes); + } + + if ($vote->isGranted()) { + ++$grant; + } + } + + // no deny votes + if ($grant > 0) { + return AccessDecision::createGranted($currentVotes); + } + + return $this->allowIfAllAbstainDecisions + ? AccessDecision::createGranted($currentVotes) + : AccessDecision::createDenied($currentVotes) + ; + } + public function decide(\Traversable $results): bool { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + $grant = 0; foreach ($results as $result) { if (VoterInterface::ACCESS_DENIED === $result) { diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index cb44dce4c0f0d..00150474df6c2 100644 --- a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php @@ -13,6 +13,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -47,8 +48,32 @@ public function __construct(AccessDecisionManagerInterface $manager) } } + public function getDecision(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): AccessDecision + { + $currentDecisionLog = [ + 'attributes' => $attributes, + 'object' => $object, + 'voterDetails' => [], + ]; + + $this->currentLog[] = &$currentDecisionLog; + + $result = $this->manager->getDecision($token, $attributes, $object, $allowMultipleAttributes); + + $currentDecisionLog['result'] = $result; + + $this->decisionLog[] = array_pop($this->currentLog); // Using a stack since getDecision can be called by voters + + return $result; + } + + /** + * {@inheritdoc} + */ public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + $currentDecisionLog = [ 'attributes' => $attributes, 'object' => $object, @@ -69,11 +94,16 @@ public function decide(TokenInterface $token, array $attributes, mixed $object = /** * Adds voter vote and class to the voter details. * - * @param array $attributes attributes used for the vote - * @param int $vote vote of the voter + * @param array $attributes attributes used for the vote + * @param Vote|int $vote vote of the voter */ - public function addVoterVote(VoterInterface $voter, array $attributes, int $vote): void + public function addVoterVote(VoterInterface $voter, array $attributes, Vote|int $vote): void { + if (!$vote instanceof Vote) { + trigger_deprecation('symfony/security-core', '6.2', 'Passing an int as the third argument to "%s::addVoterVote()" is deprecated, pass an instance of "%s" instead.', __CLASS__, Vote::class); + $vote = new Vote($vote); + } + $currentLogIndex = \count($this->currentLog) - 1; $this->currentLog[$currentLogIndex]['voterDetails'][] = [ 'voter' => $voter, diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php index d7b2b22431b04..5225086a7397e 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php @@ -40,13 +40,13 @@ public function __construct(AuthenticationTrustResolverInterface $authentication $this->authenticationTrustResolver = $authenticationTrustResolver; } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function getVote(TokenInterface $token, mixed $subject, array $attributes): Vote { if ($attributes === [self::PUBLIC_ACCESS]) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } - $result = VoterInterface::ACCESS_ABSTAIN; + $result = Vote::createAbstain(); foreach ($attributes as $attribute) { if (null === $attribute || (self::IS_AUTHENTICATED_FULLY !== $attribute && self::IS_AUTHENTICATED_REMEMBERED !== $attribute @@ -56,35 +56,42 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): continue; } - $result = VoterInterface::ACCESS_DENIED; + $result = Vote::createDenied(); if (self::IS_AUTHENTICATED_FULLY === $attribute && $this->authenticationTrustResolver->isFullFledged($token)) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } if (self::IS_AUTHENTICATED_REMEMBERED === $attribute && ($this->authenticationTrustResolver->isRememberMe($token) || $this->authenticationTrustResolver->isFullFledged($token))) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } if (self::IS_AUTHENTICATED === $attribute && $this->authenticationTrustResolver->isAuthenticated($token)) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } if (self::IS_REMEMBERED === $attribute && $this->authenticationTrustResolver->isRememberMe($token)) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } if (self::IS_IMPERSONATOR === $attribute && $token instanceof SwitchUserToken) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } } return $result; } + public function vote(TokenInterface $token, mixed $subject, array $attributes): int + { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + + return $this->getVote($token, $subject, $attributes)->getAccess(); + } + public function supportsAttribute(string $attribute): bool { return \in_array($attribute, [ diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php index 9369ef45d2839..eb980aebcaec3 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php @@ -49,9 +49,9 @@ public function supportsType(string $subjectType): bool return true; } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function getVote(TokenInterface $token, mixed $subject, array $attributes): Vote { - $result = VoterInterface::ACCESS_ABSTAIN; + $result = Vote::createAbstain(); $variables = null; foreach ($attributes as $attribute) { if (!$attribute instanceof Expression) { @@ -60,15 +60,22 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): $variables ??= $this->getVariables($token, $subject); - $result = VoterInterface::ACCESS_DENIED; + $result = Vote::createDenied(); if ($this->expressionLanguage->evaluate($attribute, $variables)) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } } return $result; } + public function vote(TokenInterface $token, mixed $subject, array $attributes): int + { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + + return $this->getVote($token, $subject, $attributes)->getAccess(); + } + private function getVariables(TokenInterface $token, mixed $subject): array { $roleNames = $token->getRoleNames(); diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php index 70dddcfff92e2..e92dc8b0ac37d 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php @@ -27,9 +27,9 @@ public function __construct(string $prefix = 'ROLE_') $this->prefix = $prefix; } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function getVote(TokenInterface $token, mixed $subject, array $attributes): Vote { - $result = VoterInterface::ACCESS_ABSTAIN; + $result = Vote::createAbstain(); $roles = $this->extractRoles($token); foreach ($attributes as $attribute) { @@ -37,10 +37,10 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): continue; } - $result = VoterInterface::ACCESS_DENIED; + $result = Vote::createDenied(); foreach ($roles as $role) { if ($attribute === $role) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } } } @@ -48,6 +48,13 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): return $result; } + public function vote(TokenInterface $token, mixed $subject, array $attributes): int + { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + + return $this->getVote($token, $subject, $attributes)->getAccess(); + } + public function supportsAttribute(string $attribute): bool { return str_starts_with($attribute, $this->prefix); diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index 412bb9760bfec..cb1ff1a256a93 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -33,13 +33,25 @@ public function __construct(VoterInterface $voter, EventDispatcherInterface $eve $this->eventDispatcher = $eventDispatcher; } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function getVote(TokenInterface $token, mixed $subject, array $attributes): Vote { - $result = $this->voter->vote($token, $subject, $attributes); + if (method_exists($this->voter, 'getVote')) { + $vote = $this->voter->getVote($token, $subject, $attributes); + } else { + $result = $this->voter->vote($token, $subject, $attributes); + $vote = new Vote($result); + } + + $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $vote), 'debug.security.authorization.vote'); + + return $vote; + } - $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $result), 'debug.security.authorization.vote'); + public function vote(TokenInterface $token, mixed $subject, array $attributes): int + { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); - return $result; + return $this->getVote($token, $subject, $attributes)->getAccess(); } public function getDecoratedVoter(): VoterInterface diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php new file mode 100644 index 0000000000000..a0df7b2e5e7b1 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -0,0 +1,91 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authorization\Voter; + +/** + * A Vote is returned by a Voter and contains the access (granted, abstain or denied). + * It can also contain a message explaining the vote decision. + * + * @author Dany Maillard + */ +final class Vote +{ + /** @var int One of the VoterInterface::ACCESS_* constants */ + private $access; + private $message; + private $context; + + /** + * @param int $access One of the VoterInterface::ACCESS_* constants + */ + public function __construct(int $access, string $message = '', array $context = []) + { + $this->access = $access; + $this->message = $message; + $this->context = $context; + } + + public function getAccess(): int + { + return $this->access; + } + + public function isGranted(): bool + { + return VoterInterface::ACCESS_GRANTED === $this->access; + } + + public function isAbstain(): bool + { + return VoterInterface::ACCESS_ABSTAIN === $this->access; + } + + public function isDenied(): bool + { + return VoterInterface::ACCESS_DENIED === $this->access; + } + + public static function create(int $access, string $message = '', array $context = []): self + { + return new self($access, $message, $context); + } + + public static function createGranted(string $message = '', array $context = []): self + { + return new self(VoterInterface::ACCESS_GRANTED, $message, $context); + } + + public static function createAbstain(string $message = '', array $context = []): self + { + return new self(VoterInterface::ACCESS_ABSTAIN, $message, $context); + } + + public static function createDenied(string $message = '', array $context = []): self + { + return new self(VoterInterface::ACCESS_DENIED, $message, $context); + } + + public function setMessage(string $message) + { + $this->message = $message; + } + + public function getMessage(): string + { + return $this->message; + } + + public function getContext(): array + { + return $this->context; + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index 1f76a42eaf1b8..d3226e6889383 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -24,10 +24,10 @@ */ abstract class Voter implements VoterInterface, CacheableVoterInterface { - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function getVote(TokenInterface $token, mixed $subject, array $attributes): Vote { // abstain vote by default in case none of the attributes are supported - $vote = self::ACCESS_ABSTAIN; + $vote = $this->abstain(); foreach ($attributes as $attribute) { try { @@ -43,17 +43,56 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): } // as soon as at least one attribute is supported, default is to deny access - $vote = self::ACCESS_DENIED; + $vote = $this->deny(); + + $decision = $this->voteOnAttribute($attribute, $subject, $token); + if (\is_bool($decision)) { + trigger_deprecation('symfony/security-core', '6.2', 'Returning a boolean in "%s::voteOnAttribute()" is deprecated, return an instance of "%s" instead.', static::class, Vote::class); + $decision = $decision ? $this->grant() : $this->deny(); + } - if ($this->voteOnAttribute($attribute, $subject, $token)) { + if ($decision->isGranted()) { // grant access as soon as at least one attribute returns a positive response - return self::ACCESS_GRANTED; + return $decision; } + + $vote->setMessage($vote->getMessage().trim(' '.$decision->getMessage())); } return $vote; } + public function vote(TokenInterface $token, mixed $subject, array $attributes): int + { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + + return $this->getVote($token, $subject, $attributes)->getAccess(); + } + + /** + * Creates a granted vote. + */ + protected function grant(string $message = '', array $context = []): Vote + { + return Vote::createGranted($message, $context); + } + + /** + * Creates an abstained vote. + */ + protected function abstain(string $message = '', array $context = []): Vote + { + return Vote::createAbstain($message, $context); + } + + /** + * Creates a denied vote. + */ + protected function deny(string $message = '', array $context = []): Vote + { + return Vote::createDenied($message, $context); + } + /** * Return false if your voter doesn't support the given attribute. Symfony will cache * that decision and won't call your voter again for that attribute. @@ -90,6 +129,8 @@ abstract protected function supports(string $attribute, mixed $subject): bool; * * @param TAttribute $attribute * @param TSubject $subject + * + * @return Vote|bool Returning a boolean is deprecated since Symfony 6.2. Return a Vote object instead. */ - abstract protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): bool; + abstract protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): Vote|bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php index 8eea57e76960f..61b9700fe8561 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php @@ -16,7 +16,9 @@ /** * VoterInterface is the interface implemented by all voters. * - * @author Fabien Potencier + * @author Fabien Potencier * + * + * @method Vote getVote(TokenInterface $token, mixed $subject, array $attributes) */ interface VoterInterface { @@ -36,6 +38,8 @@ interface VoterInterface * @return int either ACCESS_GRANTED, ACCESS_ABSTAIN, or ACCESS_DENIED * * @psalm-return self::ACCESS_* must be transformed into @return on Symfony 7 + * + * @deprecated since Symfony 6.2, use {@see getVote()} instead. */ public function vote(TokenInterface $token, mixed $subject, array $attributes); } diff --git a/src/Symfony/Component/Security/Core/Event/VoteEvent.php b/src/Symfony/Component/Security/Core/Event/VoteEvent.php index 1b1d6a336d6a1..cd194d76dfe99 100644 --- a/src/Symfony/Component/Security/Core/Event/VoteEvent.php +++ b/src/Symfony/Component/Security/Core/Event/VoteEvent.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Core\Event; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Contracts\EventDispatcher\Event; @@ -26,13 +27,19 @@ final class VoteEvent extends Event private VoterInterface $voter; private mixed $subject; private array $attributes; - private int $vote; + private Vote|int $vote; - public function __construct(VoterInterface $voter, mixed $subject, array $attributes, int $vote) + public function __construct(VoterInterface $voter, mixed $subject, array $attributes, Vote|int $vote) { $this->voter = $voter; $this->subject = $subject; $this->attributes = $attributes; + + if (!$vote instanceof Vote) { + trigger_deprecation('symfony/security-core', '6.2', 'Passing an int as the fourth argument to "%s::__construct" is deprecated, pass a "%s" instance instead.', __CLASS__, Vote::class); + + $vote = new Vote($vote); + } $this->vote = $vote; } @@ -51,7 +58,17 @@ public function getAttributes(): array return $this->attributes; } + /** + * @deprecated since Symfony 6.2, use {@see getVoteDecision()} instead. + */ public function getVote(): int + { + trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::getVote()" has been deprecated, use "%s::getVoteDecision()" instead.', __CLASS__, __CLASS__); + + return $this->vote->getAccess(); + } + + public function getVoteDecision(): Vote { return $this->vote; } diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index c95bae03b4fae..6c8a9ffee7c09 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -12,6 +12,8 @@ namespace Symfony\Component\Security\Core\Exception; use Symfony\Component\HttpKernel\Attribute\WithHttpStatus; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; /** * AccessDeniedException is thrown when the account has not the required role. @@ -23,6 +25,7 @@ class AccessDeniedException extends RuntimeException { private array $attributes = []; private mixed $subject = null; + private ?AccessDecision $accessDecision; public function __construct(string $message = 'Access Denied.', \Throwable $previous = null, int $code = 403) { @@ -54,4 +57,33 @@ public function setSubject(mixed $subject) { $this->subject = $subject; } + + /** + * Sets an access decision and appends the denied reasons to the exception message. + * + * @return void + */ + public function setAccessDecision(AccessDecision $accessDecision) + { + $this->accessDecision = $accessDecision; + if (!$accessDecision->getDeniedVotes()) { + return; + } + + $messages = array_map(static function (Vote $vote) { + return $vote->getMessage(); + }, $accessDecision->getDeniedVotes()); + + if ($messages) { + $this->message .= ' '.implode(' ', $messages); + } + } + + /** + * Gets the access decision. + */ + public function getAccessDecision(): ?AccessDecision + { + return $this->accessDecision; + } } diff --git a/src/Symfony/Component/Security/Core/Security.php b/src/Symfony/Component/Security/Core/Security.php index bb2576a7ab9dc..510993a62c7a4 100644 --- a/src/Symfony/Component/Security/Core/Security.php +++ b/src/Symfony/Component/Security/Core/Security.php @@ -14,6 +14,7 @@ use Psr\Container\ContainerInterface; use Symfony\Bundle\SecurityBundle\Security as NewSecurityHelper; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -58,12 +59,24 @@ public function getUser(): ?UserInterface */ public function isGranted(mixed $attributes, mixed $subject = null): bool { - return $this->container->get('security.authorization_checker') - ->isGranted($attributes, $subject); + return $this->getDecision($attributes, $subject)->isGranted(); } public function getToken(): ?TokenInterface { return $this->container->get('security.token_storage')->getToken(); } + + /** + * Get the access decision against the current authentication token and optionally supplied subject. + */ + public function getDecision(mixed $attribute, mixed $subject = null): AccessDecision + { + $checker = $this->container->get('security.authorization_checker'); + if (method_exists($checker, 'getDecision')) { + return $checker->getDecision($attribute, $subject); + } + + return $checker->isGranted($attribute, $subject) ? AccessDecision::createGranted() : AccessDecision::createDenied(); + } } diff --git a/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php b/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php index bf2a2b9a15ec3..460526b03255c 100644 --- a/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php +++ b/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php @@ -13,8 +13,10 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -29,12 +31,26 @@ abstract class AccessDecisionStrategyTestCase extends TestCase * * @param VoterInterface[] $voters */ - final public function testDecide(AccessDecisionStrategyInterface $strategy, array $voters, bool $expected) + final public function testGetDecision(AccessDecisionStrategyInterface $strategy, array $voters, AccessDecision $expected) { $token = $this->createMock(TokenInterface::class); $manager = new AccessDecisionManager($voters, $strategy); - $this->assertSame($expected, $manager->decide($token, ['ROLE_FOO'])); + $this->assertEquals($expected, $manager->getDecision($token, ['ROLE_FOO'])); + } + + /** + * @group legacy + * @dataProvider provideStrategyTests + * + * @param VoterInterface[] $voters + */ + final public function testDecideLegacy(AccessDecisionStrategyInterface $strategy, array $voters, AccessDecision $expected) + { + $token = $this->createMock(TokenInterface::class); + $manager = new AccessDecisionManager($voters, $strategy); + + $this->assertSame($expected->isGranted(), $manager->decide($token, ['ROLE_FOO'])); } /** diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index 37848ab7a9ee4..6bab3d98c5592 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -14,9 +14,11 @@ use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class AccessDecisionManagerTest extends TestCase @@ -34,8 +36,54 @@ public function testVoterCalls() $token = $this->createMock(TokenInterface::class); $voters = [ - $this->getExpectedVoter(VoterInterface::ACCESS_DENIED), - $this->getExpectedVoter(VoterInterface::ACCESS_GRANTED), + $this->getExpectedVoter(Vote::createDenied()), + $this->getExpectedVoter(Vote::createGranted()), + $this->getUnexpectedVoter(), + ]; + + $strategy = new class() implements AccessDecisionStrategyInterface { + public function getDecision(\Traversable $votes): AccessDecision + { + $i = 0; + /** @var Vote $vote */ + foreach ($votes as $vote) { + switch ($i++) { + case 0: + Assert::assertSame(VoterInterface::ACCESS_DENIED, $vote->getAccess()); + + break; + case 1: + Assert::assertSame(VoterInterface::ACCESS_GRANTED, $vote->getAccess()); + + return AccessDecision::createGranted(); + } + } + + return AccessDecision::createDenied(); + } + + public function decide(\Traversable $results): bool + { + throw new \RuntimeException('Method should not be called'); + } + }; + + $manager = new AccessDecisionManager($voters, $strategy); + + $expectedDecision = AccessDecision::createGranted(); + $this->assertEquals($expectedDecision, $manager->getDecision($token, ['ROLE_FOO'])); + } + + /** + * @group legacy + */ + public function testVoterCallsLegacy() + { + $token = $this->createMock(TokenInterface::class); + + $voters = [ + $this->getExpectedVoterLegacy(VoterInterface::ACCESS_DENIED), + $this->getExpectedVoterLegacy(VoterInterface::ACCESS_GRANTED), $this->getUnexpectedVoter(), ]; @@ -68,7 +116,10 @@ public function decide(\Traversable $results): bool public function testCacheableVoters() { $token = $this->createMock(TokenInterface::class); - $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter = $this + ->getMockBuilder(CacheableVoterInterface::class) + ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->getMock(); $voter ->expects($this->once()) ->method('supportsAttribute') @@ -79,11 +130,45 @@ public function testCacheableVoters() ->method('supportsType') ->with('string') ->willReturn(true); + + $vote = Vote::createGranted(); $voter ->expects($this->once()) - ->method('vote') + ->method('getVote') + ->with($token, 'bar', ['foo']) + ->willReturn($vote); + + $manager = new AccessDecisionManager([$voter]); + + $expectedDecision = AccessDecision::createGranted([$vote]); + $this->assertEquals($expectedDecision, $manager->getDecision($token, ['foo'], 'bar')); + } + + /** + * @group legacy + */ + public function testCacheableVotersLegacy() + { + $token = $this->createMock(TokenInterface::class); + $voter = $this + ->getMockBuilder(CacheableVoterInterface::class) + ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->getMock(); + $voter + ->expects($this->once()) + ->method('supportsAttribute') + ->with('foo') + ->willReturn(true); + $voter + ->expects($this->once()) + ->method('supportsType') + ->with('string') + ->willReturn(true); + $voter + ->expects($this->once()) + ->method('getVote') ->with($token, 'bar', ['foo']) - ->willReturn(VoterInterface::ACCESS_GRANTED); + ->willReturn(Vote::createGranted()); $manager = new AccessDecisionManager([$voter]); $this->assertTrue($manager->decide($token, ['foo'], 'bar')); @@ -92,7 +177,10 @@ public function testCacheableVoters() public function testCacheableVotersIgnoresNonStringAttributes() { $token = $this->createMock(TokenInterface::class); - $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter = $this + ->getMockBuilder(CacheableVoterInterface::class) + ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->getMock(); $voter ->expects($this->never()) ->method('supportsAttribute'); @@ -103,18 +191,21 @@ public function testCacheableVotersIgnoresNonStringAttributes() ->willReturn(true); $voter ->expects($this->once()) - ->method('vote') + ->method('getVote') ->with($token, 'bar', [1337]) - ->willReturn(VoterInterface::ACCESS_GRANTED); + ->willReturn(Vote::createGranted()); $manager = new AccessDecisionManager([$voter]); - $this->assertTrue($manager->decide($token, [1337], 'bar')); + $this->assertTrue($manager->getDecision($token, [1337], 'bar')->isGranted()); } public function testCacheableVotersWithMultipleAttributes() { $token = $this->createMock(TokenInterface::class); - $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter = $this + ->getMockBuilder(CacheableVoterInterface::class) + ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->getMock(); $voter ->expects($this->exactly(2)) ->method('supportsAttribute') @@ -136,18 +227,21 @@ public function testCacheableVotersWithMultipleAttributes() ->willReturn(true); $voter ->expects($this->once()) - ->method('vote') + ->method('getVote') ->with($token, 'bar', ['foo', 'bar']) - ->willReturn(VoterInterface::ACCESS_GRANTED); + ->willReturn(Vote::createGranted()); $manager = new AccessDecisionManager([$voter]); - $this->assertTrue($manager->decide($token, ['foo', 'bar'], 'bar', true)); + $this->assertTrue($manager->getDecision($token, ['foo', 'bar'], 'bar', true)->isGranted()); } public function testCacheableVotersWithEmptyAttributes() { $token = $this->createMock(TokenInterface::class); - $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter = $this + ->getMockBuilder(CacheableVoterInterface::class) + ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->getMock(); $voter ->expects($this->never()) ->method('supportsAttribute'); @@ -158,18 +252,21 @@ public function testCacheableVotersWithEmptyAttributes() ->willReturn(true); $voter ->expects($this->once()) - ->method('vote') + ->method('getVote') ->with($token, 'bar', []) - ->willReturn(VoterInterface::ACCESS_GRANTED); + ->willReturn(Vote::createGranted()); $manager = new AccessDecisionManager([$voter]); - $this->assertTrue($manager->decide($token, [], 'bar')); + $this->assertTrue($manager->getDecision($token, [], 'bar')->isGranted()); } public function testCacheableVotersSupportsMethodsCalledOnce() { $token = $this->createMock(TokenInterface::class); - $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter = $this + ->getMockBuilder(CacheableVoterInterface::class) + ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->getMock(); $voter ->expects($this->once()) ->method('supportsAttribute') @@ -182,19 +279,22 @@ public function testCacheableVotersSupportsMethodsCalledOnce() ->willReturn(true); $voter ->expects($this->exactly(2)) - ->method('vote') + ->method('getVote') ->with($token, 'bar', ['foo']) - ->willReturn(VoterInterface::ACCESS_GRANTED); + ->willReturn(Vote::createGranted()); $manager = new AccessDecisionManager([$voter]); - $this->assertTrue($manager->decide($token, ['foo'], 'bar')); - $this->assertTrue($manager->decide($token, ['foo'], 'bar')); + $this->assertTrue($manager->getDecision($token, ['foo'], 'bar')->isGranted()); + $this->assertTrue($manager->getDecision($token, ['foo'], 'bar')->isGranted()); } public function testCacheableVotersNotCalled() { $token = $this->createMock(TokenInterface::class); - $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter = $this + ->getMockBuilder(CacheableVoterInterface::class) + ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->getMock(); $voter ->expects($this->once()) ->method('supportsAttribute') @@ -205,16 +305,19 @@ public function testCacheableVotersNotCalled() ->method('supportsType'); $voter ->expects($this->never()) - ->method('vote'); + ->method('getVote'); $manager = new AccessDecisionManager([$voter]); - $this->assertFalse($manager->decide($token, ['foo'], 'bar')); + $this->assertFalse($manager->getDecision($token, ['foo'], 'bar')->isGranted()); } public function testCacheableVotersWithMultipleAttributesAndNonString() { $token = $this->createMock(TokenInterface::class); - $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter = $this + ->getMockBuilder(CacheableVoterInterface::class) + ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->getMock(); $voter ->expects($this->once()) ->method('supportsAttribute') @@ -228,48 +331,28 @@ public function testCacheableVotersWithMultipleAttributesAndNonString() ->willReturn(true); $voter ->expects($this->once()) - ->method('vote') + ->method('getVote') ->with($token, 'bar', ['foo', 1337]) - ->willReturn(VoterInterface::ACCESS_GRANTED); + ->willReturn(Vote::createGranted()); $manager = new AccessDecisionManager([$voter]); - $this->assertTrue($manager->decide($token, ['foo', 1337], 'bar', true)); - } - - protected static function getVoters($grants, $denies, $abstains): array - { - $voters = []; - for ($i = 0; $i < $grants; ++$i) { - $voters[] = self::getVoter(VoterInterface::ACCESS_GRANTED); - } - for ($i = 0; $i < $denies; ++$i) { - $voters[] = self::getVoter(VoterInterface::ACCESS_DENIED); - } - for ($i = 0; $i < $abstains; ++$i) { - $voters[] = self::getVoter(VoterInterface::ACCESS_ABSTAIN); - } - - return $voters; + $this->assertTrue($manager->getDecision($token, ['foo', 1337], 'bar', true)->isGranted()); } - protected static function getVoter($vote) + private function getExpectedVoter(Vote $vote): VoterInterface { - return new class($vote) implements VoterInterface { - private int $vote; - - public function __construct(int $vote) - { - $this->vote = $vote; - } + $voter = $this + ->getMockBuilder(VoterInterface::class) + ->setMethods(['getVote', 'vote']) + ->getMock(); + $voter->expects($this->once()) + ->method('getVote') + ->willReturn($vote); - public function vote(TokenInterface $token, $subject, array $attributes) - { - return $this->vote; - } - }; + return $voter; } - private function getExpectedVoter(int $vote): VoterInterface + private function getExpectedVoterLegacy(int $vote): VoterInterface { $voter = $this->createMock(VoterInterface::class); $voter->expects($this->once()) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 36b048c8976d1..72ca4c14bdd48 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -11,17 +11,20 @@ namespace Symfony\Component\Security\Core\Tests\Authorization; -use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationChecker; use Symfony\Component\Security\Core\User\InMemoryUser; class AuthorizationCheckerTest extends TestCase { + use ExpectDeprecationTrait; + private MockObject&AccessDecisionManagerInterface $accessDecisionManager; private AuthorizationChecker $authorizationChecker; private TokenStorage $tokenStorage; @@ -35,26 +38,80 @@ protected function setUp(): void } public function testVoteWithoutAuthenticationToken() + { + $accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); + $accessDecisionManager + ->expects($this->once()) + ->method('getDecision') + ->with($this->isInstanceOf(NullToken::class), ['ROLE_FOO']) + ->willReturn(AccessDecision::createGranted()); + + $authorizationChecker = new AuthorizationChecker( + $this->tokenStorage, + $accessDecisionManager + ); + + $authorizationChecker->isGranted('ROLE_FOO'); + } + + /** + * @group legacy + */ + public function testVoteWithoutAuthenticationTokenLegacy() { $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->accessDecisionManager); $this->accessDecisionManager->expects($this->once())->method('decide')->with($this->isInstanceOf(NullToken::class))->willReturn(false); + $this->expectDeprecation('Since symfony/security-core 6.2: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); $authorizationChecker->isGranted('ROLE_FOO'); } /** + * @group legacy * @dataProvider isGrantedProvider */ public function testIsGranted($decide) { $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + $accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); + $accessDecisionManager + ->expects($this->once()) + ->method('getDecision') + ->with($this->identicalTo($token), $this->identicalTo(['ROLE_FOO'])) + ->willReturn($decide ? AccessDecision::createGranted() : AccessDecision::createDenied()); + + $this->tokenStorage->setToken($token); + $authorizationChecker = new AuthorizationChecker( + $this->tokenStorage, + $accessDecisionManager + ); + + $this->assertSame($decide, $authorizationChecker->isGranted('ROLE_FOO')); + } + + /** + * @group legacy + * @dataProvider isGrantedProvider + */ + public function testIsGrantedLegacy($decide) + { + $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + $this->accessDecisionManager ->expects($this->once()) ->method('decide') ->willReturn($decide); $this->tokenStorage->setToken($token); + + $this->expectDeprecation('Since symfony/security-core 6.2: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); $this->assertSame($decide, $this->authorizationChecker->isGranted('ROLE_FOO')); } @@ -69,12 +126,42 @@ public function testIsGrantedWithObjectAttribute() $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + $accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); + $accessDecisionManager + ->expects($this->once()) + ->method('getDecision') + ->with($this->identicalTo($token), $this->identicalTo([$attribute])) + ->willReturn(AccessDecision::createGranted()); + + $this->tokenStorage->setToken($token); + $authorizationChecker = new AuthorizationChecker( + $this->tokenStorage, + $accessDecisionManager + ); + + $this->assertTrue($authorizationChecker->isGranted($attribute)); + } + + /** + * @group legacy + */ + public function testIsGrantedWithObjectAttributeLegacy() + { + $attribute = new \stdClass(); + + $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + $this->accessDecisionManager ->expects($this->once()) ->method('decide') ->with($this->identicalTo($token), $this->identicalTo([$attribute])) ->willReturn(true); $this->tokenStorage->setToken($token); + + $this->expectDeprecation('Since symfony/security-core 6.2: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); $this->assertTrue($this->authorizationChecker->isGranted($attribute)); } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php index b467a920b0f67..3f89f1e6dd5bd 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php @@ -9,9 +9,11 @@ * file that was distributed with this source code. */ -namespace Authorization\Strategy; +namespace Symfony\Component\Security\Core\Tests\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Test\AccessDecisionStrategyTestCase; class AffirmativeStrategyTest extends AccessDecisionStrategyTestCase @@ -20,13 +22,23 @@ public static function provideStrategyTests(): iterable { $strategy = new AffirmativeStrategy(); - yield [$strategy, self::getVoters(1, 0, 0), true]; - yield [$strategy, self::getVoters(1, 2, 0), true]; - yield [$strategy, self::getVoters(0, 1, 0), false]; - yield [$strategy, self::getVoters(0, 0, 1), false]; + yield [$strategy, self::getVoters(1, 0, 0), AccessDecision::createGranted([ + Vote::createGranted(), + ])]; + yield [$strategy, self::getVoters(1, 2, 0), AccessDecision::createGranted([ + Vote::createGranted(), + ])]; + yield [$strategy, self::getVoters(0, 1, 0), AccessDecision::createDenied([ + Vote::createDenied(), + ])]; + yield [$strategy, self::getVoters(0, 0, 1), AccessDecision::createDenied([ + Vote::createAbstain(), + ])]; $strategy = new AffirmativeStrategy(true); - yield [$strategy, self::getVoters(0, 0, 1), true]; + yield [$strategy, self::getVoters(0, 0, 1), AccessDecision::createGranted([ + Vote::createAbstain(), + ])]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php index bde6fb0d624b7..47eac3e115aa0 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php @@ -9,9 +9,11 @@ * file that was distributed with this source code. */ -namespace Authorization\Strategy; +namespace Symfony\Component\Security\Core\Tests\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\Strategy\ConsensusStrategy; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Test\AccessDecisionStrategyTestCase; class ConsensusStrategyTest extends AccessDecisionStrategyTestCase @@ -20,21 +22,57 @@ public static function provideStrategyTests(): iterable { $strategy = new ConsensusStrategy(); - yield [$strategy, self::getVoters(1, 0, 0), true]; - yield [$strategy, self::getVoters(1, 2, 0), false]; - yield [$strategy, self::getVoters(2, 1, 0), true]; - yield [$strategy, self::getVoters(0, 0, 1), false]; + yield [$strategy, self::getVoters(1, 0, 0), AccessDecision::createGranted([ + Vote::createGranted(), + ])]; + yield [$strategy, self::getVoters(1, 2, 0), AccessDecision::createDenied([ + Vote::createGranted(), + Vote::createDenied(), + Vote::createDenied(), + ])]; + yield [$strategy, self::getVoters(2, 1, 0), AccessDecision::createGranted([ + Vote::createGranted(), + Vote::createGranted(), + Vote::createDenied(), + ])]; + yield [$strategy, self::getVoters(0, 0, 1), AccessDecision::createDenied([ + Vote::createAbstain(), + ])]; - yield [$strategy, self::getVoters(2, 2, 0), true]; - yield [$strategy, self::getVoters(2, 2, 1), true]; + yield [$strategy, self::getVoters(2, 2, 0), AccessDecision::createGranted([ + Vote::createGranted(), + Vote::createGranted(), + Vote::createDenied(), + Vote::createDenied(), + ])]; + yield [$strategy, self::getVoters(2, 2, 1), AccessDecision::createGranted([ + Vote::createGranted(), + Vote::createGranted(), + Vote::createDenied(), + Vote::createDenied(), + Vote::createAbstain(), + ])]; $strategy = new ConsensusStrategy(true); - yield [$strategy, self::getVoters(0, 0, 1), true]; + yield [$strategy, self::getVoters(0, 0, 1), AccessDecision::createGranted([ + Vote::createAbstain(), + ])]; $strategy = new ConsensusStrategy(false, false); - yield [$strategy, self::getVoters(2, 2, 0), false]; - yield [$strategy, self::getVoters(2, 2, 1), false]; + yield [$strategy, self::getVoters(2, 2, 0), AccessDecision::createDenied([ + Vote::createGranted(), + Vote::createGranted(), + Vote::createDenied(), + Vote::createDenied(), + ])]; + yield [$strategy, self::getVoters(2, 2, 1), AccessDecision::createDenied([ + Vote::createGranted(), + Vote::createGranted(), + Vote::createDenied(), + Vote::createDenied(), + Vote::createAbstain(), + ])]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php index aef3aaf0b27e3..e3339b8c0e254 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php @@ -9,9 +9,11 @@ * file that was distributed with this source code. */ -namespace Authorization\Strategy; +namespace Symfony\Component\Security\Core\Tests\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\Strategy\PriorityStrategy; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Test\AccessDecisionStrategyTestCase; @@ -22,23 +24,35 @@ public static function provideStrategyTests(): iterable $strategy = new PriorityStrategy(); yield [$strategy, [ - self::getVoter(VoterInterface::ACCESS_ABSTAIN), - self::getVoter(VoterInterface::ACCESS_GRANTED), - self::getVoter(VoterInterface::ACCESS_DENIED), - self::getVoter(VoterInterface::ACCESS_DENIED), - ], true]; + self::getVoter(VoterInterface::ACCESS_ABSTAIN), + self::getVoter(VoterInterface::ACCESS_GRANTED), + self::getVoter(VoterInterface::ACCESS_DENIED), + self::getVoter(VoterInterface::ACCESS_DENIED), + ], AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createGranted(), + ])]; yield [$strategy, [ - self::getVoter(VoterInterface::ACCESS_ABSTAIN), - self::getVoter(VoterInterface::ACCESS_DENIED), - self::getVoter(VoterInterface::ACCESS_GRANTED), - self::getVoter(VoterInterface::ACCESS_GRANTED), - ], false]; - - yield [$strategy, self::getVoters(0, 0, 2), false]; + self::getVoter(VoterInterface::ACCESS_ABSTAIN), + self::getVoter(VoterInterface::ACCESS_DENIED), + self::getVoter(VoterInterface::ACCESS_GRANTED), + self::getVoter(VoterInterface::ACCESS_GRANTED), + ], AccessDecision::createDenied([ + Vote::createAbstain(), + Vote::createDenied(), + ])]; + + yield [$strategy, self::getVoters(0, 0, 2), AccessDecision::createDenied([ + Vote::createAbstain(), + Vote::createAbstain(), + ])]; $strategy = new PriorityStrategy(true); - yield [$strategy, self::getVoters(0, 0, 2), true]; + yield [$strategy, self::getVoters(0, 0, 2), AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createAbstain(), + ])]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php index e00a50e3186ba..f51d142ab98cf 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php @@ -9,9 +9,11 @@ * file that was distributed with this source code. */ -namespace Authorization\Strategy; +namespace Symfony\Component\Security\Core\Tests\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\Strategy\UnanimousStrategy; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Test\AccessDecisionStrategyTestCase; class UnanimousStrategyTest extends AccessDecisionStrategyTestCase @@ -20,14 +22,28 @@ public static function provideStrategyTests(): iterable { $strategy = new UnanimousStrategy(); - yield [$strategy, self::getVoters(1, 0, 0), true]; - yield [$strategy, self::getVoters(1, 0, 1), true]; - yield [$strategy, self::getVoters(1, 1, 0), false]; - - yield [$strategy, self::getVoters(0, 0, 2), false]; + yield [$strategy, self::getVoters(1, 0, 0), AccessDecision::createGranted([ + Vote::createGranted(), + ])]; + yield [$strategy, self::getVoters(1, 0, 1), AccessDecision::createGranted([ + Vote::createGranted(), + Vote::createAbstain(), + ])]; + yield [$strategy, self::getVoters(1, 1, 0), AccessDecision::createDenied([ + Vote::createGranted(), + Vote::createDenied(), + ])]; + + yield [$strategy, self::getVoters(0, 0, 2), AccessDecision::createDenied([ + Vote::createAbstain(), + Vote::createAbstain(), + ])]; $strategy = new UnanimousStrategy(true); - yield [$strategy, self::getVoters(0, 0, 2), true]; + yield [$strategy, self::getVoters(0, 0, 2), AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createAbstain(), + ])]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php index cefe8dbc1273a..4ad3637de4146 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -13,9 +13,11 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Tests\Fixtures\DummyVoter; @@ -24,28 +26,31 @@ class TraceableAccessDecisionManagerTest extends TestCase /** * @dataProvider provideObjectsAndLogs */ - public function testDecideLog(array $expectedLog, array $attributes, $object, array $voterVotes, bool $result) + public function testDecideLog(array $expectedLog, array $attributes, $object, array $voterVotes, AccessDecision $decision) { $token = $this->createMock(TokenInterface::class); - $admMock = $this->createMock(AccessDecisionManagerInterface::class); + $admMock = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); $adm = new TraceableAccessDecisionManager($admMock); $admMock ->expects($this->once()) - ->method('decide') + ->method('getDecision') ->with($token, $attributes, $object) - ->willReturnCallback(function ($token, $attributes, $object) use ($voterVotes, $adm, $result) { + ->willReturnCallback(function ($token, $attributes, $object) use ($voterVotes, $adm, $decision) { foreach ($voterVotes as $voterVote) { [$voter, $vote] = $voterVote; $adm->addVoterVote($voter, $attributes, $vote); } - return $result; + return $decision; }) ; - $adm->decide($token, $attributes, $object); + $adm->getDecision($token, $attributes, $object); $this->assertEquals($expectedLog, $adm->getDecisionLog()); } @@ -59,115 +64,115 @@ public static function provideObjectsAndLogs(): \Generator [[ 'attributes' => ['ATTRIBUTE_1'], 'object' => null, - 'result' => true, + 'result' => ($result = AccessDecision::createGranted()), 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1'], 'vote' => VoterInterface::ACCESS_GRANTED], - ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1'], 'vote' => $granted_vote1 = Vote::createGranted()], + ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1'], 'vote' => $granted_vote2 = Vote::createGranted()], ], ]], ['ATTRIBUTE_1'], null, [ - [$voter1, VoterInterface::ACCESS_GRANTED], - [$voter2, VoterInterface::ACCESS_GRANTED], + [$voter1, $granted_vote1], + [$voter2, $granted_vote2], ], - true, + $result, ]; yield [ [[ 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'object' => true, - 'result' => false, + 'result' => ($result = AccessDecision::createDenied()), 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'vote' => $abstain_vote1 = Vote::createAbstain()], + ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1', 'ATTRIBUTE_2'], 'vote' => $granted_vote2 = Vote::createGranted()], ], ]], ['ATTRIBUTE_1', 'ATTRIBUTE_2'], true, [ - [$voter1, VoterInterface::ACCESS_ABSTAIN], - [$voter2, VoterInterface::ACCESS_GRANTED], + [$voter1, $abstain_vote1], + [$voter2, $granted_vote2], ], - false, + $result, ]; yield [ [[ 'attributes' => [null], 'object' => 'jolie string', - 'result' => false, + 'result' => ($result = AccessDecision::createDenied()), 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => [null], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => [null], 'vote' => VoterInterface::ACCESS_DENIED], + ['voter' => $voter1, 'attributes' => [null], 'vote' => $abstain_vote1 = Vote::createAbstain()], + ['voter' => $voter2, 'attributes' => [null], 'vote' => $abstain_denied2 = Vote::createAbstain()], ], ]], [null], 'jolie string', [ - [$voter1, VoterInterface::ACCESS_ABSTAIN], - [$voter2, VoterInterface::ACCESS_DENIED], + [$voter1, $abstain_vote1], + [$voter2, $abstain_denied2], ], - false, + $result, ]; yield [ [[ 'attributes' => [12], 'object' => 12345, - 'result' => true, + 'result' => ($result = AccessDecision::createGranted()), 'voterDetails' => [], ]], 'attributes' => [12], 12345, [], - true, + $result, ]; yield [ [[ 'attributes' => [new \stdClass()], 'object' => $x = fopen(__FILE__, 'r'), - 'result' => true, + 'result' => ($result = AccessDecision::createGranted()), 'voterDetails' => [], ]], [new \stdClass()], $x, [], - true, + $result, ]; yield [ [[ 'attributes' => ['ATTRIBUTE_2'], 'object' => $x = [], - 'result' => false, + 'result' => ($result = AccessDecision::createDenied()), 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], + ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_2'], 'vote' => $abstain_vote1 = Vote::createAbstain()], + ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_2'], 'vote' => $abstain_vote2 = Vote::createAbstain()], ], ]], ['ATTRIBUTE_2'], $x, [ - [$voter1, VoterInterface::ACCESS_ABSTAIN], - [$voter2, VoterInterface::ACCESS_ABSTAIN], + [$voter1, $abstain_vote1], + [$voter2, $abstain_vote2], ], - false, + $result, ]; yield [ [[ 'attributes' => [12.13], 'object' => new \stdClass(), - 'result' => false, + 'result' => ($result = AccessDecision::createDenied()), 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => [12.13], 'vote' => VoterInterface::ACCESS_DENIED], - ['voter' => $voter2, 'attributes' => [12.13], 'vote' => VoterInterface::ACCESS_DENIED], + ['voter' => $voter1, 'attributes' => [12.13], 'vote' => $denied_vote1 = Vote::createDenied()], + ['voter' => $voter2, 'attributes' => [12.13], 'vote' => $denied_vote2 = Vote::createDenied()], ], ]], [12.13], new \stdClass(), [ - [$voter1, VoterInterface::ACCESS_DENIED], - [$voter2, VoterInterface::ACCESS_DENIED], + [$voter1, $denied_vote1], + [$voter2, $denied_vote2], ], - false, + $result, ]; } @@ -178,26 +183,26 @@ public function testAccessDecisionManagerCalledByVoter() { $voter1 = $this ->getMockBuilder(VoterInterface::class) - ->onlyMethods(['vote']) + ->onlyMethods(['getVote', 'vote']) ->getMock(); $voter2 = $this ->getMockBuilder(VoterInterface::class) - ->onlyMethods(['vote']) + ->onlyMethods(['getVote', 'vote']) ->getMock(); $voter3 = $this ->getMockBuilder(VoterInterface::class) - ->onlyMethods(['vote']) + ->onlyMethods(['getVote', 'vote']) ->getMock(); $sut = new TraceableAccessDecisionManager(new AccessDecisionManager([$voter1, $voter2, $voter3])); $voter1 ->expects($this->any()) - ->method('vote') + ->method('getVote') ->willReturnCallback(function (TokenInterface $token, $subject, array $attributes) use ($sut, $voter1) { - $vote = \in_array('attr1', $attributes) ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_ABSTAIN; + $vote = \in_array('attr1', $attributes) ? Vote::createGranted() : Vote::createAbstain(); $sut->addVoterVote($voter1, $attributes, $vote); return $vote; @@ -205,12 +210,12 @@ public function testAccessDecisionManagerCalledByVoter() $voter2 ->expects($this->any()) - ->method('vote') + ->method('getVote') ->willReturnCallback(function (TokenInterface $token, $subject, array $attributes) use ($sut, $voter2) { if (\in_array('attr2', $attributes)) { - $vote = null == $subject ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_DENIED; + $vote = null == $subject ? Vote::createGranted() : Vote::createDenied(); } else { - $vote = VoterInterface::ACCESS_ABSTAIN; + $vote = Vote::createAbstain(); } $sut->addVoterVote($voter2, $attributes, $vote); @@ -220,12 +225,12 @@ public function testAccessDecisionManagerCalledByVoter() $voter3 ->expects($this->any()) - ->method('vote') + ->method('getVote') ->willReturnCallback(function (TokenInterface $token, $subject, array $attributes) use ($sut, $voter3) { if (\in_array('attr2', $attributes) && $subject) { - $vote = $sut->decide($token, $attributes) ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_DENIED; + $vote = $sut->getDecision($token, $attributes)->isGranted() ? Vote::createGranted() : Vote::createDenied(); } else { - $vote = VoterInterface::ACCESS_ABSTAIN; + $vote = Vote::createAbstain(); } $sut->addVoterVote($voter3, $attributes, $vote); @@ -234,36 +239,45 @@ public function testAccessDecisionManagerCalledByVoter() }); $token = $this->createMock(TokenInterface::class); - $sut->decide($token, ['attr1'], null); - $sut->decide($token, ['attr2'], $obj = new \stdClass()); + $sut->getDecision($token, ['attr1'], null); + $sut->getDecision($token, ['attr2'], $obj = new \stdClass()); $this->assertEquals([ [ 'attributes' => ['attr1'], 'object' => null, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => AccessDecision::createGranted([ + Vote::createGranted(), + ]), ], [ 'attributes' => ['attr2'], 'object' => null, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => Vote::createAbstain()], + ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createGranted(), + ]), ], [ 'attributes' => ['attr2'], 'object' => $obj, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_DENIED], - ['voter' => $voter3, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => Vote::createAbstain()], + ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => Vote::createDenied()], + ['voter' => $voter3, 'attributes' => ['attr2'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createDenied(), + Vote::createGranted(), + ]), ], ], $sut->getDecisionLog()); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php index 88544c081f78c..62b989a8e4980 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -12,28 +12,64 @@ namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\RememberMeToken; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\User\InMemoryUser; class AuthenticatedVoterTest extends TestCase { + use ExpectDeprecationTrait; + + /** + * @dataProvider getGetVoteTests + */ + public function testGetVote($authenticated, $attributes, $expected) + { + $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); + + $this->assertEquals($expected, $voter->getVote($this->getToken($authenticated), null, $attributes)); + } + + public static function getGetVoteTests() + { + return [ + ['fully', [], Vote::createAbstain()], + ['fully', ['FOO'], Vote::createAbstain()], + ['remembered', [], Vote::createAbstain()], + ['remembered', ['FOO'], Vote::createAbstain()], + + ['fully', ['IS_AUTHENTICATED_REMEMBERED'], Vote::createGranted()], + ['remembered', ['IS_AUTHENTICATED_REMEMBERED'], Vote::createGranted()], + + ['fully', ['IS_AUTHENTICATED_FULLY'], Vote::createGranted()], + ['remembered', ['IS_AUTHENTICATED_FULLY'], Vote::createDenied()], + + ['fully', ['IS_IMPERSONATOR'], Vote::createDenied()], + ['remembered', ['IS_IMPERSONATOR'], Vote::createDenied()], + ['impersonated', ['IS_IMPERSONATOR'], Vote::createGranted()], + ]; + } + /** - * @dataProvider getVoteTests + * @group legacy + * @dataProvider getVoteTestsLegacy */ - public function testVote($authenticated, $attributes, $expected) + public function testVoteLegacy($authenticated, $attributes, $expected) { $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); + $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $this->assertSame($expected, $voter->vote($this->getToken($authenticated), null, $attributes)); } - public static function getVoteTests() + public static function getVoteTestsLegacy() { return [ ['fully', [], VoterInterface::ACCESS_ABSTAIN], diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php index 369b17f0460ea..a003392b36060 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php @@ -12,27 +12,56 @@ namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\ExpressionLanguage\Expression; use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Authorization\ExpressionLanguage; use Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class ExpressionVoterTest extends TestCase { + use ExpectDeprecationTrait; + + /** + * @dataProvider getGetVoteTests + */ + public function testGetVoteWithTokenThatReturnsRoleNames(array $roles, array $attributes, Vote $expected, $tokenExpectsGetRoles = true, $expressionLanguageExpectsEvaluate = true) + { + $voter = new ExpressionVoter($this->createExpressionLanguage($expressionLanguageExpectsEvaluate), $this->createTrustResolver(), $this->createAuthorizationChecker()); + + $this->assertEquals($expected, $voter->getVote($this->getTokenWithRoleNames($roles, $tokenExpectsGetRoles), null, $attributes)); + } + + public function getGetVoteTests() + { + return [ + [[], [], Vote::createAbstain(), false, false], + [[], ['FOO'], Vote::createAbstain(), false, false], + + [[], [$this->createExpression()], Vote::createDenied(), true, false], + + [['ROLE_FOO'], [$this->createExpression(), $this->createExpression()], Vote::createGranted()], + [['ROLE_BAR', 'ROLE_FOO'], [$this->createExpression()], Vote::createGranted()], + ]; + } + /** - * @dataProvider getVoteTests + * @group legacy + * @dataProvider getVoteTestsLegacy */ - public function testVoteWithTokenThatReturnsRoleNames($roles, $attributes, $expected, $tokenExpectsGetRoles = true, $expressionLanguageExpectsEvaluate = true) + public function testVoteWithTokenThatReturnsRoleNamesLegacy($roles, $attributes, $expected, $tokenExpectsGetRoles = true, $expressionLanguageExpectsEvaluate = true) { $voter = new ExpressionVoter($this->createExpressionLanguage($expressionLanguageExpectsEvaluate), $this->createTrustResolver(), $this->createAuthorizationChecker()); + $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles, $tokenExpectsGetRoles), null, $attributes)); } - public static function getVoteTests() + public static function getVoteTestsLegacy() { return [ [[], [], VoterInterface::ACCESS_ABSTAIN, false, false], diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleHierarchyVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleHierarchyVoterTest.php index b811bd745bb85..53318e12eea63 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleHierarchyVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleHierarchyVoterTest.php @@ -12,19 +12,38 @@ namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; use Symfony\Component\Security\Core\Authorization\Voter\RoleHierarchyVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Role\RoleHierarchy; class RoleHierarchyVoterTest extends RoleVoterTest { /** + * @dataProvider getGetVoteTests + */ + public function testGetVoteUsingTokenThatReturnsRoleNames($roles, $attributes, $expected) + { + $voter = new RoleHierarchyVoter(new RoleHierarchy(['ROLE_FOO' => ['ROLE_FOOBAR']])); + + $this->assertEquals($expected, $voter->getVote($this->getTokenWithRoleNames($roles), null, $attributes)); + } + + public function getGetVoteTests() + { + return array_merge(parent::getGetVoteTests(), [ + [['ROLE_FOO'], ['ROLE_FOOBAR'], Vote::createGranted()], + ]); + } + + /** + * @group legacy * @dataProvider getVoteTests */ - public function testVoteUsingTokenThatReturnsRoleNames($roles, $attributes, $expected) + public function testVoteUsingTokenThatReturnsRoleNamesLegacy($roles, $attributes, $expected) { $voter = new RoleHierarchyVoter(new RoleHierarchy(['ROLE_FOO' => ['ROLE_FOOBAR']])); - $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)); + $this->assertEquals($expected, $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)); } public static function getVoteTests() @@ -41,7 +60,7 @@ public function testVoteWithEmptyHierarchyUsingTokenThatReturnsRoleNames($roles, { $voter = new RoleHierarchyVoter(new RoleHierarchy([])); - $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)); + $this->assertEquals($expected, $voter->getVote($this->getTokenWithRoleNames($roles), null, $attributes)->getAccess()); } public static function getVoteWithEmptyHierarchyTests() diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php index dfa0555652fba..afdcec55b25a9 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php @@ -12,21 +12,53 @@ namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Authorization\Voter\RoleVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class RoleVoterTest extends TestCase { + use ExpectDeprecationTrait; + + /** + * @dataProvider getGetVoteTests + */ + public function testGetVoteUsingTokenThatReturnsRoleNames($roles, $attributes, $expected) + { + $voter = new RoleVoter(); + + $this->assertEquals($expected, $voter->getVote($this->getTokenWithRoleNames($roles), null, $attributes)); + } + + public function getGetVoteTests() + { + return [ + [[], [], Vote::createAbstain()], + [[], ['FOO'], Vote::createAbstain()], + [[], ['ROLE_FOO'], Vote::createDenied()], + [['ROLE_FOO'], ['ROLE_FOO'], Vote::createGranted()], + [['ROLE_FOO'], ['FOO', 'ROLE_FOO'], Vote::createGranted()], + [['ROLE_BAR', 'ROLE_FOO'], ['ROLE_FOO'], Vote::createGranted()], + + // Test mixed Types + [[], [[]], Vote::createAbstain()], + [[], [new \stdClass()], Vote::createAbstain()], + ]; + } + /** + * @group legacy * @dataProvider getVoteTests */ - public function testVoteUsingTokenThatReturnsRoleNames($roles, $attributes, $expected) + public function testVoteUsingTokenThatReturnsRoleNamesLegacy($roles, $attributes, $expected) { $voter = new RoleVoter(); + $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php index d0f8ae08f97db..7cc2decd26bbb 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php @@ -12,15 +12,19 @@ namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Event\VoteEvent; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; class TraceableVoterTest extends TestCase { + use ExpectDeprecationTrait; + public function testGetDecoratedVoterClass() { $voter = $this->getMockBuilder(VoterInterface::class)->getMockForAbstractClass(); @@ -30,6 +34,38 @@ public function testGetDecoratedVoterClass() } public function testVote() + { + $voter = $this + ->getMockBuilder(VoterInterface::class) + ->setMethods(['getVote', 'vote']) + ->getMock(); + + $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass(); + $token = $this->getMockBuilder(TokenInterface::class)->getMockForAbstractClass(); + + $vote = Vote::createDenied(); + + $voter + ->expects($this->once()) + ->method('getVote') + ->with($token, 'anysubject', ['attr1']) + ->willReturn($vote); + + $eventDispatcher + ->expects($this->once()) + ->method('dispatch') + ->with(new VoteEvent($voter, 'anysubject', ['attr1'], $vote), 'debug.security.authorization.vote'); + + $sut = new TraceableVoter($voter, $eventDispatcher); + $result = $sut->getVote($token, 'anysubject', ['attr1']); + + $this->assertSame($vote, $result); + } + + /** + * @group legacy + */ + public function testVoteLegacy() { $voter = $this->getMockBuilder(VoterInterface::class)->getMockForAbstractClass(); @@ -48,6 +84,8 @@ public function testVote() ->with(new VoteEvent($voter, 'anysubject', ['attr1'], VoterInterface::ACCESS_DENIED), 'debug.security.authorization.vote'); $sut = new TraceableVoter($voter, $eventDispatcher); + + $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $result = $sut->vote($token, 'anysubject', ['attr1']); $this->assertSame(VoterInterface::ACCESS_DENIED, $result); diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php index 80c3f4a00b6a2..994ecb0bf64f5 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php @@ -12,12 +12,16 @@ namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\Voter; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class VoterTest extends TestCase { + use ExpectDeprecationTrait; + protected TokenInterface $token; protected function setUp(): void @@ -30,6 +34,44 @@ public static function getTests(): array $voter = new VoterTest_Voter(); $integerVoter = new IntegerVoterTest_Voter(); + return [ + [$voter, ['EDIT'], Vote::createGranted(), new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'], + [$voter, ['CREATE'], Vote::createDenied(), new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access'], + + [$voter, ['DELETE', 'EDIT'], Vote::createGranted(), new \stdClass(), 'ACCESS_GRANTED if one attribute is supported and grants access'], + [$voter, ['DELETE', 'CREATE'], Vote::createDenied(), new \stdClass(), 'ACCESS_DENIED if one attribute is supported and denies access'], + + [$voter, ['CREATE', 'EDIT'], Vote::createGranted(), new \stdClass(), 'ACCESS_GRANTED if one attribute grants access'], + + [$voter, ['DELETE'], Vote::createAbstain(), new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'], + + [$voter, ['EDIT'], Vote::createAbstain(), $this, 'ACCESS_ABSTAIN if class is not supported'], + + [$voter, ['EDIT'], Vote::createAbstain(), null, 'ACCESS_ABSTAIN if object is null'], + + [$voter, [], Vote::createAbstain(), new \stdClass(), 'ACCESS_ABSTAIN if no attributes were provided'], + + [$voter, [new StringableAttribute()], Vote::createGranted(), new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'], + + [$voter, [new \stdClass()], Vote::createAbstain(), new \stdClass(), 'ACCESS_ABSTAIN if attributes were not strings'], + + [$integerVoter, [42], Vote::createGranted(), new \stdClass(), 'ACCESS_GRANTED if attribute is an integer'], + ]; + } + + /** + * @dataProvider getTests + */ + public function testGetVote(VoterInterface $voter, array $attributes, $expectedVote, $object, $message) + { + $this->assertEquals($expectedVote, $voter->getVote($this->token, $object, $attributes), $message); + } + + public function getTestsLegacy() + { + $voter = new VoterLegacyTest_Voter(); + $integerVoter = new IntegerVoterLegacyTest_Voter(); + return [ [$voter, ['EDIT'], VoterInterface::ACCESS_GRANTED, new \stdClass(), 'ACCESS_GRANTED if attribute and class are supported and attribute grants access'], [$voter, ['CREATE'], VoterInterface::ACCESS_DENIED, new \stdClass(), 'ACCESS_DENIED if attribute and class are supported and attribute does not grant access'], @@ -56,23 +98,45 @@ public static function getTests(): array } /** - * @dataProvider getTests + * @group legacy + * @dataProvider getTestsLegacy */ - public function testVote(VoterInterface $voter, array $attributes, $expectedVote, $object, $message) + public function testVoteLegacy(VoterInterface $voter, array $attributes, $expectedVote, $object, $message) { + $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message); } - public function testVoteWithTypeError() + public function testVoteMessage() + { + $voter = new IntegerVoterVoteTest_Voter(); + $vote = $voter->getVote($this->token, new \stdClass(), [43, 44]); + $this->assertSame('foobar message', $vote->getMessage()); + } + + public function testGetVoteWithTypeError() { $this->expectException(\TypeError::class); $this->expectExceptionMessage('Should error'); $voter = new TypeErrorVoterTest_Voter(); - $voter->vote($this->token, new \stdClass(), ['EDIT']); + $voter->getVote($this->token, new \stdClass(), ['EDIT']); } } class VoterTest_Voter extends Voter +{ + protected function voteOnAttribute(string $attribute, $object, TokenInterface $token): Vote + { + return 'EDIT' === $attribute ? Vote::createGranted() : Vote::createDenied(); + } + + protected function supports(string $attribute, $object): bool + { + return $object instanceof \stdClass && \in_array($attribute, ['EDIT', 'CREATE']); + } +} + +class VoterLegacyTest_Voter extends Voter { protected function voteOnAttribute(string $attribute, $object, TokenInterface $token): bool { @@ -86,6 +150,19 @@ protected function supports(string $attribute, $object): bool } class IntegerVoterTest_Voter extends Voter +{ + protected function voteOnAttribute($attribute, $object, TokenInterface $token): Vote + { + return 42 === $attribute ? Vote::createGranted() : Vote::createDenied(); + } + + protected function supports($attribute, $object): bool + { + return $object instanceof \stdClass && \is_int($attribute); + } +} + +class IntegerVoterLegacyTest_Voter extends Voter { protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool { @@ -98,6 +175,23 @@ protected function supports($attribute, $object): bool } } +class IntegerVoterVoteTest_Voter extends Voter +{ + protected function voteOnAttribute($attribute, $object, TokenInterface $token): Vote + { + if (42 === $attribute) { + return Vote::createGranted(); + } + + return Vote::createDenied(' foobar message '); + } + + protected function supports($attribute, $object): bool + { + return $object instanceof \stdClass && \is_int($attribute); + } +} + class TypeErrorVoterTest_Voter extends Voter { protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool diff --git a/src/Symfony/Component/Security/Core/Tests/Exception/AccessDeniedExceptionTest.php b/src/Symfony/Component/Security/Core/Tests/Exception/AccessDeniedExceptionTest.php new file mode 100644 index 0000000000000..8c28ccf09abda --- /dev/null +++ b/src/Symfony/Component/Security/Core/Tests/Exception/AccessDeniedExceptionTest.php @@ -0,0 +1,64 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Tests\Exception; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Exception\AccessDeniedException; + +final class AccessDeniedExceptionTest extends TestCase +{ + /** + * @dataProvider getAccessDescisions + */ + public function testSetAccessDecision(AccessDecision $accessDecision, string $expected) + { + $exception = new AccessDeniedException(); + $exception->setAccessDecision($accessDecision); + + $this->assertSame($expected, $exception->getMessage()); + } + + public function getAccessDescisions(): \Generator + { + yield [ + AccessDecision::createDenied([ + Vote::createDenied('foo'), + Vote::createDenied('bar'), + Vote::createDenied('baz'), + ]), + 'Access Denied. foo bar baz', + ]; + + yield [ + AccessDecision::createDenied(), + 'Access Denied.', + ]; + + yield [ + AccessDecision::createDenied([ + Vote::createAbstain('foo'), + Vote::createDenied('bar'), + Vote::createAbstain('baz'), + ]), + 'Access Denied. bar', + ]; + + yield [ + AccessDecision::createGranted([ + Vote::createDenied('foo'), + ]), + 'Access Denied. foo', + ]; + } +} diff --git a/src/Symfony/Component/Security/Http/Firewall/AccessListener.php b/src/Symfony/Component/Security/Http/Firewall/AccessListener.php index 9f4337975c58b..e88d3032c9086 100644 --- a/src/Symfony/Component/Security/Http/Firewall/AccessListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/AccessListener.php @@ -15,6 +15,7 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Exception\AccessDeniedException; @@ -77,16 +78,23 @@ public function authenticate(RequestEvent $event): void $token = $this->tokenStorage->getToken() ?? new NullToken(); - if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) { - throw $this->createAccessDeniedException($request, $attributes); + if (method_exists($this->accessDecisionManager, 'getDecision')) { + $decision = $this->accessDecisionManager->getDecision($token, $attributes, $request, true); + } else { + $decision = $this->accessDecisionManager->decide($token, $attributes, $request, true) ? AccessDecision::createGranted() : AccessDecision::createDenied(); + } + + if ($decision->isDenied()) { + throw $this->createAccessDeniedException($request, $attributes, $decision); } } - private function createAccessDeniedException(Request $request, array $attributes): AccessDeniedException + private function createAccessDeniedException(Request $request, array $attributes, AccessDecision $accessDecision): AccessDeniedException { $exception = new AccessDeniedException(); $exception->setAttributes($attributes); $exception->setSubject($request); + $exception->setAccessDecision($accessDecision); return $exception; } diff --git a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php index e72107190449a..207fa0e550804 100644 --- a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php @@ -19,6 +19,7 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; @@ -168,9 +169,16 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn throw $e; } - if (false === $this->accessDecisionManager->decide($token, [$this->role], $user)) { + if (method_exists($this->accessDecisionManager, 'getDecision')) { + $decision = $this->accessDecisionManager->getDecision($token, [$this->role], $user); + } else { + $decision = $this->accessDecisionManager->decide($token, [$this->role], $user) ? AccessDecision::createGranted() : AccessDecision::createDenied(); + } + + if ($decision->isDenied()) { $exception = new AccessDeniedException(); $exception->setAttributes($this->role); + $exception->setAccessDecision($decision); throw $exception; } diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index 181454e43ec33..f00e50c39193d 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -20,6 +20,7 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Exception\AccessDeniedException; @@ -56,6 +57,55 @@ public function getCredentials(): mixed ->willReturn($token) ; + $accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); + $accessDecisionManager + ->expects($this->once()) + ->method('getDecision') + ->with($this->equalTo($token), $this->equalTo(['foo' => 'bar']), $this->equalTo($request)) + ->willReturn(AccessDecision::createDenied()) + ; + + $listener = new AccessListener( + $tokenStorage, + $accessDecisionManager, + $accessMap + ); + + $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); + } + + /** + * @group legacy + */ + public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccessLegacy() + { + $this->expectException(AccessDeniedException::class); + $request = new Request(); + + $accessMap = $this->createMock(AccessMapInterface::class); + $accessMap + ->expects($this->any()) + ->method('getPatterns') + ->with($this->equalTo($request)) + ->willReturn([['foo' => 'bar'], null]) + ; + + $token = new class() extends AbstractToken { + public function getCredentials(): mixed + { + } + }; + + $tokenStorage = $this->createMock(TokenStorageInterface::class); + $tokenStorage + ->expects($this->any()) + ->method('getToken') + ->willReturn($token) + ; + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $accessDecisionManager ->expects($this->once()) @@ -142,11 +192,14 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken() ->willReturn([['foo' => 'bar'], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); $accessDecisionManager->expects($this->once()) - ->method('decide') + ->method('getDecision') ->with($this->isInstanceOf(NullToken::class)) - ->willReturn(false); + ->willReturn(AccessDecision::createDenied()); $listener = new AccessListener( $tokenStorage, @@ -170,11 +223,14 @@ public function testHandleWhenPublicAccessIsAllowed() ->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); $accessDecisionManager->expects($this->once()) - ->method('decide') + ->method('getDecision') ->with($this->isInstanceOf(NullToken::class), [AuthenticatedVoter::PUBLIC_ACCESS]) - ->willReturn(true); + ->willReturn(AccessDecision::createGranted()); $listener = new AccessListener( $tokenStorage, @@ -200,11 +256,14 @@ public function testHandleWhenPublicAccessWhileAuthenticated() ->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); $accessDecisionManager->expects($this->once()) - ->method('decide') + ->method('getDecision') ->with($this->equalTo($token), [AuthenticatedVoter::PUBLIC_ACCESS]) - ->willReturn(true); + ->willReturn(AccessDecision::createGranted()); $listener = new AccessListener( $tokenStorage, @@ -233,12 +292,15 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() $tokenStorage = new TokenStorage(); $tokenStorage->setToken($authenticatedToken); - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); $accessDecisionManager ->expects($this->once()) - ->method('decide') + ->method('getDecision') ->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), true) - ->willReturn(true) + ->willReturn(AccessDecision::createGranted()) ; $listener = new AccessListener( diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php index 916e54d669376..13c45005fd2e5 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php @@ -20,6 +20,7 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; @@ -45,7 +46,10 @@ protected function setUp(): void $this->tokenStorage = new TokenStorage(); $this->userProvider = new InMemoryUserProvider(['kuba' => []]); $this->userChecker = $this->createMock(UserCheckerInterface::class); - $this->accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $this->accessDecisionManager = $this + ->getMockBuilder(AccessDecisionManagerInterface::class) + ->setMethods(['getDecision', 'decide']) + ->getMock(); $this->request = new Request(); $this->event = new RequestEvent($this->createMock(HttpKernelInterface::class), $this->request, HttpKernelInterface::MAIN_REQUEST); } @@ -142,8 +146,8 @@ public function testSwitchUserIsDisallowed() $this->request->query->set('_switch_user', 'kuba'); $this->accessDecisionManager->expects($this->once()) - ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH']) - ->willReturn(false); + ->method('getDecision')->with($token, ['ROLE_ALLOWED_TO_SWITCH']) + ->willReturn(AccessDecision::createDenied()); $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager); $listener($this->event); @@ -158,7 +162,7 @@ public function testSwitchUserTurnsAuthenticationExceptionTo403() $this->request->query->set('_switch_user', 'not-existing'); $this->accessDecisionManager->expects($this->never()) - ->method('decide'); + ->method('getDecision'); $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager); $listener($this->event); @@ -172,8 +176,8 @@ public function testSwitchUser() $this->request->query->set('_switch_user', 'kuba'); $this->accessDecisionManager->expects($this->once()) - ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier())) - ->willReturn(true); + ->method('getDecision')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier())) + ->willReturn(AccessDecision::createGranted()); $this->userChecker->expects($this->once()) ->method('checkPostAuth')->with($this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier())); @@ -186,6 +190,32 @@ public function testSwitchUser() $this->assertInstanceOf(UsernamePasswordToken::class, $this->tokenStorage->getToken()); } + /** + * @group legacy + */ + public function testSwitchUserLegacy() + { + $token = new UsernamePasswordToken(new InMemoryUser('username', '', ['ROLE_FOO']), 'key', ['ROLE_FOO']); + + $this->tokenStorage->setToken($token); + $this->request->query->set('_switch_user', 'kuba'); + + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager->expects($this->once()) + ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(function ($user) { return 'kuba' === $user->getUserIdentifier(); })) + ->willReturn(true); + + $this->userChecker->expects($this->once()) + ->method('checkPostAuth')->with($this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier())); + + $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $accessDecisionManager); + $listener($this->event); + + $this->assertSame([], $this->request->query->all()); + $this->assertSame('', $this->request->server->get('QUERY_STRING')); + $this->assertInstanceOf(UsernamePasswordToken::class, $this->tokenStorage->getToken()); + } + public function testSwitchUserAlreadySwitched() { $originalToken = new UsernamePasswordToken(new InMemoryUser('original', null, ['ROLE_FOO']), 'key', ['ROLE_FOO']); @@ -198,8 +228,8 @@ public function testSwitchUserAlreadySwitched() $targetsUser = $this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier()); $this->accessDecisionManager->expects($this->once()) - ->method('decide')->with($originalToken, ['ROLE_ALLOWED_TO_SWITCH'], $targetsUser) - ->willReturn(true); + ->method('getDecision')->with($originalToken, ['ROLE_ALLOWED_TO_SWITCH'], $targetsUser) + ->willReturn(AccessDecision::createGranted()); $this->userChecker->expects($this->once()) ->method('checkPostAuth')->with($targetsUser); @@ -224,8 +254,8 @@ public function testSwitchUserWorksWithFalsyUsernames() $this->userProvider->createUser($user = new InMemoryUser('0', null)); $this->accessDecisionManager->expects($this->once()) - ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH']) - ->willReturn(true); + ->method('getDecision')->with($token, ['ROLE_ALLOWED_TO_SWITCH']) + ->willReturn(AccessDecision::createGranted()); $this->userChecker->expects($this->once()) ->method('checkPostAuth')->with($this->callback(fn ($argUser) => $user->isEqualTo($argUser))); @@ -251,8 +281,8 @@ public function testSwitchUserKeepsOtherQueryStringParameters() $targetsUser = $this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier()); $this->accessDecisionManager->expects($this->once()) - ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $targetsUser) - ->willReturn(true); + ->method('getDecision')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $targetsUser) + ->willReturn(AccessDecision::createGranted()); $this->userChecker->expects($this->once()) ->method('checkPostAuth')->with($targetsUser); @@ -276,8 +306,8 @@ public function testSwitchUserWithReplacedToken() $this->request->query->set('_switch_user', 'kuba'); $this->accessDecisionManager->expects($this->any()) - ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier())) - ->willReturn(true); + ->method('getDecision')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier())) + ->willReturn(AccessDecision::createGranted()); $dispatcher = $this->createMock(EventDispatcherInterface::class); $dispatcher @@ -319,8 +349,8 @@ public function testSwitchUserStateless() $targetsUser = $this->callback(fn ($user) => 'kuba' === $user->getUserIdentifier()); $this->accessDecisionManager->expects($this->once()) - ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $targetsUser) - ->willReturn(true); + ->method('getDecision')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $targetsUser) + ->willReturn(AccessDecision::createGranted()); $this->userChecker->expects($this->once()) ->method('checkPostAuth')->with($targetsUser); From 5e4733be766c03c5f2cf71d6d298c2d6a9d80cc5 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Tue, 28 Jun 2022 22:40:06 +0200 Subject: [PATCH 2/4] [Security] Vote can have multiple messages --- .../Core/Authorization/Voter/Vote.php | 79 ++++++++++++++----- .../Core/Authorization/Voter/Voter.php | 26 ++++-- .../Tests/Authorization/Voter/VoteTest.php | 38 +++++++++ .../Tests/Authorization/Voter/VoterTest.php | 11 ++- 4 files changed, 126 insertions(+), 28 deletions(-) create mode 100644 src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoteTest.php diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php index a0df7b2e5e7b1..2ee8f69e0e4be 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -11,26 +11,36 @@ namespace Symfony\Component\Security\Core\Authorization\Voter; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; + /** * A Vote is returned by a Voter and contains the access (granted, abstain or denied). - * It can also contain a message explaining the vote decision. + * It can also contain one or multiple messages explaining the vote decision. * * @author Dany Maillard + * @author Antoine Lamirault */ final class Vote { - /** @var int One of the VoterInterface::ACCESS_* constants */ - private $access; - private $message; - private $context; + /** + * @var int One of the VoterInterface::ACCESS_* constants + */ + private int $access; + + /** + * @var string[] + */ + private array $messages; + + private array $context; /** * @param int $access One of the VoterInterface::ACCESS_* constants */ - public function __construct(int $access, string $message = '', array $context = []) + public function __construct(int $access, string|array $messages = [], array $context = []) { $this->access = $access; - $this->message = $message; + $this->setMessages($messages); $this->context = $context; } @@ -54,34 +64,67 @@ public function isDenied(): bool return VoterInterface::ACCESS_DENIED === $this->access; } - public static function create(int $access, string $message = '', array $context = []): self + /** + * @param string|string[] $messages + */ + public static function create(int $access, string|array $messages = [], array $context = []): self { - return new self($access, $message, $context); + return new self($access, $messages, $context); } - public static function createGranted(string $message = '', array $context = []): self + /** + * @param string|string[] $messages + */ + public static function createGranted(string|array $messages = [], array $context = []): self { - return new self(VoterInterface::ACCESS_GRANTED, $message, $context); + return new self(VoterInterface::ACCESS_GRANTED, $messages, $context); } - public static function createAbstain(string $message = '', array $context = []): self + /** + * @param string|string[] $messages + */ + public static function createAbstain(string|array $messages = [], array $context = []): self { - return new self(VoterInterface::ACCESS_ABSTAIN, $message, $context); + return new self(VoterInterface::ACCESS_ABSTAIN, $messages, $context); } - public static function createDenied(string $message = '', array $context = []): self + /** + * @param string|string[] $messages + */ + public static function createDenied(string|array $messages = [], array $context = []): self + { + return new self(VoterInterface::ACCESS_DENIED, $messages, $context); + } + + /** + * @param string|string[] $messages + */ + public function setMessages(string|array $messages) + { + $this->messages = (array) $messages; + foreach ($this->messages as $message) { + if (!\is_string($message)) { + throw new InvalidArgumentException(sprintf('Message must be string, "%s" given.', get_debug_type($message))); + } + } + } + + public function addMessage(string $message) { - return new self(VoterInterface::ACCESS_DENIED, $message, $context); + $this->messages[] = $message; } - public function setMessage(string $message) + /** + * @return string[] + */ + public function getMessages(): array { - $this->message = $message; + return $this->messages; } public function getMessage(): string { - return $this->message; + return implode(', ', $this->messages); } public function getContext(): array diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index d3226e6889383..da9cdc1c08bee 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -43,7 +43,9 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes } // as soon as at least one attribute is supported, default is to deny access - $vote = $this->deny(); + if (!$vote->isDenied()) { + $vote = $this->deny(); + } $decision = $this->voteOnAttribute($attribute, $subject, $token); if (\is_bool($decision)) { @@ -56,7 +58,9 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes return $decision; } - $vote->setMessage($vote->getMessage().trim(' '.$decision->getMessage())); + if ('' !== $decision->getMessage()) { + $vote->addMessage($decision->getMessage()); + } } return $vote; @@ -71,26 +75,32 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): /** * Creates a granted vote. + * + * @param string|string[] $messages */ - protected function grant(string $message = '', array $context = []): Vote + protected function grant(string|array $messages = [], array $context = []): Vote { - return Vote::createGranted($message, $context); + return Vote::createGranted($messages, $context); } /** * Creates an abstained vote. + * + * @param string|string[] $messages */ - protected function abstain(string $message = '', array $context = []): Vote + protected function abstain(string|array $messages = [], array $context = []): Vote { - return Vote::createAbstain($message, $context); + return Vote::createAbstain($messages, $context); } /** * Creates a denied vote. + * + * @param string|string[] $messages */ - protected function deny(string $message = '', array $context = []): Vote + protected function deny(string|array $messages = [], array $context = []): Vote { - return Vote::createDenied($message, $context); + return Vote::createDenied($messages, $context); } /** diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoteTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoteTest.php new file mode 100644 index 0000000000000..96bdbbfa0df80 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoteTest.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; + +use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\Exception\InvalidArgumentException; + +class VoteTest extends TestCase +{ + public function testMessages() + { + $vote = new Vote(VoterInterface::ACCESS_GRANTED, 'foo'); + + $this->assertSame('foo', $vote->getMessage()); + + $vote->addMessage('bar'); + $this->assertSame('foo, bar', $vote->getMessage()); + } + + public function testMessagesWithNotString() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Message must be string, "bool" given.'); + + new Vote(VoterInterface::ACCESS_GRANTED, ['foo', true]); + } +} diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php index 994ecb0bf64f5..78b51ceda32f6 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php @@ -110,10 +110,17 @@ public function testVoteLegacy(VoterInterface $voter, array $attributes, $expect public function testVoteMessage() { $voter = new IntegerVoterVoteTest_Voter(); - $vote = $voter->getVote($this->token, new \stdClass(), [43, 44]); + $vote = $voter->getVote($this->token, new \stdClass(), [43]); $this->assertSame('foobar message', $vote->getMessage()); } + public function testVoteMessageMultipleAttributes() + { + $voter = new IntegerVoterVoteTest_Voter(); + $vote = $voter->getVote($this->token, new \stdClass(), [43, 44]); + $this->assertSame('foobar message, foobar message', $vote->getMessage()); + } + public function testGetVoteWithTypeError() { $this->expectException(\TypeError::class); @@ -183,7 +190,7 @@ protected function voteOnAttribute($attribute, $object, TokenInterface $token): return Vote::createGranted(); } - return Vote::createDenied(' foobar message '); + return Vote::createDenied('foobar message'); } protected function supports($attribute, $object): bool From a23e15b384e5221a93f2f36e04cebffd3c147236 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Fri, 10 Mar 2023 21:41:37 +0100 Subject: [PATCH 3/4] Rebase for 6.3 --- UPGRADE-6.4.md | 10 +++++++ .../SecurityDataCollectorTest.php | 5 ++++ .../Tests/EventListener/VoteListenerTest.php | 2 +- .../Core/Authorization/AccessDecision.php | 14 ++-------- .../Authorization/AccessDecisionManager.php | 10 ++----- .../AccessDecisionManagerInterface.php | 2 +- .../Authorization/AuthorizationChecker.php | 2 +- .../AccessDecisionStrategyInterface.php | 2 +- .../Strategy/AffirmativeStrategy.php | 2 +- .../Strategy/ConsensusStrategy.php | 8 +----- .../Strategy/PriorityStrategy.php | 2 +- .../Strategy/UnanimousStrategy.php | 2 +- .../TraceableAccessDecisionManager.php | 7 ++--- .../Voter/AuthenticatedVoter.php | 2 +- .../Authorization/Voter/ExpressionVoter.php | 2 +- .../Core/Authorization/Voter/RoleVoter.php | 2 +- .../Authorization/Voter/TraceableVoter.php | 2 +- .../Core/Authorization/Voter/Voter.php | 10 +++---- .../Authorization/Voter/VoterInterface.php | 4 +-- .../Component/Security/Core/CHANGELOG.md | 10 +++++++ .../Security/Core/Event/VoteEvent.php | 6 ++--- .../Core/Exception/AccessDeniedException.php | 14 ++++------ .../Test/AccessDecisionStrategyTestCase.php | 6 +++++ .../AccessDecisionManagerTest.php | 27 ++++++++++++------- .../AuthorizationCheckerTest.php | 16 ++++++----- .../TraceableAccessDecisionManagerTest.php | 12 ++++++--- .../Voter/AuthenticatedVoterTest.php | 2 +- .../Voter/ExpressionVoterTest.php | 2 +- .../Authorization/Voter/RoleVoterTest.php | 2 +- .../Voter/TraceableVoterTest.php | 5 ++-- .../Tests/Authorization/Voter/VoterTest.php | 6 ++--- .../Exception/AccessDeniedExceptionTest.php | 14 +++++++--- .../Core/Tests/Fixtures/DummyVoter.php | 5 ++++ .../Tests/Firewall/AccessListenerTest.php | 15 +++++++---- .../Tests/Firewall/SwitchUserListenerTest.php | 3 ++- 35 files changed, 137 insertions(+), 98 deletions(-) diff --git a/UPGRADE-6.4.md b/UPGRADE-6.4.md index ddfc9883a5acc..ce908321a7fef 100644 --- a/UPGRADE-6.4.md +++ b/UPGRADE-6.4.md @@ -136,6 +136,16 @@ Security * [BC break] `UserValueResolver` no longer implements `ArgumentValueResolverInterface` * [BC break] Make `PersistentToken` immutable * Deprecate accepting only `DateTime` for `TokenProviderInterface::updateToken()`, use `DateTimeInterface` instead + * Add method `getDecision()` to `AccessDecisionStrategyInterface` + * Deprecate `AccessDecisionStrategyInterface::decide()` in favor of `AccessDecisionStrategyInterface::getDecision()` + * Add method `getVote()` to `VoterInterface` + * Deprecate `VoterInterface::vote()` in favor of `AccessDecisionStrategyInterface::getVote()` + * Deprecate returning `bool` from `Voter::voteOnAttribute()` (it must return a `Vote`) + * Add method `getDecision()` to `AccessDecisionManagerInterface` + * Deprecate `AccessDecisionManagerInterface::decide()` in favor of `AccessDecisionManagerInterface::getDecision()` + * Add method `getDecision()` to `AuthorizationCheckerInterface` + * Add methods `setAccessDecision()` and `getAccessDecision()` to `AccessDeniedException` + * Add method `getDecision()` to `Security` SecurityBundle -------------- diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php index 9d2b056385de3..dc5f075f38adb 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php @@ -28,6 +28,7 @@ use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Role\RoleHierarchy; use Symfony\Component\Security\Core\User\InMemoryUser; @@ -434,4 +435,8 @@ final class DummyVoter implements VoterInterface public function vote(TokenInterface $token, mixed $subject, array $attributes): int { } + + public function getVote(TokenInterface $token, mixed $subject, array $attributes): Vote + { + } } diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php index 5ea42ee02fd3e..b2bc95b50ec28 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php @@ -50,7 +50,7 @@ public function testOnVoterVoteLegacy() $traceableAccessDecisionManager = $this ->getMockBuilder(TraceableAccessDecisionManager::class) ->disableOriginalConstructor() - ->setMethods(['addVoterVote']) + ->onlyMethods(['addVoterVote']) ->getMock(); $traceableAccessDecisionManager diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php index 0f1ea42c62976..dc53d7467e8b0 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -22,20 +22,12 @@ */ final class AccessDecision { - /** @var int One of the VoterInterface::ACCESS_* constants */ - private int $access; - - /** @var Vote[] */ - private array $votes = []; - /** * @param int $access One of the VoterInterface::ACCESS_* constants * @param Vote[] $votes */ - private function __construct(int $access, array $votes = []) + private function __construct(private readonly int $access, private readonly array $votes = []) { - $this->access = $access; - $this->votes = $votes; } public function getAccess(): int @@ -111,8 +103,6 @@ public function getDeniedVotes(): array */ private function getVotesByAccess(int $access): array { - return array_filter($this->votes, static function (Vote $vote) use ($access): bool { - return $vote->getAccess() === $access; - }); + return array_filter($this->votes, static fn (Vote $vote): bool => $vote->getAccess() === $access); } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index cdc6e48a5d8b9..0d7836dec0a27 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -27,12 +27,6 @@ */ final class AccessDecisionManager implements AccessDecisionManagerInterface { - private const VALID_VOTES = [ - VoterInterface::ACCESS_GRANTED => true, - VoterInterface::ACCESS_DENIED => true, - VoterInterface::ACCESS_ABSTAIN => true, - ]; - private iterable $voters; private array $votersCacheAttributes = []; private array $votersCacheObject = []; @@ -70,11 +64,11 @@ public function getDecision(TokenInterface $token, array $attributes, mixed $obj /** * @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array * - * @deprecated since Symfony 6.2, use {@see getDecision()} instead. + * @deprecated since Symfony 6.3, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); // Special case for AccessListener, do not remove the right side of the condition before 6.0 if (\count($attributes) > 1 && !$allowMultipleAttributes) { diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index 6c9629babae7a..b369ffe6b2f09 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -28,7 +28,7 @@ interface AccessDecisionManagerInterface * @param array $attributes An array of attributes associated with the method being invoked * @param mixed $object The object to secure * - * @deprecated since Symfony 6.2, use {@see getDecision()} instead. + * @deprecated since Symfony 6.3, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, mixed $object = null): bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index 32c0e96e4bbcc..dd0bd9b10c67a 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -51,7 +51,7 @@ final public function getDecision($attribute, $subject = null): AccessDecision } if (!method_exists($this->accessDecisionManager, 'getDecision')) { - trigger_deprecation('symfony/security-core', '6.2', 'Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.', \get_class($this->accessDecisionManager)); + trigger_deprecation('symfony/security-core', '6.3', 'Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.', \get_class($this->accessDecisionManager)); return $this->accessDecisionManager->decide($token, [$attribute], $subject) ? AccessDecision::createGranted() : AccessDecision::createDenied(); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php index a1bbe8fdab82c..ef0dfb7486b19 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php @@ -25,7 +25,7 @@ interface AccessDecisionStrategyInterface /** * @param \Traversable $results * - * @deprecated since Symfony 6.2, use {@see getDecision()} instead. + * @deprecated since Symfony 6.3, use {@see getDecision()} instead. */ public function decide(\Traversable $results): bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php index 0686a74f62dab..12bfd96238b7e 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php @@ -63,7 +63,7 @@ public function getDecision(\Traversable $votes): AccessDecision public function decide(\Traversable $results): bool { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); $deny = 0; foreach ($results as $result) { diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php index 8e20ed6df8d4f..e266618c23cd2 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php @@ -44,9 +44,6 @@ public function __construct(bool $allowIfAllAbstainDecisions = false, bool $allo $this->allowIfEqualGrantedDeniedDecisions = $allowIfEqualGrantedDeniedDecisions; } - /** - * {@inheritdoc} - */ public function getDecision(\Traversable $votes): AccessDecision { $currentVotes = []; @@ -84,12 +81,9 @@ public function getDecision(\Traversable $votes): AccessDecision ; } - /** - * {@inheritdoc} - */ public function decide(\Traversable $results): bool { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); $grant = 0; $deny = 0; diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php index b8674b5f3de45..24bd30cc63575 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php @@ -59,7 +59,7 @@ public function getDecision(\Traversable $votes): AccessDecision public function decide(\Traversable $results): bool { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); foreach ($results as $result) { if (VoterInterface::ACCESS_GRANTED === $result) { diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php index 3cf62aac99608..8f374772fc242 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php @@ -64,7 +64,7 @@ public function getDecision(\Traversable $votes): AccessDecision public function decide(\Traversable $results): bool { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); $grant = 0; foreach ($results as $result) { diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index 00150474df6c2..a196a3ea3efad 100644 --- a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php @@ -67,12 +67,9 @@ public function getDecision(TokenInterface $token, array $attributes, mixed $obj return $result; } - /** - * {@inheritdoc} - */ public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); $currentDecisionLog = [ 'attributes' => $attributes, @@ -100,7 +97,7 @@ public function decide(TokenInterface $token, array $attributes, mixed $object = public function addVoterVote(VoterInterface $voter, array $attributes, Vote|int $vote): void { if (!$vote instanceof Vote) { - trigger_deprecation('symfony/security-core', '6.2', 'Passing an int as the third argument to "%s::addVoterVote()" is deprecated, pass an instance of "%s" instead.', __CLASS__, Vote::class); + trigger_deprecation('symfony/security-core', '6.3', 'Passing an int as the third argument to "%s::addVoterVote()" is deprecated, pass an instance of "%s" instead.', __CLASS__, Vote::class); $vote = new Vote($vote); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php index 5225086a7397e..e08378d82354b 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php @@ -87,7 +87,7 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes public function vote(TokenInterface $token, mixed $subject, array $attributes): int { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); return $this->getVote($token, $subject, $attributes)->getAccess(); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php index eb980aebcaec3..1eb2643a223b0 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php @@ -71,7 +71,7 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes public function vote(TokenInterface $token, mixed $subject, array $attributes): int { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); return $this->getVote($token, $subject, $attributes)->getAccess(); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php index e92dc8b0ac37d..c4b49ed4bb3b5 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php @@ -50,7 +50,7 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes public function vote(TokenInterface $token, mixed $subject, array $attributes): int { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); return $this->getVote($token, $subject, $attributes)->getAccess(); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index cb1ff1a256a93..86f2efe834876 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -49,7 +49,7 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes public function vote(TokenInterface $token, mixed $subject, array $attributes): int { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); return $this->getVote($token, $subject, $attributes)->getAccess(); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index da9cdc1c08bee..b591d6113079f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -49,7 +49,7 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes $decision = $this->voteOnAttribute($attribute, $subject, $token); if (\is_bool($decision)) { - trigger_deprecation('symfony/security-core', '6.2', 'Returning a boolean in "%s::voteOnAttribute()" is deprecated, return an instance of "%s" instead.', static::class, Vote::class); + trigger_deprecation('symfony/security-core', '6.3', 'Returning a boolean in "%s::voteOnAttribute()" is deprecated, return an instance of "%s" instead.', static::class, Vote::class); $decision = $decision ? $this->grant() : $this->deny(); } @@ -58,8 +58,8 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes return $decision; } - if ('' !== $decision->getMessage()) { - $vote->addMessage($decision->getMessage()); + if ('' !== $decisionMessage = $decision->getMessage()) { + $vote->addMessage($decisionMessage); } } @@ -68,7 +68,7 @@ public function getVote(TokenInterface $token, mixed $subject, array $attributes public function vote(TokenInterface $token, mixed $subject, array $attributes): int { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.', __CLASS__, __CLASS__); return $this->getVote($token, $subject, $attributes)->getAccess(); } @@ -140,7 +140,7 @@ abstract protected function supports(string $attribute, mixed $subject): bool; * @param TAttribute $attribute * @param TSubject $subject * - * @return Vote|bool Returning a boolean is deprecated since Symfony 6.2. Return a Vote object instead. + * @return Vote|bool Returning a boolean is deprecated since Symfony 6.3. Return a Vote object instead. */ abstract protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token): Vote|bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php index 61b9700fe8561..93195cab7a515 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php @@ -16,7 +16,7 @@ /** * VoterInterface is the interface implemented by all voters. * - * @author Fabien Potencier * + * @author Fabien Potencier * * @method Vote getVote(TokenInterface $token, mixed $subject, array $attributes) */ @@ -39,7 +39,7 @@ interface VoterInterface * * @psalm-return self::ACCESS_* must be transformed into @return on Symfony 7 * - * @deprecated since Symfony 6.2, use {@see getVote()} instead. + * @deprecated since Symfony 6.3, use {@see getVote()} instead. */ public function vote(TokenInterface $token, mixed $subject, array $attributes); } diff --git a/src/Symfony/Component/Security/Core/CHANGELOG.md b/src/Symfony/Component/Security/Core/CHANGELOG.md index 5c56c2f79c13b..fec86a2aa17a9 100644 --- a/src/Symfony/Component/Security/Core/CHANGELOG.md +++ b/src/Symfony/Component/Security/Core/CHANGELOG.md @@ -6,6 +6,16 @@ CHANGELOG * Make `PersistentToken` immutable * Deprecate accepting only `DateTime` for `TokenProviderInterface::updateToken()`, use `DateTimeInterface` instead + * Add method `getDecision()` to `AccessDecisionStrategyInterface` + * Deprecate `AccessDecisionStrategyInterface::decide()` in favor of `AccessDecisionStrategyInterface::getDecision()` + * Add method `getVote()` to `VoterInterface` + * Deprecate `VoterInterface::vote()` in favor of `AccessDecisionStrategyInterface::getVote()` + * Deprecate returning `bool` from `Voter::voteOnAttribute()` (it must return a `Vote`) + * Add method `getDecision()` to `AccessDecisionManagerInterface` + * Deprecate `AccessDecisionManagerInterface::decide()` in favor of `AccessDecisionManagerInterface::getDecision()` + * Add method `getDecision()` to `AuthorizationCheckerInterface` + * Add methods `setAccessDecision()` and `getAccessDecision()` to `AccessDeniedException` + * Add method `getDecision()` to `Security` 6.3 --- diff --git a/src/Symfony/Component/Security/Core/Event/VoteEvent.php b/src/Symfony/Component/Security/Core/Event/VoteEvent.php index cd194d76dfe99..59dae34396df2 100644 --- a/src/Symfony/Component/Security/Core/Event/VoteEvent.php +++ b/src/Symfony/Component/Security/Core/Event/VoteEvent.php @@ -36,7 +36,7 @@ public function __construct(VoterInterface $voter, mixed $subject, array $attrib $this->attributes = $attributes; if (!$vote instanceof Vote) { - trigger_deprecation('symfony/security-core', '6.2', 'Passing an int as the fourth argument to "%s::__construct" is deprecated, pass a "%s" instance instead.', __CLASS__, Vote::class); + trigger_deprecation('symfony/security-core', '6.3', 'Passing an int as the fourth argument to "%s::__construct" is deprecated, pass a "%s" instance instead.', __CLASS__, Vote::class); $vote = new Vote($vote); } @@ -59,11 +59,11 @@ public function getAttributes(): array } /** - * @deprecated since Symfony 6.2, use {@see getVoteDecision()} instead. + * @deprecated since Symfony 6.3, use {@see getVoteDecision()} instead. */ public function getVote(): int { - trigger_deprecation('symfony/security-core', '6.2', 'Method "%s::getVote()" has been deprecated, use "%s::getVoteDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '6.3', 'Method "%s::getVote()" has been deprecated, use "%s::getVoteDecision()" instead.', __CLASS__, __CLASS__); return $this->vote->getAccess(); } diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index 6c8a9ffee7c09..4c3ef9e739827 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -25,7 +25,7 @@ class AccessDeniedException extends RuntimeException { private array $attributes = []; private mixed $subject = null; - private ?AccessDecision $accessDecision; + private ?AccessDecision $accessDecision = null; public function __construct(string $message = 'Access Denied.', \Throwable $previous = null, int $code = 403) { @@ -60,22 +60,18 @@ public function setSubject(mixed $subject) /** * Sets an access decision and appends the denied reasons to the exception message. - * - * @return void */ - public function setAccessDecision(AccessDecision $accessDecision) + public function setAccessDecision(AccessDecision $accessDecision): void { $this->accessDecision = $accessDecision; - if (!$accessDecision->getDeniedVotes()) { + if (!$deniedVotes = $accessDecision->getDeniedVotes()) { return; } - $messages = array_map(static function (Vote $vote) { - return $vote->getMessage(); - }, $accessDecision->getDeniedVotes()); + $messages = array_map(static fn (Vote $vote): string => sprintf('"%s"', $vote->getMessage()), $deniedVotes); if ($messages) { - $this->message .= ' '.implode(' ', $messages); + $this->message .= sprintf(' Decision message%s %s', \count($messages) > 1 ? 's are' : ' is', implode(' and ', $messages)); } } diff --git a/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php b/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php index 460526b03255c..b6a9db7bdee51 100644 --- a/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php +++ b/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php @@ -41,6 +41,7 @@ final public function testGetDecision(AccessDecisionStrategyInterface $strategy, /** * @group legacy + * * @dataProvider provideStrategyTests * * @param VoterInterface[] $voters @@ -91,6 +92,11 @@ public function vote(TokenInterface $token, $subject, array $attributes): int { return $this->vote; } + + public function getVote(TokenInterface $token, mixed $subject, array $attributes): Vote + { + return new Vote($this->vote); + } }; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index 6bab3d98c5592..fcfeb1c0ea644 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -118,7 +118,8 @@ public function testCacheableVoters() $token = $this->createMock(TokenInterface::class); $voter = $this ->getMockBuilder(CacheableVoterInterface::class) - ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->onlyMethods(['supportsAttribute', 'supportsType', 'vote']) + ->addMethods(['getVote']) ->getMock(); $voter ->expects($this->once()) @@ -152,7 +153,8 @@ public function testCacheableVotersLegacy() $token = $this->createMock(TokenInterface::class); $voter = $this ->getMockBuilder(CacheableVoterInterface::class) - ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->onlyMethods(['supportsAttribute', 'supportsType', 'vote']) + ->addMethods(['getVote']) ->getMock(); $voter ->expects($this->once()) @@ -179,7 +181,8 @@ public function testCacheableVotersIgnoresNonStringAttributes() $token = $this->createMock(TokenInterface::class); $voter = $this ->getMockBuilder(CacheableVoterInterface::class) - ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->onlyMethods(['supportsAttribute', 'supportsType', 'vote']) + ->addMethods(['getVote']) ->getMock(); $voter ->expects($this->never()) @@ -204,7 +207,8 @@ public function testCacheableVotersWithMultipleAttributes() $token = $this->createMock(TokenInterface::class); $voter = $this ->getMockBuilder(CacheableVoterInterface::class) - ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->onlyMethods(['supportsAttribute', 'supportsType', 'vote']) + ->addMethods(['getVote']) ->getMock(); $voter ->expects($this->exactly(2)) @@ -240,7 +244,8 @@ public function testCacheableVotersWithEmptyAttributes() $token = $this->createMock(TokenInterface::class); $voter = $this ->getMockBuilder(CacheableVoterInterface::class) - ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->onlyMethods(['supportsAttribute', 'supportsType', 'vote']) + ->addMethods(['getVote']) ->getMock(); $voter ->expects($this->never()) @@ -265,7 +270,8 @@ public function testCacheableVotersSupportsMethodsCalledOnce() $token = $this->createMock(TokenInterface::class); $voter = $this ->getMockBuilder(CacheableVoterInterface::class) - ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->onlyMethods(['supportsAttribute', 'supportsType', 'vote']) + ->addMethods(['getVote']) ->getMock(); $voter ->expects($this->once()) @@ -293,7 +299,8 @@ public function testCacheableVotersNotCalled() $token = $this->createMock(TokenInterface::class); $voter = $this ->getMockBuilder(CacheableVoterInterface::class) - ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->onlyMethods(['supportsAttribute', 'supportsType', 'vote']) + ->addMethods(['getVote']) ->getMock(); $voter ->expects($this->once()) @@ -316,7 +323,8 @@ public function testCacheableVotersWithMultipleAttributesAndNonString() $token = $this->createMock(TokenInterface::class); $voter = $this ->getMockBuilder(CacheableVoterInterface::class) - ->setMethods(['getVote', 'supportsAttribute', 'supportsType', 'vote']) + ->onlyMethods(['supportsAttribute', 'supportsType', 'vote']) + ->addMethods(['getVote']) ->getMock(); $voter ->expects($this->once()) @@ -343,7 +351,8 @@ private function getExpectedVoter(Vote $vote): VoterInterface { $voter = $this ->getMockBuilder(VoterInterface::class) - ->setMethods(['getVote', 'vote']) + ->onlyMethods(['vote']) + ->addMethods(['getVote']) ->getMock(); $voter->expects($this->once()) ->method('getVote') diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 72ca4c14bdd48..b981d77536aa1 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -11,6 +11,7 @@ namespace Symfony\Component\Security\Core\Tests\Authorization; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\NullToken; @@ -41,7 +42,8 @@ public function testVoteWithoutAuthenticationToken() { $accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $accessDecisionManager ->expects($this->once()) @@ -66,7 +68,7 @@ public function testVoteWithoutAuthenticationTokenLegacy() $this->accessDecisionManager->expects($this->once())->method('decide')->with($this->isInstanceOf(NullToken::class))->willReturn(false); - $this->expectDeprecation('Since symfony/security-core 6.2: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); + $this->expectDeprecation('Since symfony/security-core 6.3: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); $authorizationChecker->isGranted('ROLE_FOO'); } @@ -80,7 +82,8 @@ public function testIsGranted($decide) $accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $accessDecisionManager ->expects($this->once()) @@ -111,7 +114,7 @@ public function testIsGrantedLegacy($decide) ->willReturn($decide); $this->tokenStorage->setToken($token); - $this->expectDeprecation('Since symfony/security-core 6.2: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); + $this->expectDeprecation('Since symfony/security-core 6.3: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); $this->assertSame($decide, $this->authorizationChecker->isGranted('ROLE_FOO')); } @@ -128,7 +131,8 @@ public function testIsGrantedWithObjectAttribute() $accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $accessDecisionManager ->expects($this->once()) @@ -161,7 +165,7 @@ public function testIsGrantedWithObjectAttributeLegacy() ->willReturn(true); $this->tokenStorage->setToken($token); - $this->expectDeprecation('Since symfony/security-core 6.2: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); + $this->expectDeprecation('Since symfony/security-core 6.3: Not implementing "%s::getDecision()" method is deprecated, and would be required in 7.0.'); $this->assertTrue($this->authorizationChecker->isGranted($attribute)); } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php index 4ad3637de4146..df5add59f3231 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -31,7 +31,8 @@ public function testDecideLog(array $expectedLog, array $attributes, $object, ar $token = $this->createMock(TokenInterface::class); $admMock = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $adm = new TraceableAccessDecisionManager($admMock); @@ -183,17 +184,20 @@ public function testAccessDecisionManagerCalledByVoter() { $voter1 = $this ->getMockBuilder(VoterInterface::class) - ->onlyMethods(['getVote', 'vote']) + ->onlyMethods(['vote']) + ->addMethods(['getVote']) ->getMock(); $voter2 = $this ->getMockBuilder(VoterInterface::class) - ->onlyMethods(['getVote', 'vote']) + ->onlyMethods(['vote']) + ->addMethods(['getVote']) ->getMock(); $voter3 = $this ->getMockBuilder(VoterInterface::class) - ->onlyMethods(['getVote', 'vote']) + ->onlyMethods(['vote']) + ->addMethods(['getVote']) ->getMock(); $sut = new TraceableAccessDecisionManager(new AccessDecisionManager([$voter1, $voter2, $voter3])); diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php index 62b989a8e4980..92e73f097dcec 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -65,7 +65,7 @@ public function testVoteLegacy($authenticated, $attributes, $expected) { $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); - $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); + $this->expectDeprecation('Since symfony/security-core 6.3: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $this->assertSame($expected, $voter->vote($this->getToken($authenticated), null, $attributes)); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php index a003392b36060..19e538e595c1e 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php @@ -57,7 +57,7 @@ public function testVoteWithTokenThatReturnsRoleNamesLegacy($roles, $attributes, { $voter = new ExpressionVoter($this->createExpressionLanguage($expressionLanguageExpectsEvaluate), $this->createTrustResolver(), $this->createAuthorizationChecker()); - $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); + $this->expectDeprecation('Since symfony/security-core 6.3: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles, $tokenExpectsGetRoles), null, $attributes)); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php index afdcec55b25a9..08c6018d2bc52 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php @@ -58,7 +58,7 @@ public function testVoteUsingTokenThatReturnsRoleNamesLegacy($roles, $attributes { $voter = new RoleVoter(); - $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); + $this->expectDeprecation('Since symfony/security-core 6.3: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php index 7cc2decd26bbb..c6e6e89a96a82 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php @@ -37,7 +37,8 @@ public function testVote() { $voter = $this ->getMockBuilder(VoterInterface::class) - ->setMethods(['getVote', 'vote']) + ->onlyMethods(['vote']) + ->addMethods(['getVote']) ->getMock(); $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass(); @@ -85,7 +86,7 @@ public function testVoteLegacy() $sut = new TraceableVoter($voter, $eventDispatcher); - $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); + $this->expectDeprecation('Since symfony/security-core 6.3: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $result = $sut->vote($token, 'anysubject', ['attr1']); $this->assertSame(VoterInterface::ACCESS_DENIED, $result); diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php index 78b51ceda32f6..95cf0cc5635dd 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php @@ -45,7 +45,7 @@ public static function getTests(): array [$voter, ['DELETE'], Vote::createAbstain(), new \stdClass(), 'ACCESS_ABSTAIN if no attribute is supported'], - [$voter, ['EDIT'], Vote::createAbstain(), $this, 'ACCESS_ABSTAIN if class is not supported'], + [$voter, ['EDIT'], Vote::createAbstain(), new class() {}, 'ACCESS_ABSTAIN if class is not supported'], [$voter, ['EDIT'], Vote::createAbstain(), null, 'ACCESS_ABSTAIN if object is null'], @@ -103,7 +103,7 @@ public function getTestsLegacy() */ public function testVoteLegacy(VoterInterface $voter, array $attributes, $expectedVote, $object, $message) { - $this->expectDeprecation('Since symfony/security-core 6.2: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); + $this->expectDeprecation('Since symfony/security-core 6.3: Method "%s::vote()" has been deprecated, use "%s::getVote()" instead.'); $this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message); } @@ -132,7 +132,7 @@ public function testGetVoteWithTypeError() class VoterTest_Voter extends Voter { - protected function voteOnAttribute(string $attribute, $object, TokenInterface $token): Vote + protected function voteOnAttribute(string $attribute, mixed $object, TokenInterface $token): Vote { return 'EDIT' === $attribute ? Vote::createGranted() : Vote::createDenied(); } diff --git a/src/Symfony/Component/Security/Core/Tests/Exception/AccessDeniedExceptionTest.php b/src/Symfony/Component/Security/Core/Tests/Exception/AccessDeniedExceptionTest.php index 8c28ccf09abda..e536bd7132648 100644 --- a/src/Symfony/Component/Security/Core/Tests/Exception/AccessDeniedExceptionTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Exception/AccessDeniedExceptionTest.php @@ -37,7 +37,7 @@ public function getAccessDescisions(): \Generator Vote::createDenied('bar'), Vote::createDenied('baz'), ]), - 'Access Denied. foo bar baz', + 'Access Denied. Decision messages are "foo" and "bar" and "baz"', ]; yield [ @@ -51,14 +51,22 @@ public function getAccessDescisions(): \Generator Vote::createDenied('bar'), Vote::createAbstain('baz'), ]), - 'Access Denied. bar', + 'Access Denied. Decision message is "bar"', ]; yield [ AccessDecision::createGranted([ Vote::createDenied('foo'), ]), - 'Access Denied. foo', + 'Access Denied. Decision message is "foo"', + ]; + + yield [ + AccessDecision::createGranted([ + Vote::createDenied(['foo', 'bar']), + Vote::createDenied(['baz', 'qux']), + ]), + 'Access Denied. Decision messages are "foo, bar" and "baz, qux"', ]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Fixtures/DummyVoter.php b/src/Symfony/Component/Security/Core/Tests/Fixtures/DummyVoter.php index 1f923423a21ed..e32769e06b28c 100644 --- a/src/Symfony/Component/Security/Core/Tests/Fixtures/DummyVoter.php +++ b/src/Symfony/Component/Security/Core/Tests/Fixtures/DummyVoter.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Tests\Fixtures; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; final class DummyVoter implements VoterInterface @@ -19,4 +20,8 @@ final class DummyVoter implements VoterInterface public function vote(TokenInterface $token, $subject, array $attributes): int { } + + public function getVote(TokenInterface $token, mixed $subject, array $attributes): Vote + { + } } diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index f00e50c39193d..3a84f3234d6de 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -59,7 +59,8 @@ public function getCredentials(): mixed $accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $accessDecisionManager ->expects($this->once()) @@ -194,7 +195,8 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken() $accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $accessDecisionManager->expects($this->once()) ->method('getDecision') @@ -225,7 +227,8 @@ public function testHandleWhenPublicAccessIsAllowed() $accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $accessDecisionManager->expects($this->once()) ->method('getDecision') @@ -258,7 +261,8 @@ public function testHandleWhenPublicAccessWhileAuthenticated() $accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $accessDecisionManager->expects($this->once()) ->method('getDecision') @@ -294,7 +298,8 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() $accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $accessDecisionManager ->expects($this->once()) diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php index 13c45005fd2e5..abaf79027082c 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php @@ -48,7 +48,8 @@ protected function setUp(): void $this->userChecker = $this->createMock(UserCheckerInterface::class); $this->accessDecisionManager = $this ->getMockBuilder(AccessDecisionManagerInterface::class) - ->setMethods(['getDecision', 'decide']) + ->onlyMethods(['decide']) + ->addMethods(['getDecision']) ->getMock(); $this->request = new Request(); $this->event = new RequestEvent($this->createMock(HttpKernelInterface::class), $this->request, HttpKernelInterface::MAIN_REQUEST); From 068a117ef4f54330b43c7582fed42f6151341668 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Tue, 5 Sep 2023 22:40:13 +0200 Subject: [PATCH 4/4] Fix rebase tests --- src/Symfony/Bundle/FrameworkBundle/composer.json | 2 +- .../Component/Security/Core/Authorization/Voter/Vote.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/composer.json b/src/Symfony/Bundle/FrameworkBundle/composer.json index 145753c33e298..4a5c8d1395cf3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/composer.json +++ b/src/Symfony/Bundle/FrameworkBundle/composer.json @@ -94,7 +94,7 @@ "symfony/property-access": "<5.4", "symfony/serializer": "<6.4", "symfony/security-csrf": "<5.4", - "symfony/security-core": "<5.4", + "symfony/security-core": "<6.4", "symfony/stopwatch": "<5.4", "symfony/translation": "<6.2.8", "symfony/twig-bridge": "<5.4", diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php index 2ee8f69e0e4be..378dbca6f0463 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -99,7 +99,7 @@ public static function createDenied(string|array $messages = [], array $context /** * @param string|string[] $messages */ - public function setMessages(string|array $messages) + public function setMessages(string|array $messages): void { $this->messages = (array) $messages; foreach ($this->messages as $message) { @@ -109,7 +109,7 @@ public function setMessages(string|array $messages) } } - public function addMessage(string $message) + public function addMessage(string $message): void { $this->messages[] = $message; }