From 81f74998995b4ebb5795cdc3cb243279dcb7ea42 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 18:15:29 +0700 Subject: [PATCH 01/35] Add the ability for voter to return decision reason Sometimes we need to know why the voter makes a certain decision. This changes allows us to return a vote object which contains a reason. --- .../Controller/AbstractController.php | 15 +- .../views/Collector/security.html.twig | 9 +- .../Tests/EventListener/VoteListenerTest.php | 26 ++- .../Core/Authorization/AccessDecision.php | 98 +++++++++ .../Authorization/AccessDecisionManager.php | 129 +++++++---- .../AccessDecisionManagerInterface.php | 12 +- .../Authorization/AuthorizationChecker.php | 10 +- .../AuthorizationCheckerInterface.php | 2 + .../TraceableAccessDecisionManager.php | 35 ++- .../Core/Authorization/Voter/AccessTrait.php | 41 ++++ .../Voter/AuthenticatedVoter.php | 20 +- .../Authorization/Voter/ExpressionVoter.php | 6 +- .../Core/Authorization/Voter/RoleVoter.php | 6 +- .../Authorization/Voter/TraceableVoter.php | 10 +- .../Core/Authorization/Voter/Vote.php | 71 ++++++ .../Core/Authorization/Voter/Voter.php | 45 +++- .../Authorization/Voter/VoterInterface.php | 2 +- .../Security/Core/Event/VoteEvent.php | 14 +- .../Core/Exception/AccessDeniedException.php | 27 +++ .../Component/Security/Core/Security.php | 20 +- .../AccessDecisionManagerTest.php | 53 +++-- .../TraceableAccessDecisionManagerTest.php | 146 +++++++------ .../Voter/AuthenticatedVoterTest.php | 54 ++--- .../Voter/ExpressionVoterTest.php | 14 +- .../Voter/RoleHierarchyVoterTest.php | 8 +- .../Authorization/Voter/RoleVoterTest.php | 24 +- .../Voter/TraceableVoterTest.php | 37 +++- .../Tests/Authorization/Voter/VoterTest.php | 15 +- .../Security/Http/Firewall/AccessListener.php | 8 +- .../Http/Firewall/SwitchUserListener.php | 10 +- .../Tests/Firewall/AccessListenerTest.php | 206 +++++++++++++++--- .../Tests/Firewall/SwitchUserListenerTest.php | 75 +++++-- 32 files changed, 969 insertions(+), 279 deletions(-) create mode 100644 src/Symfony/Component/Security/Core/Authorization/AccessDecision.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php create mode 100644 src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index df6b8cb9c3f88..bcb1f6930c769 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php @@ -38,6 +38,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; @@ -237,10 +238,22 @@ protected function isGranted($attribute, $subject = null): bool */ protected function denyAccessUnlessGranted($attribute, $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/Resources/views/Collector/security.html.twig b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig index 54196a8adb81d..d229ae5caaf7f 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig @@ -366,16 +366,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'].reason|default('~') }} {% endfor %} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php index 3406e16503f43..b3dac90639aac 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() ->setMethods(['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..706deca31c0c2 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -0,0 +1,98 @@ + + * + * 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\AccessTrait; +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 +{ + use AccessTrait; + + /** @var Vote[] */ + private $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; + } + + /** + * @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 82f9e0ae827d3..d0696810e44ec 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -12,6 +12,8 @@ namespace Symfony\Component\Security\Core\Authorization; 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; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; @@ -54,13 +56,22 @@ public function __construct(iterable $voters = [], string $strategy = self::STRA $this->allowIfEqualGrantedDeniedDecisions = $allowIfEqualGrantedDeniedDecisions; } + public function getDecision(TokenInterface $token, array $attributes, object $object = null): AccessDecision + { + return $this->{$this->strategy}($token, $attributes, $object); + } + /** * @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array * * {@inheritdoc} + * + * @deprecated since 5.3, use {@see getDecision()} instead. */ - public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/) + public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/): bool { + trigger_deprecation('symfony/security-core', '5.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + $allowMultipleAttributes = 3 < \func_num_args() && func_get_arg(3); // Special case for AccessListener, do not remove the right side of the condition before 6.0 @@ -68,7 +79,7 @@ public function decide(TokenInterface $token, array $attributes, $object = null/ throw new InvalidArgumentException(sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__)); } - return $this->{$this->strategy}($token, $attributes, $object); + return $this->getDecision($token, $attributes, $object)->isGranted(); } /** @@ -77,28 +88,31 @@ public function decide(TokenInterface $token, array $attributes, $object = null/ * If all voters abstained from voting, the decision will be based on the * allowIfAllAbstainDecisions property value (defaults to false). */ - private function decideAffirmative(TokenInterface $token, array $attributes, $object = null): bool + private function decideAffirmative(TokenInterface $token, array $attributes, $object = null): AccessDecision { + $votes = []; $deny = 0; foreach ($this->voters as $voter) { - $result = $voter->vote($token, $object, $attributes); + $vote = $this->vote($voter, $token, $object, $attributes); + if (null === $vote) { + continue; + } - if (VoterInterface::ACCESS_GRANTED === $result) { - return true; + $votes[] = $vote; + if ($vote->isGranted()) { + return AccessDecision::createGranted($votes); } - if (VoterInterface::ACCESS_DENIED === $result) { + if ($vote->isDenied()) { ++$deny; - } elseif (VoterInterface::ACCESS_ABSTAIN !== $result) { - trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class); } } if ($deny > 0) { - return false; + return AccessDecision::createDenied($votes); } - return $this->allowIfAllAbstainDecisions; + return $this->decideIfAllAbstainDecisions($votes); } /** @@ -115,35 +129,41 @@ private function decideAffirmative(TokenInterface $token, array $attributes, $ob * If all voters abstained from voting, the decision will be based on the * allowIfAllAbstainDecisions property value (defaults to false). */ - private function decideConsensus(TokenInterface $token, array $attributes, $object = null): bool + private function decideConsensus(TokenInterface $token, array $attributes, $object = null): AccessDecision { + $votes = []; $grant = 0; $deny = 0; foreach ($this->voters as $voter) { - $result = $voter->vote($token, $object, $attributes); + $vote = $this->vote($voter, $token, $object, $attributes); + if (null === $vote) { + continue; + } - if (VoterInterface::ACCESS_GRANTED === $result) { + $votes[] = $vote; + if ($vote->isGranted()) { ++$grant; - } elseif (VoterInterface::ACCESS_DENIED === $result) { + } elseif ($vote->isDenied()) { ++$deny; - } elseif (VoterInterface::ACCESS_ABSTAIN !== $result) { - trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class); } } if ($grant > $deny) { - return true; + return AccessDecision::createGranted($votes); } if ($deny > $grant) { - return false; + return AccessDecision::createDenied($votes); } if ($grant > 0) { - return $this->allowIfEqualGrantedDeniedDecisions; + return $this->allowIfEqualGrantedDeniedDecisions + ? AccessDecision::createGranted($votes) + : AccessDecision::createDenied($votes) + ; } - return $this->allowIfAllAbstainDecisions; + return $this->decideIfAllAbstainDecisions($votes); } /** @@ -152,31 +172,34 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje * If all voters abstained from voting, the decision will be based on the * allowIfAllAbstainDecisions property value (defaults to false). */ - private function decideUnanimous(TokenInterface $token, array $attributes, $object = null): bool + private function decideUnanimous(TokenInterface $token, array $attributes, $object = null): AccessDecision { + $votes = []; $grant = 0; foreach ($this->voters as $voter) { foreach ($attributes as $attribute) { - $result = $voter->vote($token, $object, [$attribute]); + $vote = $this->vote($voter, $token, $object, [$attribute]); + if (null === $vote) { + continue; + } - if (VoterInterface::ACCESS_DENIED === $result) { - return false; + $votes[] = $vote; + if ($vote->isDenied()) { + return AccessDecision::createDenied($votes); } - if (VoterInterface::ACCESS_GRANTED === $result) { + if ($vote->isGranted()) { ++$grant; - } elseif (VoterInterface::ACCESS_ABSTAIN !== $result) { - trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class); } } } // no deny votes if ($grant > 0) { - return true; + return AccessDecision::createGranted($votes); } - return $this->allowIfAllAbstainDecisions; + return $this->decideIfAllAbstainDecisions($votes); } /** @@ -188,22 +211,50 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje */ private function decidePriority(TokenInterface $token, array $attributes, $object = null) { + $votes = []; foreach ($this->voters as $voter) { - $result = $voter->vote($token, $object, $attributes); - - if (VoterInterface::ACCESS_GRANTED === $result) { - return true; + $vote = $this->vote($voter, $token, $object, $attributes); + if (null === $vote) { + continue; } - if (VoterInterface::ACCESS_DENIED === $result) { - return false; + $votes[] = $vote; + if ($vote->isGranted()) { + return AccessDecision::createGranted($votes); } - if (VoterInterface::ACCESS_ABSTAIN !== $result) { - trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return one of "%s" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".', var_export($result, true), get_debug_type($voter), VoterInterface::class); + if ($vote->isDenied()) { + return AccessDecision::createDenied($votes); } } - return $this->allowIfAllAbstainDecisions; + return $this->decideIfAllAbstainDecisions($votes); + } + + /** + * @param Vote[] $votes + */ + private function decideIfAllAbstainDecisions(array $votes): AccessDecision + { + return $this->allowIfAllAbstainDecisions + ? AccessDecision::createGranted($votes) + : AccessDecision::createDenied($votes) + ; + } + + private function vote(VoterInterface $voter, TokenInterface $token, $subject, array $attributes): ?Vote + { + $result = $voter->vote($token, $subject, $attributes); + if ($result instanceof Vote) { + return $result; + } + + trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return a "%s" object instead.', var_export($result, true), get_debug_type($voter), Vote::class); + + if (\in_array($result, [VoterInterface::ACCESS_ABSTAIN, VoterInterface::ACCESS_DENIED, VoterInterface::ACCESS_GRANTED], true)) { + return Vote::create($result); + } + + return null; } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index b807861109a9e..ce495457fee9e 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -17,16 +17,20 @@ * AccessDecisionManagerInterface makes authorization decisions. * * @author Fabien Potencier + * + * @method AccessDecision getDecision(TokenInterface $token, array $attributes, object $object = null) */ interface AccessDecisionManagerInterface { /** * Decides whether the access is possible or not. * - * @param array $attributes An array of attributes associated with the method being invoked - * @param mixed $object The object to secure + * @param array $attributes An array of attributes associated with the method being invoked + * @param object $object The object to secure + * + * @return bool true if the access is granted, false otherwise * - * @return bool + * @deprecated since 5.3, use {@see getDecision()} instead. */ - public function decide(TokenInterface $token, array $attributes, $object = null); + public function decide(TokenInterface $token, array $attributes, $object = null): bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index 2f26dff0f9f56..1f761535c1169 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -65,7 +65,7 @@ public function __construct(TokenStorageInterface $tokenStorage, /*AccessDecisio * * @throws AuthenticationCredentialsNotFoundException when the token storage has no authentication token and $exceptionOnNoToken is set to true */ - final public function isGranted($attribute, $subject = null): bool + final public function isGranted($attribute, $subject = null): AccessDecision { $token = $this->tokenStorage->getToken(); @@ -86,6 +86,12 @@ final public function isGranted($attribute, $subject = null): bool } } - return $this->accessDecisionManager->decide($token, [$attribute], $subject); + if (!method_exists($this->accessDecisionManager, 'getDecision')) { + trigger_deprecation('symfony/security-core', 5.3, 'Not implementing "%s::getDecision()" method is deprecated, and would be required in 6.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 f60c80b76a7ee..676f5386932f7 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/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index 3b5004edf2fcd..fbec6361a0d2d 100644 --- a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Authorization; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -45,13 +46,36 @@ public function __construct(AccessDecisionManagerInterface $manager) } } + public function getDecision(TokenInterface $token, array $attributes, object $object = null): AccessDecision + { + $currentDecisionLog = [ + 'attributes' => $attributes, + 'object' => $object, + 'voterDetails' => [], + ]; + + $this->currentLog[] = &$currentDecisionLog; + + $result = $this->manager->getDecision($token, $attributes, $object); + + $currentDecisionLog['result'] = $result; + + $this->decisionLog[] = array_pop($this->currentLog); // Using a stack since getDecision can be called by voters + + return $result; + } + /** * {@inheritdoc} * * @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array + * + * @deprecated since 5.3, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/): bool { + trigger_deprecation('symfony/security-core', '5.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + $currentDecisionLog = [ 'attributes' => $attributes, 'object' => $object, @@ -72,11 +96,16 @@ public function decide(TokenInterface $token, array $attributes, $object = null/ /** * 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 int|Vote $vote vote of the voter */ - public function addVoterVote(VoterInterface $voter, array $attributes, int $vote) + public function addVoterVote(VoterInterface $voter, array $attributes, $vote) { + if (!$vote instanceof Vote) { + trigger_deprecation('symfony/security-core', '5.3', 'Passing an int as the third argument to "%s::addVoterVote()" is deprecated, pass an instance of "%s" instead.', __CLASS__, Vote::class); + $vote = Vote::create($vote); + } + $currentLogIndex = \count($this->currentLog) - 1; $this->currentLog[$currentLogIndex]['voterDetails'][] = [ 'voter' => $voter, diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php new file mode 100644 index 0000000000000..c008d7cf0c4d0 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php @@ -0,0 +1,41 @@ + + * + * 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; + +/** + * @author Dany Maillard + */ +trait AccessTrait +{ + /** @var int One of the VoterInterface::ACCESS_* constants */ + protected $access; + + 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; + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php index 577d509cd7826..c4a02c58f4b98 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php @@ -54,10 +54,10 @@ public function __construct(AuthenticationTrustResolverInterface $authentication public function vote(TokenInterface $token, $subject, array $attributes) { 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 @@ -69,17 +69,17 @@ public function vote(TokenInterface $token, $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_ANONYMOUSLY === $attribute @@ -88,7 +88,7 @@ public function vote(TokenInterface $token, $subject, array $attributes) || $this->authenticationTrustResolver->isFullFledged($token))) { trigger_deprecation('symfony/security-core', '5.4', 'The "IS_AUTHENTICATED_ANONYMOUSLY" security attribute is deprecated, use "IS_AUTHENTICATED" or "IS_AUTHENTICATED_FULLY" instead if you want to check if the request is (fully) authenticated.'); - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } // @deprecated $this->authenticationTrustResolver must implement isAuthenticated() in 6.0 @@ -96,21 +96,21 @@ public function vote(TokenInterface $token, $subject, array $attributes) && (method_exists($this->authenticationTrustResolver, 'isAuthenticated') ? $this->authenticationTrustResolver->isAuthenticated($token) : ($token && $token->getUser()))) { - 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_ANONYMOUS === $attribute && $this->authenticationTrustResolver->isAnonymous($token)) { trigger_deprecation('symfony/security-core', '5.4', 'The "IS_ANONYMOUSLY" security attribute is deprecated, anonymous no longer exists in version 6.'); - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } if (self::IS_IMPERSONATOR === $attribute && $token instanceof SwitchUserToken) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php index f02c42460ec37..cfb512c18da09 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php @@ -44,7 +44,7 @@ public function __construct(ExpressionLanguage $expressionLanguage, Authenticati */ public function vote(TokenInterface $token, $subject, array $attributes) { - $result = VoterInterface::ACCESS_ABSTAIN; + $result = Vote::createAbstain(); $variables = null; foreach ($attributes as $attribute) { if (!$attribute instanceof Expression) { @@ -55,9 +55,9 @@ public function vote(TokenInterface $token, $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(); } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php index 661acab20f5cc..4565ad51a1aea 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php @@ -32,7 +32,7 @@ public function __construct(string $prefix = 'ROLE_') */ public function vote(TokenInterface $token, $subject, array $attributes) { - $result = VoterInterface::ACCESS_ABSTAIN; + $result = Vote::createAbstain(); $roles = $this->extractRoles($token); foreach ($attributes as $attribute) { @@ -44,10 +44,10 @@ public function vote(TokenInterface $token, $subject, array $attributes) trigger_deprecation('symfony/security-core', '5.1', 'The ROLE_PREVIOUS_ADMIN role is deprecated and will be removed in version 6.0, use the IS_IMPERSONATOR attribute instead.'); } - $result = VoterInterface::ACCESS_DENIED; + $result = Vote::createDenied(); foreach ($roles as $role) { if ($attribute === $role) { - return VoterInterface::ACCESS_GRANTED; + return Vote::createGranted(); } } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index 48f7651256710..e463710625b37 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -35,9 +35,15 @@ public function __construct(VoterInterface $voter, EventDispatcherInterface $eve public function vote(TokenInterface $token, $subject, array $attributes): int { - $result = $this->voter->vote($token, $subject, $attributes); + $vote = $this->voter->vote($token, $subject, $attributes); + $result = $vote; + if (!$vote instanceof Vote && \in_array($vote, [VoterInterface::ACCESS_DENIED, VoterInterface::ACCESS_GRANTED, VoterInterface::ACCESS_ABSTAIN])) { + trigger_deprecation('symfony/security-core', '5.3', 'Returning an int from "%s::vote" is deprecated, return an instance of "%s" instead.', \get_class($this->voter), Vote::class); - $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $result), 'debug.security.authorization.vote'); + $result = Vote::create($vote); + } + + $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $vote), 'debug.security.authorization.vote'); return $result; } 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..653c879ccddbf --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -0,0 +1,71 @@ + + * + * 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 reason explaining the vote decision. + * + * @author Dany Maillard + */ +final class Vote +{ + use AccessTrait; + + private $reason; + private $parameters; + + /** + * @param int $access One of the VoterInterface::ACCESS_* constants + */ + private function __construct(int $access, string $reason = '', array $parameters = []) + { + $this->access = $access; + $this->reason = $reason; + $this->parameters = $parameters; + } + + public static function create(int $access, string $reason = '', array $parameters = []): self + { + return new self($access, $reason, $parameters); + } + + public static function createGranted(string $reason = '', array $parameters = []): self + { + return new self(VoterInterface::ACCESS_GRANTED, $reason, $parameters); + } + + public static function createAbstain(string $reason = '', array $parameters = []): self + { + return new self(VoterInterface::ACCESS_ABSTAIN, $reason, $parameters); + } + + public static function createDenied(string $reason = '', array $parameters = []): self + { + return new self(VoterInterface::ACCESS_DENIED, $reason, $parameters); + } + + public function setReason(string $reason) + { + $this->reason = $reason; + } + + public function getReason(): string + { + return $this->reason; + } + + public function getParameters(): array + { + return $this->parameters; + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index 7044d9d0f8a86..408c9f29a3cb9 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -18,6 +18,7 @@ * * @author Roman Marintšenko * @author Grégoire Pineau + * @author Dany Maillard */ abstract class Voter implements VoterInterface { @@ -27,7 +28,7 @@ abstract class Voter implements VoterInterface public function vote(TokenInterface $token, $subject, array $attributes) { // abstain vote by default in case none of the attributes are supported - $vote = self::ACCESS_ABSTAIN; + $vote = $this->abstain(); foreach ($attributes as $attribute) { try { @@ -48,24 +49,56 @@ public function vote(TokenInterface $token, $subject, array $attributes) } // as soon as at least one attribute is supported, default is to deny access - $vote = self::ACCESS_DENIED; + $vote = $this->deny(); - if ($this->voteOnAttribute($attribute, $subject, $token)) { + $decision = $this->voteOnAttribute($attribute, $subject, $token); + if (\is_bool($decision)) { + trigger_deprecation('symfony/security-core', '5.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(); + } + + if ($decision->isGranted()) { // grant access as soon as at least one attribute returns a positive response - return self::ACCESS_GRANTED; + return $decision; } + + $vote->setReason($vote->getReason().trim(' '.$decision->getReason())); } return $vote; } + /** + * Creates a granted vote. + */ + public function grant(string $reason = '', array $parameters = []): Vote + { + return Vote::createGranted($reason, $parameters); + } + + /** + * Creates an abstained vote. + */ + public function abstain(string $reason = '', array $parameters = []): Vote + { + return Vote::createAbstain($reason, $parameters); + } + + /** + * Creates a denied vote. + */ + public function deny(string $reason = '', array $parameters = []): Vote + { + return Vote::createDenied($reason, $parameters); + } + /** * Determines if the attribute and subject are supported by this voter. * * @param string $attribute An attribute * @param mixed $subject The subject to secure, e.g. an object the user wants to access or any other PHP type * - * @return bool + * @return bool True if the attribute and subject are supported, false otherwise */ abstract protected function supports(string $attribute, $subject); @@ -75,7 +108,7 @@ abstract protected function supports(string $attribute, $subject); * * @param mixed $subject * - * @return bool + * @return Vote Returning a boolean is deprecated since Symfony 5.1. Return a Vote object instead. */ abstract protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php index a50af88ee63e4..207db96001717 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php @@ -33,7 +33,7 @@ interface VoterInterface * @param mixed $subject The subject to secure * @param array $attributes An array of attributes associated with the method being invoked * - * @return int either ACCESS_GRANTED, ACCESS_ABSTAIN, or ACCESS_DENIED + * @return int|Vote either ACCESS_GRANTED, ACCESS_ABSTAIN, or ACCESS_DENIED */ public function vote(TokenInterface $token, $subject, array $attributes); } diff --git a/src/Symfony/Component/Security/Core/Event/VoteEvent.php b/src/Symfony/Component/Security/Core/Event/VoteEvent.php index 78ac2f900ba48..43e739ab72779 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; @@ -28,11 +29,20 @@ final class VoteEvent extends Event private $attributes; private $vote; - public function __construct(VoterInterface $voter, $subject, array $attributes, int $vote) + /** + * @param Vote|int $vote + */ + public function __construct(VoterInterface $voter, $subject, array $attributes, $vote) { $this->voter = $voter; $this->subject = $subject; $this->attributes = $attributes; + if (!$vote instanceof Vote) { + trigger_deprecation('symfony/security-core', '5.3', 'Passing an int as the fourth argument to "%s::__construct" is deprecated, pass a "%s" instance instead.', __CLASS__, Vote::class); + + $vote = Vote::create($vote); + } + $this->vote = $vote; } @@ -51,7 +61,7 @@ public function getAttributes(): array return $this->attributes; } - public function getVote(): int + public function getVote(): 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 0e59dc4077a91..6f2f73418dee8 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -11,6 +11,9 @@ namespace Symfony\Component\Security\Core\Exception; +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. * @@ -20,6 +23,8 @@ class AccessDeniedException extends RuntimeException { private $attributes = []; private $subject; + /** @var AccessDecision */ + private $accessDecision; public function __construct(string $message = 'Access Denied.', \Throwable $previous = null) { @@ -57,4 +62,26 @@ public function setSubject($subject) { $this->subject = $subject; } + + /** + * Sets an access decision and appends the denied reasons to the exception message. + */ + public function setAccessDecision(AccessDecision $accessDecision) + { + $this->accessDecision = $accessDecision; + if (!$accessDecision->getDeniedVotes()) { + return; + } + + $reasons = array_map(static function (Vote $vote) { + return $vote->getReason(); + }, $accessDecision->getDeniedVotes()); + + $this->message = 'Access Denied: '.rtrim(' '.implode(' ', $reasons)); + } + + 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 0e0d4e5b364e1..0cefa6ca4fcec 100644 --- a/src/Symfony/Component/Security/Core/Security.php +++ b/src/Symfony/Component/Security/Core/Security.php @@ -13,6 +13,7 @@ use Psr\Container\ContainerInterface; 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; @@ -59,12 +60,27 @@ public function getUser(): ?UserInterface */ public function isGranted($attributes, $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. + * + * @param mixed $attributes + * @param mixed $subject + */ + public function getDecision($attribute, $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/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index 375fb6d6d49ef..a94bca0a60822 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -15,6 +15,7 @@ use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class AccessDecisionManagerTest extends TestCase @@ -24,7 +25,7 @@ class AccessDecisionManagerTest extends TestCase public function testSetUnsupportedStrategy() { $this->expectException(\InvalidArgumentException::class); - new AccessDecisionManager([$this->getVoter(VoterInterface::ACCESS_GRANTED)], 'fooBar'); + new AccessDecisionManager([$this->getVoter(Vote::createGranted())], 'fooBar'); } /** @@ -35,6 +36,19 @@ public function testStrategies($strategy, $voters, $allowIfAllAbstainDecisions, $token = $this->createMock(TokenInterface::class); $manager = new AccessDecisionManager($voters, $strategy, $allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions); + $this->assertSame($expected, $manager->getDecision($token, ['ROLE_FOO'])->isGranted()); + } + + /** + * @dataProvider getStrategyTests + * + * @group legacy + */ + public function testStrategiesLegacy($strategy, $voters, $allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions, $expected) + { + $token = $this->createMock(TokenInterface::class); + $manager = new AccessDecisionManager($voters, $strategy, $allowIfAllAbstainDecisions, $allowIfEqualGrantedDeniedDecisions); + $this->assertSame($expected, $manager->decide($token, ['ROLE_FOO'])); } @@ -47,7 +61,8 @@ public function testDeprecatedVoter($strategy) $token = $this->createMock(TokenInterface::class); $manager = new AccessDecisionManager([$this->getVoter(3)], $strategy); - $this->expectDeprecation('Since symfony/security-core 5.3: Returning "3" in "%s::vote()" is deprecated, return one of "Symfony\Component\Security\Core\Authorization\Voter\VoterInterface" constants: "ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN".'); + $this->expectDeprecation('Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.'); + $this->expectDeprecation('Since symfony/security-core 5.3: Returning "3" in "%s::vote()" is deprecated, return an instance of "%s" instead.'); $manager->decide($token, ['ROLE_FOO']); } @@ -87,27 +102,27 @@ public function getStrategyTests() // priority [AccessDecisionManager::STRATEGY_PRIORITY, [ - $this->getVoter(VoterInterface::ACCESS_ABSTAIN), - $this->getVoter(VoterInterface::ACCESS_GRANTED), - $this->getVoter(VoterInterface::ACCESS_DENIED), - $this->getVoter(VoterInterface::ACCESS_DENIED), + $this->getVoter(Vote::createAbstain()), + $this->getVoter(Vote::createGranted()), + $this->getVoter(Vote::createDenied()), + $this->getVoter(Vote::createDenied()), ], true, true, true], [AccessDecisionManager::STRATEGY_PRIORITY, [ - $this->getVoter(VoterInterface::ACCESS_ABSTAIN), - $this->getVoter(VoterInterface::ACCESS_DENIED), - $this->getVoter(VoterInterface::ACCESS_GRANTED), - $this->getVoter(VoterInterface::ACCESS_GRANTED), + $this->getVoter(Vote::createAbstain()), + $this->getVoter(Vote::createDenied()), + $this->getVoter(Vote::createGranted()), + $this->getVoter(Vote::createGranted()), ], true, true, false], [AccessDecisionManager::STRATEGY_PRIORITY, [ - $this->getVoter(VoterInterface::ACCESS_ABSTAIN), - $this->getVoter(VoterInterface::ACCESS_ABSTAIN), + $this->getVoter(Vote::createAbstain()), + $this->getVoter(Vote::createAbstain()), ], false, true, false], [AccessDecisionManager::STRATEGY_PRIORITY, [ - $this->getVoter(VoterInterface::ACCESS_ABSTAIN), - $this->getVoter(VoterInterface::ACCESS_ABSTAIN), + $this->getVoter(Vote::createAbstain()), + $this->getVoter(Vote::createAbstain()), ], true, true, true], ]; } @@ -124,13 +139,13 @@ protected function getVoters($grants, $denies, $abstains) { $voters = []; for ($i = 0; $i < $grants; ++$i) { - $voters[] = $this->getVoter(VoterInterface::ACCESS_GRANTED); + $voters[] = $this->getVoter(Vote::createGranted()); } for ($i = 0; $i < $denies; ++$i) { - $voters[] = $this->getVoter(VoterInterface::ACCESS_DENIED); + $voters[] = $this->getVoter(Vote::createDenied()); } for ($i = 0; $i < $abstains; ++$i) { - $voters[] = $this->getVoter(VoterInterface::ACCESS_ABSTAIN); + $voters[] = $this->getVoter(Vote::createAbstain()); } return $voters; @@ -140,8 +155,8 @@ protected function getVoter($vote) { $voter = $this->createMock(VoterInterface::class); $voter->expects($this->any()) - ->method('vote') - ->willReturn($vote); + ->method('vote') + ->willReturn($vote); return $voter; } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php index d4f89396d3372..4aa5a067e00f4 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\DebugAccessDecisionManager; use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class TraceableAccessDecisionManagerTest extends TestCase @@ -23,20 +25,17 @@ 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, $result) { $token = $this->createMock(TokenInterface::class); - $admMock = $this - ->getMockBuilder(AccessDecisionManager::class) - ->setMethods(['decide']) - ->getMock(); + $admMock = $this->createMock(AccessDecisionManager::class); $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) { foreach ($voterVotes as $voterVote) { @@ -48,9 +47,9 @@ public function testDecideLog(array $expectedLog, array $attributes, $object, ar }) ; - $adm->decide($token, $attributes, $object); + $adm->getDecision($token, $attributes, $object); - $this->assertEquals($expectedLog, $adm->getDecisionLog()); + $this->assertSame($expectedLog, $adm->getDecisionLog()); } public function provideObjectsAndLogs(): \Generator @@ -62,115 +61,121 @@ public 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, + 'object' => null, + '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, + null, [ - [$voter1, VoterInterface::ACCESS_ABSTAIN], - [$voter2, VoterInterface::ACCESS_GRANTED], + [$voter1, $abstain_vote1], + [$voter2, $granted_vote2], ], - false, + $result, ]; + yield [ [[ 'attributes' => [null], - 'object' => 'jolie string', - 'result' => false, + 'object' => null, + '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' => $granted_vote2 = Vote::createGranted()], ], ]], [null], - 'jolie string', + null, [ - [$voter1, VoterInterface::ACCESS_ABSTAIN], - [$voter2, VoterInterface::ACCESS_DENIED], + [$voter1, $abstain_vote1], + [$voter2, $granted_vote2], ], - false, + $result, ]; + yield [ [[ 'attributes' => [12], - 'object' => 12345, - 'result' => true, + 'object' => null, + 'result' => ($result = AccessDecision::createGranted()), 'voterDetails' => [], ]], 'attributes' => [12], - 12345, + null, [], - true, + $result, ]; + yield [ [[ 'attributes' => [new \stdClass()], - 'object' => $x = fopen(__FILE__, 'r'), - 'result' => true, + 'object' => null, + 'result' => ($result = AccessDecision::createGranted()), 'voterDetails' => [], ]], [new \stdClass()], - $x, + null, [], - true, + $result, ]; + yield [ [[ 'attributes' => ['ATTRIBUTE_2'], - 'object' => $x = [], - 'result' => false, + 'object' => null, + '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, + null, [ - [$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, + 'object' => ($x = new \stdClass()), + '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(), + $x, [ - [$voter1, VoterInterface::ACCESS_DENIED], - [$voter2, VoterInterface::ACCESS_DENIED], + [$voter1, $denied_vote1], + [$voter2, $denied_vote2], ], - false, + $result, ]; } @@ -207,7 +212,7 @@ public function testAccessDecisionManagerCalledByVoter() ->expects($this->any()) ->method('vote') ->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; @@ -218,9 +223,9 @@ public function testAccessDecisionManagerCalledByVoter() ->method('vote') ->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); @@ -233,9 +238,16 @@ public function testAccessDecisionManagerCalledByVoter() ->method('vote') ->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; + $decision = $sut->getDecision($token, $attributes); + if ($decision->isGranted()) { + $vote = Vote::createGranted(); + } elseif ($decision->isAbstain()) { + $vote = Vote::createAbstain(); + } else { + $vote = Vote::createDenied(); + } } else { - $vote = VoterInterface::ACCESS_ABSTAIN; + $vote = Vote::createAbstain(); } $sut->addVoterVote($voter3, $attributes, $vote); @@ -244,15 +256,15 @@ 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, ], @@ -260,8 +272,8 @@ public function testAccessDecisionManagerCalledByVoter() '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, ], @@ -269,9 +281,9 @@ public function testAccessDecisionManagerCalledByVoter() '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, ], 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 32633ac68d17a..a1ec7543f8308 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -18,7 +18,7 @@ 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\VoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\User\InMemoryUser; class AuthenticatedVoterTest extends TestCase @@ -36,20 +36,20 @@ public function testVote($authenticated, $attributes, $expected) public function getVoteTests() { return [ - ['fully', [], VoterInterface::ACCESS_ABSTAIN], - ['fully', ['FOO'], VoterInterface::ACCESS_ABSTAIN], - ['remembered', [], VoterInterface::ACCESS_ABSTAIN], - ['remembered', ['FOO'], VoterInterface::ACCESS_ABSTAIN], + ['fully', [], Vote::createAbstain()], + ['fully', ['FOO'], Vote::createAbstain()], + ['remembered', [], Vote::createAbstain()], + ['remembered', ['FOO'], Vote::createAbstain()], - ['fully', ['IS_AUTHENTICATED_REMEMBERED'], VoterInterface::ACCESS_GRANTED], - ['remembered', ['IS_AUTHENTICATED_REMEMBERED'], VoterInterface::ACCESS_GRANTED], + ['fully', ['IS_AUTHENTICATED_REMEMBERED'], Vote::createGranted()], + ['remembered', ['IS_AUTHENTICATED_REMEMBERED'], Vote::createGranted()], - ['fully', ['IS_AUTHENTICATED_FULLY'], VoterInterface::ACCESS_GRANTED], - ['remembered', ['IS_AUTHENTICATED_FULLY'], VoterInterface::ACCESS_DENIED], + ['fully', ['IS_AUTHENTICATED_FULLY'], Vote::createGranted()], + ['remembered', ['IS_AUTHENTICATED_FULLY'], Vote::createDenied()], - ['fully', ['IS_IMPERSONATOR'], VoterInterface::ACCESS_DENIED], - ['remembered', ['IS_IMPERSONATOR'], VoterInterface::ACCESS_DENIED], - ['impersonated', ['IS_IMPERSONATOR'], VoterInterface::ACCESS_GRANTED], + ['fully', ['IS_IMPERSONATOR'], Vote::createDenied()], + ['remembered', ['IS_IMPERSONATOR'], Vote::createDenied()], + ['impersonated', ['IS_IMPERSONATOR'], Vote::createGranted()], ]; } @@ -65,21 +65,21 @@ public function testLegacyVote($authenticated, $attributes, $expected) public function getLegacyVoteTests() { return [ - ['anonymously', [], VoterInterface::ACCESS_ABSTAIN], - ['anonymously', ['FOO'], VoterInterface::ACCESS_ABSTAIN], - ['anonymously', ['IS_AUTHENTICATED_ANONYMOUSLY'], VoterInterface::ACCESS_GRANTED], - ['anonymously', ['IS_AUTHENTICATED_REMEMBERED'], VoterInterface::ACCESS_DENIED], - ['anonymously', ['IS_AUTHENTICATED_FULLY'], VoterInterface::ACCESS_DENIED], - ['anonymously', ['IS_ANONYMOUS'], VoterInterface::ACCESS_GRANTED], - ['anonymously', ['IS_IMPERSONATOR'], VoterInterface::ACCESS_DENIED], - - ['fully', ['IS_ANONYMOUS'], VoterInterface::ACCESS_DENIED], - ['remembered', ['IS_ANONYMOUS'], VoterInterface::ACCESS_DENIED], - ['anonymously', ['IS_ANONYMOUS'], VoterInterface::ACCESS_GRANTED], - - ['fully', ['IS_AUTHENTICATED_ANONYMOUSLY'], VoterInterface::ACCESS_GRANTED], - ['remembered', ['IS_AUTHENTICATED_ANONYMOUSLY'], VoterInterface::ACCESS_GRANTED], - ['anonymously', ['IS_AUTHENTICATED_ANONYMOUSLY'], VoterInterface::ACCESS_GRANTED], + ['anonymously', [], Vote::createAbstain()], + ['anonymously', ['FOO'], Vote::createAbstain()], + ['anonymously', ['IS_AUTHENTICATED_ANONYMOUSLY'], Vote::createGranted()], + ['anonymously', ['IS_AUTHENTICATED_REMEMBERED'], Vote::createDenied()], + ['anonymously', ['IS_AUTHENTICATED_FULLY'], Vote::createDenied()], + ['anonymously', ['IS_ANONYMOUS'], Vote::createGranted()], + ['anonymously', ['IS_IMPERSONATOR'], Vote::createDenied()], + + ['fully', ['IS_ANONYMOUS'], Vote::createDenied()], + ['remembered', ['IS_ANONYMOUS'], Vote::createDenied()], + ['anonymously', ['IS_ANONYMOUS'], Vote::createGranted()], + + ['fully', ['IS_AUTHENTICATED_ANONYMOUSLY'], Vote::createGranted()], + ['remembered', ['IS_AUTHENTICATED_ANONYMOUSLY'], Vote::createGranted()], + ['anonymously', ['IS_AUTHENTICATED_ANONYMOUSLY'], Vote::createGranted()], ]; } 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 a3e516950bcc5..3baa3ce3f3a4f 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/ExpressionVoterTest.php @@ -18,7 +18,7 @@ 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\VoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; class ExpressionVoterTest extends TestCase { @@ -29,19 +29,19 @@ public function testVoteWithTokenThatReturnsRoleNames($roles, $attributes, $expe { $voter = new ExpressionVoter($this->createExpressionLanguage($expressionLanguageExpectsEvaluate), $this->createTrustResolver(), $this->createAuthorizationChecker()); - $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles, $tokenExpectsGetRoles), null, $attributes)); + $this->assertEquals($expected, $voter->vote($this->getTokenWithRoleNames($roles, $tokenExpectsGetRoles), null, $attributes)); } public function getVoteTests() { return [ - [[], [], VoterInterface::ACCESS_ABSTAIN, false, false], - [[], ['FOO'], VoterInterface::ACCESS_ABSTAIN, false, false], + [[], [], Vote::createAbstain(), false, false], + [[], ['FOO'], Vote::createAbstain(), false, false], - [[], [$this->createExpression()], VoterInterface::ACCESS_DENIED, true, false], + [[], [$this->createExpression()], Vote::createDenied(), true, false], - [['ROLE_FOO'], [$this->createExpression(), $this->createExpression()], VoterInterface::ACCESS_GRANTED], - [['ROLE_BAR', 'ROLE_FOO'], [$this->createExpression()], VoterInterface::ACCESS_GRANTED], + [['ROLE_FOO'], [$this->createExpression(), $this->createExpression()], Vote::createGranted()], + [['ROLE_BAR', 'ROLE_FOO'], [$this->createExpression()], Vote::createGranted()], ]; } 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 571270072706b..bffa6199e71f9 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleHierarchyVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleHierarchyVoterTest.php @@ -12,7 +12,7 @@ namespace Symfony\Component\Security\Core\Tests\Authorization\Voter; use Symfony\Component\Security\Core\Authorization\Voter\RoleHierarchyVoter; -use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Role\RoleHierarchy; class RoleHierarchyVoterTest extends RoleVoterTest @@ -24,13 +24,13 @@ public function testVoteUsingTokenThatReturnsRoleNames($roles, $attributes, $exp { $voter = new RoleHierarchyVoter(new RoleHierarchy(['ROLE_FOO' => ['ROLE_FOOBAR']])); - $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)); + $this->assertEquals($expected->getAccess(), $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)->getAccess()); } public function getVoteTests() { return array_merge(parent::getVoteTests(), [ - [['ROLE_FOO'], ['ROLE_FOOBAR'], VoterInterface::ACCESS_GRANTED], + [['ROLE_FOO'], ['ROLE_FOOBAR'], Vote::createGranted()], ]); } @@ -41,7 +41,7 @@ public function testVoteWithEmptyHierarchyUsingTokenThatReturnsRoleNames($roles, { $voter = new RoleHierarchyVoter(new RoleHierarchy([])); - $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)); + $this->assertSame($expected->getAccess(), $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)->getAccess()); } public 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 43f802481e413..eb3268eafeda7 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php @@ -15,7 +15,7 @@ use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authorization\Voter\RoleVoter; -use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; class RoleVoterTest extends TestCase { @@ -28,22 +28,22 @@ public function testVoteUsingTokenThatReturnsRoleNames($roles, $attributes, $exp { $voter = new RoleVoter(); - $this->assertSame($expected, $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)); + $this->assertSame($expected->getAccess(), $voter->vote($this->getTokenWithRoleNames($roles), null, $attributes)->getAccess()); } public function getVoteTests() { return [ - [[], [], VoterInterface::ACCESS_ABSTAIN], - [[], ['FOO'], VoterInterface::ACCESS_ABSTAIN], - [[], ['ROLE_FOO'], VoterInterface::ACCESS_DENIED], - [['ROLE_FOO'], ['ROLE_FOO'], VoterInterface::ACCESS_GRANTED], - [['ROLE_FOO'], ['FOO', 'ROLE_FOO'], VoterInterface::ACCESS_GRANTED], - [['ROLE_BAR', 'ROLE_FOO'], ['ROLE_FOO'], VoterInterface::ACCESS_GRANTED], + [[], [], 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 - [[], [[]], VoterInterface::ACCESS_ABSTAIN], - [[], [new \stdClass()], VoterInterface::ACCESS_ABSTAIN], + [[], [[]], Vote::createAbstain()], + [[], [new \stdClass()], Vote::createAbstain()], ]; } @@ -62,8 +62,8 @@ protected function getTokenWithRoleNames(array $roles) { $token = $this->createMock(AbstractToken::class); $token->expects($this->once()) - ->method('getRoleNames') - ->willReturn($roles); + ->method('getRoleNames') + ->willReturn($roles); return $token; } 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 53e6a277453f5..d28d6542bd07a 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php @@ -12,14 +12,18 @@ 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\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(); @@ -35,6 +39,33 @@ public function testVote() $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass(); $token = $this->getMockBuilder(TokenInterface::class)->getMockForAbstractClass(); + $voter + ->expects($this->once()) + ->method('vote') + ->with($token, 'anysubject', ['attr1']) + ->willReturn($vote = Vote::createDenied()); + + $eventDispatcher + ->expects($this->once()) + ->method('dispatch') + ->with(new VoteEvent($voter, 'anysubject', ['attr1'], $vote), 'debug.security.authorization.vote'); + + $sut = new TraceableVoter($voter, $eventDispatcher); + $result = $sut->vote($token, 'anysubject', ['attr1']); + + $this->assertSame($vote, $result); + } + + /** + * @group legacy + */ + public function testVoteLegacy() + { + $voter = $this->getMockBuilder(VoterInterface::class)->getMockForAbstractClass(); + + $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass(); + $token = $this->getMockBuilder(TokenInterface::class)->getMockForAbstractClass(); + $voter ->expects($this->once()) ->method('vote') @@ -47,8 +78,12 @@ public function testVote() ->with(new VoteEvent($voter, 'anysubject', ['attr1'], VoterInterface::ACCESS_DENIED), 'debug.security.authorization.vote'); $sut = new TraceableVoter($voter, $eventDispatcher); + + $this->expectDeprecation(sprintf('Since symfony/security-core 5.3: Returning an int from "%s::vote" is deprecated, return an instance of "%s" instead.', \get_class($voter), Vote::class)); + $result = $sut->vote($token, 'anysubject', ['attr1']); - $this->assertSame(VoterInterface::ACCESS_DENIED, $result); + $this->assertInstanceOf(Vote::class, $result); + $this->assertSame(VoterInterface::ACCESS_DENIED, $result->getAccess()); } } 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 4b729d6a017df..eca855ac52acd 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/VoterTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\TestCase; 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; @@ -60,7 +61,7 @@ public function getTests() */ public function testVote(VoterInterface $voter, array $attributes, $expectedVote, $object, $message) { - $this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes), $message); + $this->assertEquals($expectedVote, $voter->vote($this->token, $object, $attributes)->getAccess(), $message); } public function testVoteWithTypeError() @@ -74,9 +75,9 @@ public function testVoteWithTypeError() class VoterTest_Voter extends Voter { - protected function voteOnAttribute(string $attribute, $object, TokenInterface $token): bool + protected function voteOnAttribute(string $attribute, $object, TokenInterface $token): Vote { - return 'EDIT' === $attribute; + return 'EDIT' === $attribute ? Vote::createGranted() : Vote::createDenied(); } protected function supports(string $attribute, $object): bool @@ -87,9 +88,9 @@ protected function supports(string $attribute, $object): bool class IntegerVoterTest_Voter extends Voter { - protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool + protected function voteOnAttribute($attribute, $object, TokenInterface $token): Vote { - return 42 === $attribute; + return 42 === $attribute ? Vote::createGranted() : Vote::createDenied(); } protected function supports($attribute, $object): bool @@ -100,9 +101,9 @@ protected function supports($attribute, $object): bool class TypeErrorVoterTest_Voter extends Voter { - protected function voteOnAttribute($attribute, $object, TokenInterface $token): bool + protected function voteOnAttribute($attribute, $object, TokenInterface $token): Vote { - return false; + return Vote::createDenied(); } protected function supports($attribute, $object): bool diff --git a/src/Symfony/Component/Security/Http/Firewall/AccessListener.php b/src/Symfony/Component/Security/Http/Firewall/AccessListener.php index 1ea6ee1563d7a..ee0eede3468d6 100644 --- a/src/Symfony/Component/Security/Http/Firewall/AccessListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/AccessListener.php @@ -122,7 +122,13 @@ public function authenticate(RequestEvent $event) } } - if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) { + if (method_exists($this->accessDecisionManager, 'getDecision')) { + $denied = $this->accessDecisionManager->getDecision($token, $attributes, $request)->isDenied(); + } else { + $denied = !$this->accessDecisionManager->decide($token, $attributes, $request); + } + + if ($denied) { throw $this->createAccessDeniedException($request, $attributes); } } diff --git a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php index 250d9ef215aee..e64815799bf1e 100644 --- a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php @@ -18,6 +18,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; @@ -174,9 +175,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 4a13a4b7f1933..5ac8a89f32007 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -16,12 +16,13 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; -use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authentication\Token\NullToken; 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\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +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\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Exception\AccessDeniedException; @@ -46,19 +47,60 @@ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() ->willReturn([['foo' => 'bar'], null]) ; - $token = new class() extends AbstractToken { - public function isAuthenticated(): bool - { - return true; - } + $token = $this->createMock(TokenInterface::class); + $token + ->expects($this->any()) + ->method('isAuthenticated') + ->willReturn(true) + ; + + $tokenStorage = $this->createMock(TokenStorageInterface::class); + $tokenStorage + ->expects($this->any()) + ->method('getToken') + ->willReturn($token) + ; + + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $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, + $this->createMock(AuthenticationManagerInterface::class) + ); + + $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); + } + + /** + * @group legacy + */ + public function testLegacyHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() + { + $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]) + ; - /** - * @return mixed - */ - public function getCredentials() - { - } - }; + $token = $this->createMock(TokenInterface::class); + $token + ->expects($this->any()) + ->method('isAuthenticated') + ->willReturn(true) + ; $tokenStorage = $this->createMock(TokenStorageInterface::class); $tokenStorage @@ -79,7 +121,71 @@ public function getCredentials() $tokenStorage, $accessDecisionManager, $accessMap, - false + $this->createMock(AuthenticationManagerInterface::class) + ); + + $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); + } + + public function testHandleWhenTheTokenIsNotAuthenticated() + { + $request = new Request(); + + $accessMap = $this->createMock(AccessMapInterface::class); + $accessMap + ->expects($this->any()) + ->method('getPatterns') + ->with($this->equalTo($request)) + ->willReturn([['foo' => 'bar'], null]) + ; + + $notAuthenticatedToken = $this->createMock(TokenInterface::class); + $notAuthenticatedToken + ->expects($this->any()) + ->method('isAuthenticated') + ->willReturn(false) + ; + + $authenticatedToken = $this->createMock(TokenInterface::class); + $authenticatedToken + ->expects($this->any()) + ->method('isAuthenticated') + ->willReturn(true) + ; + + $authManager = $this->createMock(AuthenticationManagerInterface::class); + $authManager + ->expects($this->once()) + ->method('authenticate') + ->with($this->equalTo($notAuthenticatedToken)) + ->willReturn($authenticatedToken) + ; + + $tokenStorage = $this->createMock(TokenStorageInterface::class); + $tokenStorage + ->expects($this->any()) + ->method('getToken') + ->willReturn($notAuthenticatedToken) + ; + $tokenStorage + ->expects($this->once()) + ->method('setToken') + ->with($this->equalTo($authenticatedToken)) + ; + + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager + ->expects($this->once()) + ->method('getDecision') + ->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar']), $this->equalTo($request)) + ->willReturn(AccessDecision::createGranted()) + ; + + $listener = new AccessListener( + $tokenStorage, + $accessDecisionManager, + $accessMap, + $authManager ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -88,7 +194,7 @@ public function getCredentials() /** * @group legacy */ - public function testHandleWhenTheTokenIsNotAuthenticated() + public function testLegacyHandleWhenTheTokenIsNotAuthenticated() { $request = new Request(); @@ -146,8 +252,7 @@ public function testHandleWhenTheTokenIsNotAuthenticated() $tokenStorage, $accessDecisionManager, $accessMap, - $authManager, - false + $authManager ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -184,7 +289,7 @@ public function testHandleWhenThereIsNoAccessMapEntryMatchingTheRequest() $tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, - false + $this->createMock(AuthenticationManagerInterface::class) ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -212,7 +317,7 @@ public function testHandleWhenAccessMapReturnsEmptyAttributes() $tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, - false + $this->createMock(AuthenticationManagerInterface::class) ); $event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST); @@ -220,10 +325,7 @@ public function testHandleWhenAccessMapReturnsEmptyAttributes() $listener(new LazyResponseEvent($event)); } - /** - * @group legacy - */ - public function testLegacyHandleWhenTheSecurityTokenStorageHasNoToken() + public function testHandleWhenTheSecurityTokenStorageHasNoToken() { $this->expectException(AuthenticationCredentialsNotFoundException::class); $tokenStorage = $this->createMock(TokenStorageInterface::class); @@ -253,7 +355,40 @@ public function testLegacyHandleWhenTheSecurityTokenStorageHasNoToken() $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); } - public function testHandleWhenTheSecurityTokenStorageHasNoToken() + public function testHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptionOnTokenIsFalse() + { + $this->expectException(AccessDeniedException::class); + $tokenStorage = new TokenStorage(); + $request = new Request(); + + $accessMap = $this->createMock(AccessMapInterface::class); + $accessMap->expects($this->any()) + ->method('getPatterns') + ->with($this->equalTo($request)) + ->willReturn([['foo' => 'bar'], null]) + ; + + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager->expects($this->once()) + ->method('getDecision') + ->with($this->isInstanceOf(NullToken::class)) + ->willReturn(AccessDecision::createDenied()); + + $listener = new AccessListener( + $tokenStorage, + $accessDecisionManager, + $accessMap, + $this->createMock(AuthenticationManagerInterface::class), + false + ); + + $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); + } + + /** + * @group legacy + */ + public function testLegacyHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptionOnTokenIsFalse() { $this->expectException(AccessDeniedException::class); $tokenStorage = new TokenStorage(); @@ -276,13 +411,14 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken() $tokenStorage, $accessDecisionManager, $accessMap, + $this->createMock(AuthenticationManagerInterface::class), false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); } - public function testHandleWhenPublicAccessIsAllowed() + public function testHandleWhenPublicAccessIsAllowedAndExceptionOnTokenIsFalse() { $tokenStorage = new TokenStorage(); $request = new Request(); @@ -296,14 +432,15 @@ public function testHandleWhenPublicAccessIsAllowed() $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $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, $accessDecisionManager, $accessMap, + $this->createMock(AuthenticationManagerInterface::class), false ); @@ -312,7 +449,7 @@ public function testHandleWhenPublicAccessIsAllowed() public function testHandleWhenPublicAccessWhileAuthenticated() { - $token = new UsernamePasswordToken(new InMemoryUser('Wouter', null, ['ROLE_USER']), 'main', ['ROLE_USER']); + $token = new UsernamePasswordToken(new InMemoryUser('Wouter', null, ['ROLE_USER']), null, 'main', ['ROLE_USER']); $tokenStorage = new TokenStorage(); $tokenStorage->setToken($token); $request = new Request(); @@ -326,14 +463,15 @@ public function testHandleWhenPublicAccessWhileAuthenticated() $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $accessDecisionManager->expects($this->once()) - ->method('decide') + ->method('getDecision') ->with($this->equalTo($token), [AuthenticatedVoter::PUBLIC_ACCESS]) - ->willReturn(true); + ->willReturn(AccessDecision::createGranted()); $listener = new AccessListener( $tokenStorage, $accessDecisionManager, $accessMap, + $this->createMock(AuthenticationManagerInterface::class), false ); @@ -352,7 +490,7 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() ->willReturn([['foo' => 'bar', 'bar' => 'baz'], null]) ; - $authenticatedToken = new UsernamePasswordToken(new InMemoryUser('test', 'test', ['ROLE_USER']), 'test', ['ROLE_USER']); + $authenticatedToken = new UsernamePasswordToken('test', 'test', 'test', ['ROLE_USER']); $tokenStorage = new TokenStorage(); $tokenStorage->setToken($authenticatedToken); @@ -360,16 +498,16 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $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( $tokenStorage, $accessDecisionManager, $accessMap, - false + $this->createMock(AuthenticationManagerInterface::class) ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php index 0338af0017e8a..db61ea38e212f 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php @@ -19,11 +19,14 @@ 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\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\InMemoryUserProvider; +use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Http\Event\SwitchUserEvent; use Symfony\Component\Security\Http\Firewall\SwitchUserListener; @@ -166,14 +169,13 @@ public function testSwitchUserIsDisallowed() { $this->expectException(AccessDeniedException::class); $token = new UsernamePasswordToken(new InMemoryUser('username', '', ['ROLE_FOO']), 'key', ['ROLE_FOO']); - $user = new InMemoryUser('username', 'password', []); $this->tokenStorage->setToken($token); $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); @@ -188,7 +190,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); @@ -202,8 +204,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(function ($user) { return 'kuba' === $user->getUserIdentifier(); })) - ->willReturn(true); + ->method('getDecision')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(function ($user) { return 'kuba' === $user->getUserIdentifier(); })) + ->willReturn(AccessDecision::createGranted()); $this->userChecker->expects($this->once()) ->method('checkPostAuth')->with($this->callback(function ($user) { return 'kuba' === $user->getUserIdentifier(); })); @@ -228,8 +230,8 @@ public function testSwitchUserAlreadySwitched() $targetsUser = $this->callback(function ($user) { return '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); @@ -254,8 +256,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(function ($argUser) use ($user) { return $user->isEqualTo($argUser); })); @@ -281,8 +283,8 @@ public function testSwitchUserKeepsOtherQueryStringParameters() $targetsUser = $this->callback(function ($user) { return '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); @@ -306,6 +308,47 @@ public function testSwitchUserWithReplacedToken() $this->request->query->set('_switch_user', 'kuba'); $this->accessDecisionManager->expects($this->any()) + ->method('getDecision')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(function ($user) { return 'kuba' === $user->getUserIdentifier(); })) + ->willReturn(AccessDecision::createGranted()); + + $dispatcher = $this->createMock(EventDispatcherInterface::class); + $dispatcher + ->expects($this->once()) + ->method('dispatch') + ->with( + $this->callback(function (SwitchUserEvent $event) use ($replacedToken, $user) { + if ('kuba' !== $event->getTargetUser()->getUserIdentifier()) { + return false; + } + $event->setToken($replacedToken); + + return true; + }), + SecurityEvents::SWITCH_USER + ); + + $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager, null, '_switch_user', 'ROLE_ALLOWED_TO_SWITCH', $dispatcher); + $listener($this->event); + + $this->assertSame($replacedToken, $this->tokenStorage->getToken()); + } + + /** + * @group legacy + */ + public function testLegacySwitchUserWithReplacedToken() + { + $user = new InMemoryUser('username', 'password', []); + $token = new UsernamePasswordToken($user, '', 'provider123', ['ROLE_FOO']); + + $user = new InMemoryUser('replaced', 'password', []); + $replacedToken = new UsernamePasswordToken($user, '', 'provider123', ['ROLE_BAR']); + + $this->tokenStorage->setToken($token); + $this->request->query->set('_switch_user', 'kuba'); + + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager->expects($this->any()) ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(function ($user) { return 'kuba' === $user->getUserIdentifier(); })) ->willReturn(true); @@ -314,7 +357,7 @@ public function testSwitchUserWithReplacedToken() ->expects($this->once()) ->method('dispatch') ->with( - $this->callback(function (SwitchUserEvent $event) use ($replacedToken) { + $this->callback(function (SwitchUserEvent $event) use ($replacedToken, $user) { if ('kuba' !== $event->getTargetUser()->getUserIdentifier()) { return false; } @@ -325,7 +368,7 @@ public function testSwitchUserWithReplacedToken() SecurityEvents::SWITCH_USER ); - $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager, null, '_switch_user', 'ROLE_ALLOWED_TO_SWITCH', $dispatcher); + $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $accessDecisionManager, null, '_switch_user', 'ROLE_ALLOWED_TO_SWITCH', $dispatcher); $listener($this->event); $this->assertSame($replacedToken, $this->tokenStorage->getToken()); @@ -349,8 +392,8 @@ public function testSwitchUserStateless() $targetsUser = $this->callback(function ($user) { return '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 85a9b3d3903c984f6949d4d43f28e2ae259c6585 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 19:05:16 +0700 Subject: [PATCH 02/35] Create mock with DecisionManager class We cannot use the interface at the moment because the new method is not available in the interface yet. We also cannot put the new method inside the interface because it can break current/old code. --- .../AuthorizationCheckerTest.php | 2 +- .../Tests/Firewall/AccessListenerTest.php | 26 +++++++++---------- .../Tests/Firewall/SwitchUserListenerTest.php | 4 +-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 160b921b3075c..6e9d3f56556cb 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -30,7 +30,7 @@ class AuthorizationCheckerTest extends TestCase protected function setUp(): void { - $this->accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $this->accessDecisionManager = $this->createMock(AccessDecisionManager::class); $this->tokenStorage = new TokenStorage(); $this->authorizationChecker = new AuthorizationChecker( diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index 5ac8a89f32007..252cad9ed23a5 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -109,7 +109,7 @@ public function testLegacyHandleWhenTheAccessDecisionManagerDecidesToRefuseAcces ->willReturn($token) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager ->expects($this->once()) ->method('decide') @@ -173,7 +173,7 @@ public function testHandleWhenTheTokenIsNotAuthenticated() ->with($this->equalTo($authenticatedToken)) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager ->expects($this->once()) ->method('getDecision') @@ -240,7 +240,7 @@ public function testLegacyHandleWhenTheTokenIsNotAuthenticated() ->with($this->equalTo($authenticatedToken)) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager ->expects($this->once()) ->method('decide') @@ -287,7 +287,7 @@ public function testHandleWhenThereIsNoAccessMapEntryMatchingTheRequest() $listener = new AccessListener( $tokenStorage, - $this->createMock(AccessDecisionManagerInterface::class), + $this->createMock(AccessDecisionManager::class), $accessMap, $this->createMock(AuthenticationManagerInterface::class) ); @@ -315,7 +315,7 @@ public function testHandleWhenAccessMapReturnsEmptyAttributes() $listener = new AccessListener( $tokenStorage, - $this->createMock(AccessDecisionManagerInterface::class), + $this->createMock(AccessDecisionManager::class), $accessMap, $this->createMock(AuthenticationManagerInterface::class) ); @@ -347,7 +347,7 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken() $listener = new AccessListener( $tokenStorage, - $this->createMock(AccessDecisionManagerInterface::class), + $this->createMock(AccessDecisionManager::class), $accessMap, $this->createMock(AuthenticationManagerInterface::class) ); @@ -368,7 +368,7 @@ public function testHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptionOnTok ->willReturn([['foo' => 'bar'], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager->expects($this->once()) ->method('getDecision') ->with($this->isInstanceOf(NullToken::class)) @@ -401,7 +401,7 @@ public function testLegacyHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptio ->willReturn([['foo' => 'bar'], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager->expects($this->once()) ->method('decide') ->with($this->isInstanceOf(NullToken::class)) @@ -430,7 +430,7 @@ public function testHandleWhenPublicAccessIsAllowedAndExceptionOnTokenIsFalse() ->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager->expects($this->once()) ->method('getDecision') ->with($this->isInstanceOf(NullToken::class), [AuthenticatedVoter::PUBLIC_ACCESS]) @@ -461,7 +461,7 @@ public function testHandleWhenPublicAccessWhileAuthenticated() ->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager->expects($this->once()) ->method('getDecision') ->with($this->equalTo($token), [AuthenticatedVoter::PUBLIC_ACCESS]) @@ -495,7 +495,7 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() $tokenStorage = new TokenStorage(); $tokenStorage->setToken($authenticatedToken); - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager ->expects($this->once()) ->method('getDecision') @@ -526,7 +526,7 @@ public function testLazyPublicPagesShouldNotAccessTokenStorage() ->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null]) ; - $listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, false); + $listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManager::class), $accessMap, false); $listener(new LazyResponseEvent(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST))); } @@ -546,7 +546,7 @@ public function testLegacyLazyPublicPagesShouldNotAccessTokenStorage() ->willReturn([[AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY], null]) ; - $listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, false); + $listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManager::class), $accessMap, false); $listener(new LazyResponseEvent(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST))); } } diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php index db61ea38e212f..47203fcfe300c 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php @@ -52,7 +52,7 @@ 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->createMock(AccessDecisionManager::class); $this->request = new Request(); $this->event = new RequestEvent($this->createMock(HttpKernelInterface::class), $this->request, HttpKernelInterface::MAIN_REQUEST); } @@ -347,7 +347,7 @@ public function testLegacySwitchUserWithReplacedToken() $this->tokenStorage->setToken($token); $this->request->query->set('_switch_user', 'kuba'); - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager->expects($this->any()) ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(function ($user) { return 'kuba' === $user->getUserIdentifier(); })) ->willReturn(true); From 293a0570d8cb7f4ae5a89f0a35d76ba13076b24b Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 19:16:52 +0700 Subject: [PATCH 03/35] Fix phpunit test errors 1. Add missing getDecision method to AuthorizationChecker 2. Do not force int return on TraceableVoter::vote for now --- .../Authorization/AuthorizationChecker.php | 20 ++++++++----------- .../Authorization/Voter/TraceableVoter.php | 2 +- 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index 1f761535c1169..88eb219834a8a 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -65,25 +65,21 @@ public function __construct(TokenStorageInterface $tokenStorage, /*AccessDecisio * * @throws AuthenticationCredentialsNotFoundException when the token storage has no authentication token and $exceptionOnNoToken is set to true */ - final public function isGranted($attribute, $subject = null): AccessDecision + final public function isGranted($attribute, $subject = null): bool { - $token = $this->tokenStorage->getToken(); + return $this->getDecision($attribute, $subject)->isGranted(); + } - if (!$token || !$token->getUser()) { + public function getDecision($attribute, $subject = null): AccessDecision + { + if (null === ($token = $this->tokenStorage->getToken())) { if ($this->exceptionOnNoToken) { throw new AuthenticationCredentialsNotFoundException('The token storage contains no authentication token. One possible reason may be that there is no firewall configured for this URL.'); } $token = new NullToken(); - } else { - $authenticated = true; - // @deprecated since Symfony 5.4 - if ($this->alwaysAuthenticate || !$authenticated = $token->isAuthenticated(false)) { - if (!($authenticated ?? true)) { - trigger_deprecation('symfony/core', '5.4', 'Returning false from "%s()" is deprecated, return null from "getUser()" instead.'); - } - $this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token)); - } + } elseif ($this->alwaysAuthenticate || !$token->isAuthenticated()) { + $this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token)); } if (!method_exists($this->accessDecisionManager, 'getDecision')) { diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index e463710625b37..119f9431bfdb4 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -33,7 +33,7 @@ public function __construct(VoterInterface $voter, EventDispatcherInterface $eve $this->eventDispatcher = $eventDispatcher; } - public function vote(TokenInterface $token, $subject, array $attributes): int + public function vote(TokenInterface $token, $subject, array $attributes) { $vote = $this->voter->vote($token, $subject, $attributes); $result = $vote; From 66d49ac74c7edd22b733428a5f2b433b268d17d5 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 19:30:36 +0700 Subject: [PATCH 04/35] Standardize Voter code 1. Change internal methods from public to protected 2. Remove useless phpdoc --- .../Security/Core/Authorization/Voter/Voter.php | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index 408c9f29a3cb9..505a1f7bcd1b8 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -71,7 +71,7 @@ public function vote(TokenInterface $token, $subject, array $attributes) /** * Creates a granted vote. */ - public function grant(string $reason = '', array $parameters = []): Vote + protected function grant(string $reason = '', array $parameters = []): Vote { return Vote::createGranted($reason, $parameters); } @@ -79,7 +79,7 @@ public function grant(string $reason = '', array $parameters = []): Vote /** * Creates an abstained vote. */ - public function abstain(string $reason = '', array $parameters = []): Vote + protected function abstain(string $reason = '', array $parameters = []): Vote { return Vote::createAbstain($reason, $parameters); } @@ -87,7 +87,7 @@ public function abstain(string $reason = '', array $parameters = []): Vote /** * Creates a denied vote. */ - public function deny(string $reason = '', array $parameters = []): Vote + protected function deny(string $reason = '', array $parameters = []): Vote { return Vote::createDenied($reason, $parameters); } @@ -98,7 +98,6 @@ public function deny(string $reason = '', array $parameters = []): Vote * @param string $attribute An attribute * @param mixed $subject The subject to secure, e.g. an object the user wants to access or any other PHP type * - * @return bool True if the attribute and subject are supported, false otherwise */ abstract protected function supports(string $attribute, $subject); @@ -108,7 +107,7 @@ abstract protected function supports(string $attribute, $subject); * * @param mixed $subject * - * @return Vote Returning a boolean is deprecated since Symfony 5.1. Return a Vote object instead. + * @return Vote Returning a boolean is deprecated since Symfony 5.4. Return a Vote object instead. */ abstract protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token); } From b43c6e577f443546db2fa2687d34e597e805620b Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 19:41:16 +0700 Subject: [PATCH 05/35] Remove AccessTrait --- .../Core/Authorization/AccessDecision.php | 24 ++++++++++- .../Core/Authorization/Voter/AccessTrait.php | 41 ------------------- .../Core/Authorization/Voter/Vote.php | 24 ++++++++++- 3 files changed, 44 insertions(+), 45 deletions(-) delete mode 100644 src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php index 706deca31c0c2..8c4bac74a0c4d 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -11,7 +11,6 @@ namespace Symfony\Component\Security\Core\Authorization; -use Symfony\Component\Security\Core\Authorization\Voter\AccessTrait; use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\Voter; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; @@ -23,7 +22,8 @@ */ final class AccessDecision { - use AccessTrait; + /** @var int One of the VoterInterface::ACCESS_* constants */ + protected $access; /** @var Vote[] */ private $votes = []; @@ -38,6 +38,26 @@ private function __construct(int $access, array $votes = []) $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 */ diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php deleted file mode 100644 index c008d7cf0c4d0..0000000000000 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php +++ /dev/null @@ -1,41 +0,0 @@ - - * - * 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; - -/** - * @author Dany Maillard - */ -trait AccessTrait -{ - /** @var int One of the VoterInterface::ACCESS_* constants */ - protected $access; - - 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; - } -} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php index 653c879ccddbf..f1363ef6377a2 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -19,8 +19,8 @@ */ final class Vote { - use AccessTrait; - + /** @var int One of the VoterInterface::ACCESS_* constants */ + protected $access; private $reason; private $parameters; @@ -34,6 +34,26 @@ private function __construct(int $access, string $reason = '', array $parameters $this->parameters = $parameters; } + 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 $reason = '', array $parameters = []): self { return new self($access, $reason, $parameters); From cba2283c8ace3501832e89ad3b3a5df97bda8193 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 19:42:22 +0700 Subject: [PATCH 06/35] Change $access property to private as requested --- .../Component/Security/Core/Authorization/AccessDecision.php | 2 +- .../Component/Security/Core/Authorization/Voter/Vote.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php index 8c4bac74a0c4d..6848d6ab93535 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -23,7 +23,7 @@ final class AccessDecision { /** @var int One of the VoterInterface::ACCESS_* constants */ - protected $access; + private $access; /** @var Vote[] */ private $votes = []; diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php index f1363ef6377a2..38b0fdb5ce4aa 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -20,7 +20,7 @@ final class Vote { /** @var int One of the VoterInterface::ACCESS_* constants */ - protected $access; + private $access; private $reason; private $parameters; From 667518223a08bab4e2528daa80f5346a2f73bd59 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 19:55:27 +0700 Subject: [PATCH 07/35] Remove bool return to avoid BC --- .../AccessDecisionManagerInterface.php | 6 +- .../Security/Core/Event/DecisionVoteEvent.php | 68 +++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 src/Symfony/Component/Security/Core/Event/DecisionVoteEvent.php diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index ce495457fee9e..538818a967a06 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -28,9 +28,7 @@ interface AccessDecisionManagerInterface * @param array $attributes An array of attributes associated with the method being invoked * @param object $object The object to secure * - * @return bool true if the access is granted, false otherwise - * - * @deprecated since 5.3, use {@see getDecision()} instead. + * @deprecated since 5.4, use {@see getDecision()} instead. */ - public function decide(TokenInterface $token, array $attributes, $object = null): bool; + public function decide(TokenInterface $token, array $attributes, $object = null); } diff --git a/src/Symfony/Component/Security/Core/Event/DecisionVoteEvent.php b/src/Symfony/Component/Security/Core/Event/DecisionVoteEvent.php new file mode 100644 index 0000000000000..43e739ab72779 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Event/DecisionVoteEvent.php @@ -0,0 +1,68 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +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; + +/** + * This event is dispatched on voter vote. + * + * @author Laurent VOULLEMIER + * + * @internal + */ +final class VoteEvent extends Event +{ + private $voter; + private $subject; + private $attributes; + private $vote; + + /** + * @param Vote|int $vote + */ + public function __construct(VoterInterface $voter, $subject, array $attributes, $vote) + { + $this->voter = $voter; + $this->subject = $subject; + $this->attributes = $attributes; + if (!$vote instanceof Vote) { + trigger_deprecation('symfony/security-core', '5.3', 'Passing an int as the fourth argument to "%s::__construct" is deprecated, pass a "%s" instance instead.', __CLASS__, Vote::class); + + $vote = Vote::create($vote); + } + + $this->vote = $vote; + } + + public function getVoter(): VoterInterface + { + return $this->voter; + } + + public function getSubject() + { + return $this->subject; + } + + public function getAttributes(): array + { + return $this->attributes; + } + + public function getVote(): Vote + { + return $this->vote; + } +} From c15b7962cb49f492e4cba23ef0eb76b8e9187695 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 19:58:36 +0700 Subject: [PATCH 08/35] Fix return typehint for decide method --- .../Security/Core/Authorization/AccessDecisionManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index d0696810e44ec..3e25e13862b7f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -68,7 +68,7 @@ public function getDecision(TokenInterface $token, array $attributes, object $ob * * @deprecated since 5.3, use {@see getDecision()} instead. */ - public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/): bool + public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/) { trigger_deprecation('symfony/security-core', '5.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); From 7e5feef95ee22271aa65dd080683a54966643a85 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 20:03:23 +0700 Subject: [PATCH 09/35] Switch to using constructor for Vote --- .../Authorization/AccessDecisionManager.php | 2 +- .../TraceableAccessDecisionManager.php | 2 +- .../Authorization/Voter/TraceableVoter.php | 2 +- .../Core/Authorization/Voter/Vote.php | 2 +- .../Security/Core/Event/DecisionVoteEvent.php | 68 ------------------- .../Security/Core/Event/VoteEvent.php | 2 +- 6 files changed, 5 insertions(+), 73 deletions(-) delete mode 100644 src/Symfony/Component/Security/Core/Event/DecisionVoteEvent.php diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 3e25e13862b7f..6c4966adfe946 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -252,7 +252,7 @@ private function vote(VoterInterface $voter, TokenInterface $token, $subject, ar trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return a "%s" object instead.', var_export($result, true), get_debug_type($voter), Vote::class); if (\in_array($result, [VoterInterface::ACCESS_ABSTAIN, VoterInterface::ACCESS_DENIED, VoterInterface::ACCESS_GRANTED], true)) { - return Vote::create($result); + return new Vote($result); } return null; diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index fbec6361a0d2d..4f7ae84fd2cf9 100644 --- a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php @@ -103,7 +103,7 @@ public function addVoterVote(VoterInterface $voter, array $attributes, $vote) { if (!$vote instanceof Vote) { trigger_deprecation('symfony/security-core', '5.3', 'Passing an int as the third argument to "%s::addVoterVote()" is deprecated, pass an instance of "%s" instead.', __CLASS__, Vote::class); - $vote = Vote::create($vote); + $vote = new Vote($vote); } $currentLogIndex = \count($this->currentLog) - 1; diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index 119f9431bfdb4..3c5732c1f7691 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -40,7 +40,7 @@ public function vote(TokenInterface $token, $subject, array $attributes) if (!$vote instanceof Vote && \in_array($vote, [VoterInterface::ACCESS_DENIED, VoterInterface::ACCESS_GRANTED, VoterInterface::ACCESS_ABSTAIN])) { trigger_deprecation('symfony/security-core', '5.3', 'Returning an int from "%s::vote" is deprecated, return an instance of "%s" instead.', \get_class($this->voter), Vote::class); - $result = Vote::create($vote); + $result = new Vote($vote); } $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $vote), 'debug.security.authorization.vote'); diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php index 38b0fdb5ce4aa..7ea734184cf06 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -27,7 +27,7 @@ final class Vote /** * @param int $access One of the VoterInterface::ACCESS_* constants */ - private function __construct(int $access, string $reason = '', array $parameters = []) + public function __construct(int $access, string $reason = '', array $parameters = []) { $this->access = $access; $this->reason = $reason; diff --git a/src/Symfony/Component/Security/Core/Event/DecisionVoteEvent.php b/src/Symfony/Component/Security/Core/Event/DecisionVoteEvent.php deleted file mode 100644 index 43e739ab72779..0000000000000 --- a/src/Symfony/Component/Security/Core/Event/DecisionVoteEvent.php +++ /dev/null @@ -1,68 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -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; - -/** - * This event is dispatched on voter vote. - * - * @author Laurent VOULLEMIER - * - * @internal - */ -final class VoteEvent extends Event -{ - private $voter; - private $subject; - private $attributes; - private $vote; - - /** - * @param Vote|int $vote - */ - public function __construct(VoterInterface $voter, $subject, array $attributes, $vote) - { - $this->voter = $voter; - $this->subject = $subject; - $this->attributes = $attributes; - if (!$vote instanceof Vote) { - trigger_deprecation('symfony/security-core', '5.3', 'Passing an int as the fourth argument to "%s::__construct" is deprecated, pass a "%s" instance instead.', __CLASS__, Vote::class); - - $vote = Vote::create($vote); - } - - $this->vote = $vote; - } - - public function getVoter(): VoterInterface - { - return $this->voter; - } - - public function getSubject() - { - return $this->subject; - } - - public function getAttributes(): array - { - return $this->attributes; - } - - public function getVote(): Vote - { - return $this->vote; - } -} diff --git a/src/Symfony/Component/Security/Core/Event/VoteEvent.php b/src/Symfony/Component/Security/Core/Event/VoteEvent.php index 43e739ab72779..1b9a1d1f4da57 100644 --- a/src/Symfony/Component/Security/Core/Event/VoteEvent.php +++ b/src/Symfony/Component/Security/Core/Event/VoteEvent.php @@ -40,7 +40,7 @@ public function __construct(VoterInterface $voter, $subject, array $attributes, if (!$vote instanceof Vote) { trigger_deprecation('symfony/security-core', '5.3', 'Passing an int as the fourth argument to "%s::__construct" is deprecated, pass a "%s" instance instead.', __CLASS__, Vote::class); - $vote = Vote::create($vote); + $vote = new Vote($vote); } $this->vote = $vote; From 55bc4cd3060c5d446e28835f9f319078931fdf89 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 20:08:44 +0700 Subject: [PATCH 10/35] Fix AccessDeniedException Add missing message passed through constructor --- .../Component/Security/Core/Exception/AccessDeniedException.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index 6f2f73418dee8..9fd49cd39559c 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -77,7 +77,7 @@ public function setAccessDecision(AccessDecision $accessDecision) return $vote->getReason(); }, $accessDecision->getDeniedVotes()); - $this->message = 'Access Denied: '.rtrim(' '.implode(' ', $reasons)); + $this->message .= ' ' . rtrim(' ' . implode(' ', $reasons)); } public function getAccessDecision(): AccessDecision From 219a3be9ba8400810712ee60b6f02c5558227d70 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 20:13:58 +0700 Subject: [PATCH 11/35] Fix coding standards --- .../Bundle/FrameworkBundle/Controller/AbstractController.php | 2 +- .../Security/Core/Authorization/AccessDecisionManager.php | 2 +- .../Core/Authorization/AccessDecisionManagerInterface.php | 2 +- .../Core/Authorization/TraceableAccessDecisionManager.php | 2 +- .../Component/Security/Core/Authorization/Voter/Voter.php | 1 - .../Component/Security/Core/Exception/AccessDeniedException.php | 2 +- .../Core/Tests/Authorization/AuthorizationCheckerTest.php | 1 - 7 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index bcb1f6930c769..74f97124b04f3 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php @@ -424,7 +424,7 @@ protected function getUser() return null; } - // @deprecated since 5.4, $user will always be a UserInterface instance + // @deprecated since Symfony 5.4, $user will always be a UserInterface instance if (!\is_object($user = $token->getUser())) { // e.g. anonymous authentication return null; diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 6c4966adfe946..a90dc230d1501 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -66,7 +66,7 @@ public function getDecision(TokenInterface $token, array $attributes, object $ob * * {@inheritdoc} * - * @deprecated since 5.3, use {@see getDecision()} instead. + * @deprecated since Symfony 5.4, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/) { diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index 538818a967a06..0758c83e4f9c1 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 object $object The object to secure * - * @deprecated since 5.4, use {@see getDecision()} instead. + * @deprecated since Symfony 5.4, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, $object = null); } diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index 4f7ae84fd2cf9..26e07a8b72b7a 100644 --- a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php @@ -70,7 +70,7 @@ public function getDecision(TokenInterface $token, array $attributes, object $ob * * @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array * - * @deprecated since 5.3, use {@see getDecision()} instead. + * @deprecated since Symfony 5.4, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/): bool { diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index 505a1f7bcd1b8..f1cd19f456d84 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -97,7 +97,6 @@ protected function deny(string $reason = '', array $parameters = []): Vote * * @param string $attribute An attribute * @param mixed $subject The subject to secure, e.g. an object the user wants to access or any other PHP type - * */ abstract protected function supports(string $attribute, $subject); diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index 9fd49cd39559c..52874b9c26626 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -77,7 +77,7 @@ public function setAccessDecision(AccessDecision $accessDecision) return $vote->getReason(); }, $accessDecision->getDeniedVotes()); - $this->message .= ' ' . rtrim(' ' . implode(' ', $reasons)); + $this->message .= ' '.rtrim(' '.implode(' ', $reasons)); } public function getAccessDecision(): AccessDecision diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 6e9d3f56556cb..1d9b19f775708 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -16,7 +16,6 @@ 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\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationChecker; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; use Symfony\Component\Security\Core\User\InMemoryUser; From 8c5fcb0041e20f53a90129730ac297042a5b61bc Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 20:45:03 +0700 Subject: [PATCH 12/35] Fix test results --- .../Tests/Authorization/TraceableAccessDecisionManagerTest.php | 2 +- .../Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php index 4aa5a067e00f4..175627d5d7e63 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -49,7 +49,7 @@ public function testDecideLog(array $expectedLog, array $attributes, $object, ar $adm->getDecision($token, $attributes, $object); - $this->assertSame($expectedLog, $adm->getDecisionLog()); + $this->assertEquals($expectedLog, $adm->getDecisionLog()); } public function provideObjectsAndLogs(): \Generator 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 a1ec7543f8308..0aa0c169530ce 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -30,7 +30,7 @@ public function testVote($authenticated, $attributes, $expected) { $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); - $this->assertSame($expected, $voter->vote($this->getToken($authenticated), null, $attributes)); + $this->assertEquals($expected, $voter->vote($this->getToken($authenticated), null, $attributes)); } public function getVoteTests() From d9709201135ec8a6b8a475e2ac9bbb2f4a1ff8f9 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 20:55:43 +0700 Subject: [PATCH 13/35] Fix deprecation to match with test --- .../Security/Core/Authorization/AccessDecisionManager.php | 2 +- .../Core/Tests/Authorization/AccessDecisionManagerTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index a90dc230d1501..cb5d200b71b8f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -249,7 +249,7 @@ private function vote(VoterInterface $voter, TokenInterface $token, $subject, ar return $result; } - trigger_deprecation('symfony/security-core', '5.3', 'Returning "%s" in "%s::vote()" is deprecated, return a "%s" object instead.', var_export($result, true), get_debug_type($voter), Vote::class); + trigger_deprecation('symfony/security-core', '5.4', 'Returning "%s" in "%s::vote()" is deprecated, return an instance of "%s" instead.', var_export($result, true), get_debug_type($voter), Vote::class); if (\in_array($result, [VoterInterface::ACCESS_ABSTAIN, VoterInterface::ACCESS_DENIED, VoterInterface::ACCESS_GRANTED], true)) { return new Vote($result); diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index a94bca0a60822..d0a16628c6d32 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -62,7 +62,7 @@ public function testDeprecatedVoter($strategy) $manager = new AccessDecisionManager([$this->getVoter(3)], $strategy); $this->expectDeprecation('Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.'); - $this->expectDeprecation('Since symfony/security-core 5.3: Returning "3" in "%s::vote()" is deprecated, return an instance of "%s" instead.'); + $this->expectDeprecation('Since symfony/security-core 5.4: Returning "3" in "%s::vote()" is deprecated, return an instance of "%s" instead.'); $manager->decide($token, ['ROLE_FOO']); } From a9578c47e386fa3058ac24893d7c8eb7101a0e84 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 21:07:47 +0700 Subject: [PATCH 14/35] Fix decide method deprecation test matching --- .../Security/Core/Authorization/AccessDecisionManager.php | 2 +- .../Core/Authorization/TraceableAccessDecisionManager.php | 2 +- .../Core/Tests/Authorization/AccessDecisionManagerTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index cb5d200b71b8f..56d9722ec2216 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -70,7 +70,7 @@ public function getDecision(TokenInterface $token, array $attributes, object $ob */ public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/) { - trigger_deprecation('symfony/security-core', '5.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '5.4', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); $allowMultipleAttributes = 3 < \func_num_args() && func_get_arg(3); diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index 26e07a8b72b7a..9358dec77db16 100644 --- a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php @@ -74,7 +74,7 @@ public function getDecision(TokenInterface $token, array $attributes, object $ob */ public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/): bool { - trigger_deprecation('symfony/security-core', '5.3', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); + trigger_deprecation('symfony/security-core', '5.4', 'Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.', __CLASS__, __CLASS__); $currentDecisionLog = [ 'attributes' => $attributes, diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index d0a16628c6d32..4735943c0c943 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -61,7 +61,7 @@ public function testDeprecatedVoter($strategy) $token = $this->createMock(TokenInterface::class); $manager = new AccessDecisionManager([$this->getVoter(3)], $strategy); - $this->expectDeprecation('Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.'); + $this->expectDeprecation('Since symfony/security-core 5.4: Method "%s::decide()" has been deprecated, use "%s::getDecision()" instead.'); $this->expectDeprecation('Since symfony/security-core 5.4: Returning "3" in "%s::vote()" is deprecated, return an instance of "%s" instead.'); $manager->decide($token, ['ROLE_FOO']); From 1357b539dd487d52077ea416b8bf90ba2a550889 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 21:30:03 +0700 Subject: [PATCH 15/35] Fix tests in AccessListenerTest --- .../Tests/Firewall/AccessListenerTest.php | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index 252cad9ed23a5..94fe5c676255e 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -95,12 +95,19 @@ public function testLegacyHandleWhenTheAccessDecisionManagerDecidesToRefuseAcces ->willReturn([['foo' => 'bar'], null]) ; - $token = $this->createMock(TokenInterface::class); - $token - ->expects($this->any()) - ->method('isAuthenticated') - ->willReturn(true) - ; + $token = new class() extends AbstractToken { + public function isAuthenticated(): bool + { + return true; + } + + /** + * @return mixed + */ + public function getCredentials() + { + } + }; $tokenStorage = $this->createMock(TokenStorageInterface::class); $tokenStorage @@ -109,7 +116,7 @@ public function testLegacyHandleWhenTheAccessDecisionManagerDecidesToRefuseAcces ->willReturn($token) ; - $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $accessDecisionManager ->expects($this->once()) ->method('decide') @@ -121,7 +128,7 @@ public function testLegacyHandleWhenTheAccessDecisionManagerDecidesToRefuseAcces $tokenStorage, $accessDecisionManager, $accessMap, - $this->createMock(AuthenticationManagerInterface::class) + false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -185,7 +192,8 @@ public function testHandleWhenTheTokenIsNotAuthenticated() $tokenStorage, $accessDecisionManager, $accessMap, - $authManager + $authManager, + false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -240,7 +248,7 @@ public function testLegacyHandleWhenTheTokenIsNotAuthenticated() ->with($this->equalTo($authenticatedToken)) ; - $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $accessDecisionManager ->expects($this->once()) ->method('decide') @@ -252,7 +260,8 @@ public function testLegacyHandleWhenTheTokenIsNotAuthenticated() $tokenStorage, $accessDecisionManager, $accessMap, - $authManager + $authManager, + false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -490,7 +499,7 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() ->willReturn([['foo' => 'bar', 'bar' => 'baz'], null]) ; - $authenticatedToken = new UsernamePasswordToken('test', 'test', 'test', ['ROLE_USER']); + $authenticatedToken = new UsernamePasswordToken(new InMemoryUser('test', 'test', ['ROLE_USER']), 'test', ['ROLE_USER']); $tokenStorage = new TokenStorage(); $tokenStorage->setToken($authenticatedToken); @@ -526,7 +535,7 @@ public function testLazyPublicPagesShouldNotAccessTokenStorage() ->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null]) ; - $listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManager::class), $accessMap, false); + $listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, false); $listener(new LazyResponseEvent(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST))); } @@ -546,7 +555,7 @@ public function testLegacyLazyPublicPagesShouldNotAccessTokenStorage() ->willReturn([[AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY], null]) ; - $listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManager::class), $accessMap, false); + $listener = new AccessListener($tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, false); $listener(new LazyResponseEvent(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST))); } } From b9493174f830c53906c81f09ea71fc9cee0467aa Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 21:51:31 +0700 Subject: [PATCH 16/35] Add missing AbstractToken in AccessListenerTest --- .../Security/Http/Tests/Firewall/AccessListenerTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index 94fe5c676255e..8d8fb0a5e92b8 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; +use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; From 6b9ec12c79956d6572492a7049c1672af5b8b21d Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Thu, 23 Sep 2021 21:52:44 +0700 Subject: [PATCH 17/35] getDecisionLog return object, not boolean --- .../TraceableAccessDecisionManagerTest.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php index 175627d5d7e63..8624e8fb4b57f 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -256,8 +256,8 @@ public function testAccessDecisionManagerCalledByVoter() }); $token = $this->createMock(TokenInterface::class); - $sut->getDecision($token, ['attr1'], null); - $sut->getDecision($token, ['attr2'], $obj = new \stdClass()); + $result1 = $sut->getDecision($token, ['attr1'], null); + $result2 = $sut->getDecision($token, ['attr2'], $obj = new \stdClass()); $this->assertEquals([ [ @@ -266,7 +266,7 @@ public function testAccessDecisionManagerCalledByVoter() 'voterDetails' => [ ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => $result1, ], [ 'attributes' => ['attr2'], @@ -275,7 +275,7 @@ public function testAccessDecisionManagerCalledByVoter() ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => Vote::createAbstain()], ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => $result2, ], [ 'attributes' => ['attr2'], @@ -285,7 +285,7 @@ public function testAccessDecisionManagerCalledByVoter() ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => Vote::createDenied()], ['voter' => $voter3, 'attributes' => ['attr2'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => $result2, ], ], $sut->getDecisionLog()); } From dfec312be84c499322415d88850c0d4d5e2ed19b Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 09:04:25 +0700 Subject: [PATCH 18/35] Attempt to fix test in AccessListenerTest --- .../Security/Http/Tests/Firewall/AccessListenerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index 8d8fb0a5e92b8..aa4dc8599bf8b 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -509,7 +509,7 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() $accessDecisionManager ->expects($this->once()) ->method('getDecision') - ->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), true) + ->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request)) ->willReturn(AccessDecision::createGranted()) ; From 95e96827b5b085541cb1ab5ac0b812b9817fd0c0 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 10:39:08 +0700 Subject: [PATCH 19/35] Fix testAccessDecisionManagerCalledByVoter --- .../TraceableAccessDecisionManagerTest.php | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php index 8624e8fb4b57f..f4b7a4333561c 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -256,8 +256,8 @@ public function testAccessDecisionManagerCalledByVoter() }); $token = $this->createMock(TokenInterface::class); - $result1 = $sut->getDecision($token, ['attr1'], null); - $result2 = $sut->getDecision($token, ['attr2'], $obj = new \stdClass()); + $sut->getDecision($token, ['attr1'], null); + $sut->getDecision($token, ['attr2'], $obj = new \stdClass()); $this->assertEquals([ [ @@ -266,7 +266,9 @@ public function testAccessDecisionManagerCalledByVoter() 'voterDetails' => [ ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => Vote::createGranted()], ], - 'result' => $result1, + 'result' => AccessDecision::createGranted([ + Vote::createGranted() + ]), ], [ 'attributes' => ['attr2'], @@ -275,7 +277,10 @@ public function testAccessDecisionManagerCalledByVoter() ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => Vote::createAbstain()], ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => Vote::createGranted()], ], - 'result' => $result2, + 'result' => AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createGranted() + ]), ], [ 'attributes' => ['attr2'], @@ -285,7 +290,11 @@ public function testAccessDecisionManagerCalledByVoter() ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => Vote::createDenied()], ['voter' => $voter3, 'attributes' => ['attr2'], 'vote' => Vote::createGranted()], ], - 'result' => $result2, + 'result' => AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createDenied(), + Vote::createGranted() + ]), ], ], $sut->getDecisionLog()); } From 282c972c6fd7e3abd6f2747f4446f2bb08eb6576 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 10:47:18 +0700 Subject: [PATCH 20/35] Add mising class in test --- .../Core/Tests/Authorization/AuthorizationCheckerTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 1d9b19f775708..f80d334154edb 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -16,6 +16,7 @@ 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\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\AuthorizationChecker; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; use Symfony\Component\Security\Core\User\InMemoryUser; From 5993a1a96778211113192be35307029e58b247e8 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 11:44:42 +0700 Subject: [PATCH 21/35] Fix coding standards --- .../Authorization/TraceableAccessDecisionManagerTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php index f4b7a4333561c..390487d42930a 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -267,7 +267,7 @@ public function testAccessDecisionManagerCalledByVoter() ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => Vote::createGranted()], ], 'result' => AccessDecision::createGranted([ - Vote::createGranted() + Vote::createGranted(), ]), ], [ @@ -279,7 +279,7 @@ public function testAccessDecisionManagerCalledByVoter() ], 'result' => AccessDecision::createGranted([ Vote::createAbstain(), - Vote::createGranted() + Vote::createGranted(), ]), ], [ @@ -293,7 +293,7 @@ public function testAccessDecisionManagerCalledByVoter() 'result' => AccessDecision::createGranted([ Vote::createAbstain(), Vote::createDenied(), - Vote::createGranted() + Vote::createGranted(), ]), ], ], $sut->getDecisionLog()); From 04159fd41343c118d3f58fa7ffcb7d3c54ed7f7a Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 11:45:25 +0700 Subject: [PATCH 22/35] Fix issue with final class mock --- .../Core/Tests/Authorization/AuthorizationCheckerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index f80d334154edb..160b921b3075c 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -16,7 +16,7 @@ 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\AccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationChecker; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; use Symfony\Component\Security\Core\User\InMemoryUser; @@ -30,7 +30,7 @@ class AuthorizationCheckerTest extends TestCase protected function setUp(): void { - $this->accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $this->accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $this->tokenStorage = new TokenStorage(); $this->authorizationChecker = new AuthorizationChecker( From 8b4430de5dda2627b80be3c57efecc559d5b8df7 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 12:05:23 +0700 Subject: [PATCH 23/35] Fix issue with final class mock --- .../Security/Http/Tests/Firewall/AccessListenerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index aa4dc8599bf8b..75db00dbfb3d4 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -411,7 +411,7 @@ public function testLegacyHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptio ->willReturn([['foo' => 'bar'], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $accessDecisionManager->expects($this->once()) ->method('decide') ->with($this->isInstanceOf(NullToken::class)) From e2212f65666045cbc27253847a809bb833b937a9 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 12:17:01 +0700 Subject: [PATCH 24/35] Fix issue with final class mock --- .../Security/Http/Tests/Firewall/SwitchUserListenerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php index 47203fcfe300c..7d648f19ba03b 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php @@ -26,7 +26,7 @@ use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Core\User\InMemoryUserProvider; -use Symfony\Component\Security\Core\User\User; + use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Http\Event\SwitchUserEvent; use Symfony\Component\Security\Http\Firewall\SwitchUserListener; @@ -347,7 +347,7 @@ public function testLegacySwitchUserWithReplacedToken() $this->tokenStorage->setToken($token); $this->request->query->set('_switch_user', 'kuba'); - $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); $accessDecisionManager->expects($this->any()) ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH'], $this->callback(function ($user) { return 'kuba' === $user->getUserIdentifier(); })) ->willReturn(true); From 4f52a2c91e633b1c11e70d676c4b86c3a81ecef0 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 12:27:16 +0700 Subject: [PATCH 25/35] Update version in deprecation message --- .../Security/Core/Authorization/AuthorizationChecker.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index 88eb219834a8a..348fb3ba147b7 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -83,7 +83,7 @@ public function getDecision($attribute, $subject = null): AccessDecision } if (!method_exists($this->accessDecisionManager, 'getDecision')) { - trigger_deprecation('symfony/security-core', 5.3, 'Not implementing "%s::getDecision()" method is deprecated, and would be required in 6.0.', \get_class($this->accessDecisionManager)); + trigger_deprecation('symfony/security-core', 5.4, 'Not implementing "%s::getDecision()" method is deprecated, and would be required in 6.0.', \get_class($this->accessDecisionManager)); return $this->accessDecisionManager->decide($token, [$attribute], $subject) ? AccessDecision::createGranted() : AccessDecision::createDenied(); } From de1f7e3858697c8bffd71edc0e1dc4fef4053216 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 12:32:54 +0700 Subject: [PATCH 26/35] Fix BC with VoteEvent --- .../Bundle/SecurityBundle/EventListener/VoteListener.php | 2 +- src/Symfony/Component/Security/Core/Event/VoteEvent.php | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php b/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php index 1b37d92373705..d9cb9934e6931 100644 --- a/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php +++ b/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php @@ -36,7 +36,7 @@ public function __construct(TraceableAccessDecisionManager $traceableAccessDecis */ public function onVoterVote(VoteEvent $event) { - $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/Component/Security/Core/Event/VoteEvent.php b/src/Symfony/Component/Security/Core/Event/VoteEvent.php index 1b9a1d1f4da57..c56caaa0243c1 100644 --- a/src/Symfony/Component/Security/Core/Event/VoteEvent.php +++ b/src/Symfony/Component/Security/Core/Event/VoteEvent.php @@ -61,7 +61,12 @@ public function getAttributes(): array return $this->attributes; } - public function getVote(): Vote + public function getVote(): int + { + return $this->vote->getAccess(); + } + + public function getVoteDecision(): Vote { return $this->vote; } From 7d4e955d959d9ad4eb0a02aa527d7cc7a07636f2 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 14:55:57 +0700 Subject: [PATCH 27/35] Fix deprecation version --- .../Core/Authorization/TraceableAccessDecisionManager.php | 2 +- .../Security/Core/Authorization/Voter/TraceableVoter.php | 2 +- .../Component/Security/Core/Authorization/Voter/Voter.php | 2 +- src/Symfony/Component/Security/Core/Event/VoteEvent.php | 2 +- .../Core/Tests/Authorization/Voter/TraceableVoterTest.php | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index 9358dec77db16..7e447567b4205 100644 --- a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php @@ -102,7 +102,7 @@ public function decide(TokenInterface $token, array $attributes, $object = null/ public function addVoterVote(VoterInterface $voter, array $attributes, $vote) { if (!$vote instanceof Vote) { - trigger_deprecation('symfony/security-core', '5.3', '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', '5.4', '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/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index 3c5732c1f7691..2ee2396612b57 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -38,7 +38,7 @@ public function vote(TokenInterface $token, $subject, array $attributes) $vote = $this->voter->vote($token, $subject, $attributes); $result = $vote; if (!$vote instanceof Vote && \in_array($vote, [VoterInterface::ACCESS_DENIED, VoterInterface::ACCESS_GRANTED, VoterInterface::ACCESS_ABSTAIN])) { - trigger_deprecation('symfony/security-core', '5.3', 'Returning an int from "%s::vote" is deprecated, return an instance of "%s" instead.', \get_class($this->voter), Vote::class); + trigger_deprecation('symfony/security-core', '5.4', 'Returning an int from "%s::vote" is deprecated, return an instance of "%s" instead.', \get_class($this->voter), Vote::class); $result = new Vote($vote); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index f1cd19f456d84..578b444a6d7d1 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -53,7 +53,7 @@ public function vote(TokenInterface $token, $subject, array $attributes) $decision = $this->voteOnAttribute($attribute, $subject, $token); if (\is_bool($decision)) { - trigger_deprecation('symfony/security-core', '5.3', 'Returning a boolean in "%s::voteOnAttribute()" is deprecated, return an instance of "%s" instead.', static::class, Vote::class); + trigger_deprecation('symfony/security-core', '5.4', 'Returning a boolean in "%s::voteOnAttribute()" is deprecated, return an instance of "%s" instead.', static::class, Vote::class); $decision = $decision ? $this->grant() : $this->deny(); } diff --git a/src/Symfony/Component/Security/Core/Event/VoteEvent.php b/src/Symfony/Component/Security/Core/Event/VoteEvent.php index c56caaa0243c1..35272767d9a22 100644 --- a/src/Symfony/Component/Security/Core/Event/VoteEvent.php +++ b/src/Symfony/Component/Security/Core/Event/VoteEvent.php @@ -38,7 +38,7 @@ public function __construct(VoterInterface $voter, $subject, array $attributes, $this->subject = $subject; $this->attributes = $attributes; if (!$vote instanceof Vote) { - trigger_deprecation('symfony/security-core', '5.3', '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', '5.4', 'Passing an int as the fourth argument to "%s::__construct" is deprecated, pass a "%s" instance instead.', __CLASS__, Vote::class); $vote = new Vote($vote); } 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 d28d6542bd07a..9d6db8c8c51eb 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php @@ -79,7 +79,7 @@ public function testVoteLegacy() $sut = new TraceableVoter($voter, $eventDispatcher); - $this->expectDeprecation(sprintf('Since symfony/security-core 5.3: Returning an int from "%s::vote" is deprecated, return an instance of "%s" instead.', \get_class($voter), Vote::class)); + $this->expectDeprecation(sprintf('Since symfony/security-core 5.4: Returning an int from "%s::vote" is deprecated, return an instance of "%s" instead.', \get_class($voter), Vote::class)); $result = $sut->vote($token, 'anysubject', ['attr1']); From 7589a16facd12e0000c528d392269394e5bde23c Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 15:03:05 +0700 Subject: [PATCH 28/35] Rename vote $reason to $message, $parameters to $context --- .../views/Collector/security.html.twig | 2 +- .../Core/Authorization/Voter/Vote.php | 40 +++++++++---------- .../Core/Authorization/Voter/Voter.php | 14 +++---- .../Core/Exception/AccessDeniedException.php | 6 +-- 4 files changed, 31 insertions(+), 31 deletions(-) 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 d229ae5caaf7f..4301fa6a942d5 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig @@ -376,7 +376,7 @@ unknown ({{ voter_detail['vote'].access }}) {% endif %} - {{ voter_detail['vote'].reason|default('~') }} + {{ voter_detail['vote'].message|default('~') }} {% endfor %} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php index 7ea734184cf06..a0df7b2e5e7b1 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -13,7 +13,7 @@ /** * A Vote is returned by a Voter and contains the access (granted, abstain or denied). - * It can also contain a reason explaining the vote decision. + * It can also contain a message explaining the vote decision. * * @author Dany Maillard */ @@ -21,17 +21,17 @@ final class Vote { /** @var int One of the VoterInterface::ACCESS_* constants */ private $access; - private $reason; - private $parameters; + private $message; + private $context; /** * @param int $access One of the VoterInterface::ACCESS_* constants */ - public function __construct(int $access, string $reason = '', array $parameters = []) + public function __construct(int $access, string $message = '', array $context = []) { $this->access = $access; - $this->reason = $reason; - $this->parameters = $parameters; + $this->message = $message; + $this->context = $context; } public function getAccess(): int @@ -54,38 +54,38 @@ public function isDenied(): bool return VoterInterface::ACCESS_DENIED === $this->access; } - public static function create(int $access, string $reason = '', array $parameters = []): self + public static function create(int $access, string $message = '', array $context = []): self { - return new self($access, $reason, $parameters); + return new self($access, $message, $context); } - public static function createGranted(string $reason = '', array $parameters = []): self + public static function createGranted(string $message = '', array $context = []): self { - return new self(VoterInterface::ACCESS_GRANTED, $reason, $parameters); + return new self(VoterInterface::ACCESS_GRANTED, $message, $context); } - public static function createAbstain(string $reason = '', array $parameters = []): self + public static function createAbstain(string $message = '', array $context = []): self { - return new self(VoterInterface::ACCESS_ABSTAIN, $reason, $parameters); + return new self(VoterInterface::ACCESS_ABSTAIN, $message, $context); } - public static function createDenied(string $reason = '', array $parameters = []): self + public static function createDenied(string $message = '', array $context = []): self { - return new self(VoterInterface::ACCESS_DENIED, $reason, $parameters); + return new self(VoterInterface::ACCESS_DENIED, $message, $context); } - public function setReason(string $reason) + public function setMessage(string $message) { - $this->reason = $reason; + $this->message = $message; } - public function getReason(): string + public function getMessage(): string { - return $this->reason; + return $this->message; } - public function getParameters(): array + public function getContext(): array { - return $this->parameters; + 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 578b444a6d7d1..087c2b3ae9bb9 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -62,7 +62,7 @@ public function vote(TokenInterface $token, $subject, array $attributes) return $decision; } - $vote->setReason($vote->getReason().trim(' '.$decision->getReason())); + $vote->setMessage($vote->getMessage().trim(' '.$decision->getMessage())); } return $vote; @@ -71,25 +71,25 @@ public function vote(TokenInterface $token, $subject, array $attributes) /** * Creates a granted vote. */ - protected function grant(string $reason = '', array $parameters = []): Vote + protected function grant(string $message = '', array $context = []): Vote { - return Vote::createGranted($reason, $parameters); + return Vote::createGranted($message, $context); } /** * Creates an abstained vote. */ - protected function abstain(string $reason = '', array $parameters = []): Vote + protected function abstain(string $message = '', array $context = []): Vote { - return Vote::createAbstain($reason, $parameters); + return Vote::createAbstain($message, $context); } /** * Creates a denied vote. */ - protected function deny(string $reason = '', array $parameters = []): Vote + protected function deny(string $message = '', array $context = []): Vote { - return Vote::createDenied($reason, $parameters); + return Vote::createDenied($message, $context); } /** diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index 52874b9c26626..cceb17bfb55dc 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -73,11 +73,11 @@ public function setAccessDecision(AccessDecision $accessDecision) return; } - $reasons = array_map(static function (Vote $vote) { - return $vote->getReason(); + $messages = array_map(static function (Vote $vote) { + return $vote->getMessage(); }, $accessDecision->getDeniedVotes()); - $this->message .= ' '.rtrim(' '.implode(' ', $reasons)); + $this->message .= ' '.rtrim(' '.implode(' ', $messages)); } public function getAccessDecision(): AccessDecision From 8aacd94abc1c4990510bdacee961370fa5b13e08 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 15:04:26 +0700 Subject: [PATCH 29/35] Add back return typehint in phpdoc --- .../Core/Authorization/AccessDecisionManagerInterface.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index 0758c83e4f9c1..4445014af7143 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -27,6 +27,7 @@ interface AccessDecisionManagerInterface * * @param array $attributes An array of attributes associated with the method being invoked * @param object $object The object to secure + * @return bool * * @deprecated since Symfony 5.4, use {@see getDecision()} instead. */ From d776986c58d9464f990bbf50b4d842ef1c624a92 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Fri, 24 Sep 2021 15:09:55 +0700 Subject: [PATCH 30/35] Fix coding standard --- .../Core/Authorization/AccessDecisionManagerInterface.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index 4445014af7143..db54236cf51e6 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -27,6 +27,7 @@ interface AccessDecisionManagerInterface * * @param array $attributes An array of attributes associated with the method being invoked * @param object $object The object to secure + * * @return bool * * @deprecated since Symfony 5.4, use {@see getDecision()} instead. From 991857d0bb1c9828525b553d7ee75ba1ab176e48 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Sun, 26 Sep 2021 21:09:59 +0700 Subject: [PATCH 31/35] Add more checkings for deprecation notices inside tests --- .../Authorization/AccessDecisionManager.php | 1 - .../AccessDecisionManagerInterface.php | 2 +- .../Authorization/AuthorizationChecker.php | 4 +- .../AuthorizationCheckerTest.php | 105 +++++++++++++++++- .../TraceableAccessDecisionManagerTest.php | 3 + .../Tests/Firewall/AccessListenerTest.php | 48 ++++---- 6 files changed, 133 insertions(+), 30 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 56d9722ec2216..2e9d9c2ae052f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -13,7 +13,6 @@ use Symfony\Component\Security\Core\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; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index db54236cf51e6..b70e3ca193ae1 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -27,7 +27,7 @@ interface AccessDecisionManagerInterface * * @param array $attributes An array of attributes associated with the method being invoked * @param object $object The object to secure - * + * * @return bool * * @deprecated since Symfony 5.4, use {@see getDecision()} instead. diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index 348fb3ba147b7..e96b498b75b75 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -35,7 +35,7 @@ class AuthorizationChecker implements AuthorizationCheckerInterface public function __construct(TokenStorageInterface $tokenStorage, /*AccessDecisionManagerInterface*/ $accessDecisionManager, /*bool*/ $alwaysAuthenticate = false, /*bool*/ $exceptionOnNoToken = true) { if ($accessDecisionManager instanceof AuthenticationManagerInterface) { - trigger_deprecation('symfony/security-core', '5.4', 'The $autenticationManager argument of "%s" is deprecated.', __METHOD__); + trigger_deprecation('symfony/security-core', '5.4', 'The $authenticationManager argument of "%s" is deprecated.', __METHOD__); $this->authenticationManager = $accessDecisionManager; $accessDecisionManager = $alwaysAuthenticate; @@ -78,7 +78,7 @@ public function getDecision($attribute, $subject = null): AccessDecision } $token = new NullToken(); - } elseif ($this->alwaysAuthenticate || !$token->isAuthenticated()) { + } elseif ($this->authenticationManager && ($this->alwaysAuthenticate || !$token->isAuthenticated())) { $this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token)); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 160b921b3075c..32c87a7f5b717 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -12,10 +12,13 @@ namespace Symfony\Component\Security\Core\Tests\Authorization; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface; 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\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationChecker; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; @@ -23,6 +26,8 @@ class AuthorizationCheckerTest extends TestCase { + use ExpectDeprecationTrait; + private $authenticationManager; private $accessDecisionManager; private $authorizationChecker; @@ -44,7 +49,7 @@ protected function setUp(): void /** * @group legacy */ - public function testVoteAuthenticatesTokenIfNecessary() + public function testLegacyVoteAuthenticatesTokenIfNecessary() { $token = new UsernamePasswordToken('username', 'password', 'provider'); $this->tokenStorage->setToken($token); @@ -77,6 +82,25 @@ public function testVoteAuthenticatesTokenIfNecessary() $this->assertSame($newToken, $this->tokenStorage->getToken()); } + public function testVoteAuthenticatesTokenIfNecessary() + { + $token = new UsernamePasswordToken('username', 'password', 'provider'); + $this->tokenStorage->setToken($token); + + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $accessDecisionManager + ->expects($this->once()) + ->method('getDecision') + ->with($this->identicalTo($token), ['foo']) + ->willReturn(AccessDecision::createGranted()); + + $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $accessDecisionManager, false, false); + + // first run the token has not been re-authenticated yet, after isGranted is called, it should be equal + $this->assertSame($token, $this->tokenStorage->getToken()); + $this->assertTrue($authorizationChecker->isGranted('foo')); + } + /** * @group legacy */ @@ -89,7 +113,10 @@ public function testLegacyVoteWithoutAuthenticationToken() $authorizationChecker->isGranted('ROLE_FOO'); } - public function testVoteWithoutAuthenticationToken() + /** + * @group legacy + */ + public function testLegacyVoteWithoutAuthenticationToken2() { $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $this->accessDecisionManager, false, false); @@ -98,14 +125,33 @@ public function testVoteWithoutAuthenticationToken() ->method('decide') ->with($this->isInstanceOf(NullToken::class)) ->willReturn(true); + $this->expectDeprecation('Since symfony/security-core 5.4: Not implementing "%s::getDecision()" method is deprecated, and would be required in 6.0.'); + $this->assertTrue($authorizationChecker->isGranted('ANONYMOUS')); + } + + public function testVoteWithoutAuthenticationToken() + { + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $accessDecisionManager + ->expects($this->once()) + ->method('getDecision') + ->with($this->isInstanceOf(NullToken::class), ['ANONYMOUS']) + ->willReturn(AccessDecision::createGranted()); + $authorizationChecker = new AuthorizationChecker( + $this->tokenStorage, + $accessDecisionManager, + false, + false + ); $this->assertTrue($authorizationChecker->isGranted('ANONYMOUS')); } /** * @dataProvider isGrantedProvider + * @group legacy */ - public function testIsGranted($decide) + public function testLegacyIsGranted($decide) { $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); @@ -114,15 +160,43 @@ public function testIsGranted($decide) ->method('decide') ->willReturn($decide); $this->tokenStorage->setToken($token); + $this->expectDeprecation('Since symfony/security-core 5.4: Not implementing "%s::getDecision()" method is deprecated, and would be required in 6.0.'); $this->assertSame($decide, $this->authorizationChecker->isGranted('ROLE_FOO')); } + /** + * @dataProvider isGrantedProvider + */ + public function testIsGranted($decide) + { + $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $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, + false, + false + ); + + $this->assertSame($decide, $authorizationChecker->isGranted('ROLE_FOO')); + } + public function isGrantedProvider() { return [[true], [false]]; } - public function testIsGrantedWithObjectAttribute() + /** + * @group legacy + */ + public function testLegacyIsGrantedWithObjectAttribute() { $attribute = new \stdClass(); @@ -134,6 +208,29 @@ public function testIsGrantedWithObjectAttribute() ->with($this->identicalTo($token), $this->identicalTo([$attribute])) ->willReturn(true); $this->tokenStorage->setToken($token); + $this->expectDeprecation('Since symfony/security-core 5.4: Not implementing "%s::getDecision()" method is deprecated, and would be required in 6.0.'); $this->assertTrue($this->authorizationChecker->isGranted($attribute)); } + + public function testIsGrantedWithObjectAttribute() + { + $attribute = new \stdClass(); + + $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $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, + false, + false + ); + $this->assertTrue($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 390487d42930a..47035c804033a 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Tests\Authorization; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; @@ -22,6 +23,8 @@ class TraceableAccessDecisionManagerTest extends TestCase { + use ExpectDeprecationTrait; + /** * @dataProvider provideObjectsAndLogs */ diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index 75db00dbfb3d4..35dae02c41e76 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Http\Tests\Firewall; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; @@ -35,6 +36,11 @@ class AccessListenerTest extends TestCase { + use ExpectDeprecationTrait; + + /** + * @group legacy + */ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() { $this->expectException(AccessDeniedException::class); @@ -55,6 +61,13 @@ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() ->willReturn(true) ; + $notAuthenticatedToken = $this->createMock(TokenInterface::class); + $notAuthenticatedToken + ->expects($this->any()) + ->method('isAuthenticated') + ->willReturn(false) + ; + $tokenStorage = $this->createMock(TokenStorageInterface::class); $tokenStorage ->expects($this->any()) @@ -70,11 +83,19 @@ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() ->willReturn(AccessDecision::createDenied()) ; +// $authManager = $this->createMock(AuthenticationManagerInterface::class); +// $authManager +// ->expects($this->once()) +// ->method('authenticate') +// ->with($this->equalTo($notAuthenticatedToken)) +// ->willReturn($token) +// ; + $listener = new AccessListener( $tokenStorage, $accessDecisionManager, $accessMap, - $this->createMock(AuthenticationManagerInterface::class) + false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -161,25 +182,12 @@ public function testHandleWhenTheTokenIsNotAuthenticated() ->willReturn(true) ; - $authManager = $this->createMock(AuthenticationManagerInterface::class); - $authManager - ->expects($this->once()) - ->method('authenticate') - ->with($this->equalTo($notAuthenticatedToken)) - ->willReturn($authenticatedToken) - ; - $tokenStorage = $this->createMock(TokenStorageInterface::class); $tokenStorage ->expects($this->any()) ->method('getToken') ->willReturn($notAuthenticatedToken) ; - $tokenStorage - ->expects($this->once()) - ->method('setToken') - ->with($this->equalTo($authenticatedToken)) - ; $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager @@ -193,7 +201,6 @@ public function testHandleWhenTheTokenIsNotAuthenticated() $tokenStorage, $accessDecisionManager, $accessMap, - $authManager, false ); @@ -299,7 +306,7 @@ public function testHandleWhenThereIsNoAccessMapEntryMatchingTheRequest() $tokenStorage, $this->createMock(AccessDecisionManager::class), $accessMap, - $this->createMock(AuthenticationManagerInterface::class) + false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -327,7 +334,7 @@ public function testHandleWhenAccessMapReturnsEmptyAttributes() $tokenStorage, $this->createMock(AccessDecisionManager::class), $accessMap, - $this->createMock(AuthenticationManagerInterface::class) + false ); $event = new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST); @@ -359,7 +366,7 @@ public function testHandleWhenTheSecurityTokenStorageHasNoToken() $tokenStorage, $this->createMock(AccessDecisionManager::class), $accessMap, - $this->createMock(AuthenticationManagerInterface::class) + true ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); @@ -388,7 +395,6 @@ public function testHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptionOnTok $tokenStorage, $accessDecisionManager, $accessMap, - $this->createMock(AuthenticationManagerInterface::class), false ); @@ -450,7 +456,6 @@ public function testHandleWhenPublicAccessIsAllowedAndExceptionOnTokenIsFalse() $tokenStorage, $accessDecisionManager, $accessMap, - $this->createMock(AuthenticationManagerInterface::class), false ); @@ -481,7 +486,6 @@ public function testHandleWhenPublicAccessWhileAuthenticated() $tokenStorage, $accessDecisionManager, $accessMap, - $this->createMock(AuthenticationManagerInterface::class), false ); @@ -517,7 +521,7 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() $tokenStorage, $accessDecisionManager, $accessMap, - $this->createMock(AuthenticationManagerInterface::class) + false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); From 504ec2675b1840646426f177cc99de60d222cb2a Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Sun, 26 Sep 2021 21:44:09 +0700 Subject: [PATCH 32/35] Merge test code with latest 5.4 --- .../Controller/AbstractController.php | 2 +- .../AccessDecisionManagerInterface.php | 4 ++-- .../Tests/Firewall/AccessListenerTest.php | 23 +++++++++---------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index 74f97124b04f3..bcb1f6930c769 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php @@ -424,7 +424,7 @@ protected function getUser() return null; } - // @deprecated since Symfony 5.4, $user will always be a UserInterface instance + // @deprecated since 5.4, $user will always be a UserInterface instance if (!\is_object($user = $token->getUser())) { // e.g. anonymous authentication return null; diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index b70e3ca193ae1..f5bea7c90d1af 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -25,8 +25,8 @@ interface AccessDecisionManagerInterface /** * Decides whether the access is possible or not. * - * @param array $attributes An array of attributes associated with the method being invoked - * @param object $object The object to secure + * @param array $attributes An array of attributes associated with the method being invoked + * @param mixed $object The object to secure * * @return bool * diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php index 35dae02c41e76..2f508e16eae23 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -344,29 +344,28 @@ public function testHandleWhenAccessMapReturnsEmptyAttributes() public function testHandleWhenTheSecurityTokenStorageHasNoToken() { - $this->expectException(AuthenticationCredentialsNotFoundException::class); - $tokenStorage = $this->createMock(TokenStorageInterface::class); - $tokenStorage - ->expects($this->any()) - ->method('getToken') - ->willReturn(null) - ; - + $this->expectException(AccessDeniedException::class); + $tokenStorage = new TokenStorage(); $request = new Request(); $accessMap = $this->createMock(AccessMapInterface::class); - $accessMap - ->expects($this->any()) + $accessMap->expects($this->any()) ->method('getPatterns') ->with($this->equalTo($request)) ->willReturn([['foo' => 'bar'], null]) ; + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); + $accessDecisionManager->expects($this->once()) + ->method('getDecision') + ->with($this->isInstanceOf(NullToken::class)) + ->willReturn(AccessDecision::createDenied()); + $listener = new AccessListener( $tokenStorage, - $this->createMock(AccessDecisionManager::class), + $accessDecisionManager, $accessMap, - true + false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); From 9d59fdd7329d57f15a7993e6f8ba39b9c1e73eaa Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Mon, 27 Sep 2021 09:40:10 +0700 Subject: [PATCH 33/35] Allow getAccessDecision to return null in AccessDeniedException --- .../Core/Exception/AccessDeniedException.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index cceb17bfb55dc..21d9098b5c180 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -23,8 +23,7 @@ class AccessDeniedException extends RuntimeException { private $attributes = []; private $subject; - /** @var AccessDecision */ - private $accessDecision; + private ?AccessDecision $accessDecision; public function __construct(string $message = 'Access Denied.', \Throwable $previous = null) { @@ -44,7 +43,7 @@ public function getAttributes() */ public function setAttributes($attributes) { - $this->attributes = (array) $attributes; + $this->attributes = (array)$attributes; } /** @@ -77,10 +76,15 @@ public function setAccessDecision(AccessDecision $accessDecision) return $vote->getMessage(); }, $accessDecision->getDeniedVotes()); - $this->message .= ' '.rtrim(' '.implode(' ', $messages)); + $this->message .= ' ' . rtrim(' ' . implode(' ', $messages)); } - public function getAccessDecision(): AccessDecision + /** + * Gets the access decision + * + * @return AccessDecision|null + */ + public function getAccessDecision(): ?AccessDecision { return $this->accessDecision; } From 93c0c10174923cd09735e376d7c7b6de76d826a5 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Sun, 17 Oct 2021 20:12:35 +0700 Subject: [PATCH 34/35] Fix coding standards --- .../FrameworkBundle/Controller/AbstractController.php | 2 +- .../Security/Core/Exception/AccessDeniedException.php | 6 ++---- .../Core/Tests/Authorization/AuthorizationCheckerTest.php | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index e89bd6dbffa3f..a2d2b91aa0a41 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php @@ -424,7 +424,7 @@ protected function getUser() return null; } - // @deprecated since 5.4, $user will always be a UserInterface instance + // @deprecated since Symfony 5.4, $user will always be a UserInterface instance if (!\is_object($user = $token->getUser())) { // e.g. anonymous authentication return null; diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index 21d9098b5c180..b3485f1e62aba 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -43,7 +43,7 @@ public function getAttributes() */ public function setAttributes($attributes) { - $this->attributes = (array)$attributes; + $this->attributes = (array) $attributes; } /** @@ -76,13 +76,11 @@ public function setAccessDecision(AccessDecision $accessDecision) return $vote->getMessage(); }, $accessDecision->getDeniedVotes()); - $this->message .= ' ' . rtrim(' ' . implode(' ', $messages)); + $this->message .= ' '.rtrim(' '.implode(' ', $messages)); } /** * Gets the access decision - * - * @return AccessDecision|null */ public function getAccessDecision(): ?AccessDecision { diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 32c87a7f5b717..0f3b05666d177 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -94,7 +94,7 @@ public function testVoteAuthenticatesTokenIfNecessary() ->with($this->identicalTo($token), ['foo']) ->willReturn(AccessDecision::createGranted()); - $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $accessDecisionManager, false, false); + $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $accessDecisionManager, false, false); // first run the token has not been re-authenticated yet, after isGranted is called, it should be equal $this->assertSame($token, $this->tokenStorage->getToken()); From 5ebf1b58ec304a4e57e8df17fff25b7c582054e5 Mon Sep 17 00:00:00 2001 From: Vu Nguyen Date: Sun, 17 Oct 2021 20:14:44 +0700 Subject: [PATCH 35/35] Add missing dot to comply with standards --- .../Component/Security/Core/Exception/AccessDeniedException.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index b3485f1e62aba..f9338b0558d0e 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -80,7 +80,7 @@ public function setAccessDecision(AccessDecision $accessDecision) } /** - * Gets the access decision + * Gets the access decision. */ public function getAccessDecision(): ?AccessDecision {