diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index 40b394607cdf2..a2d2b91aa0a41 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; } @@ -411,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/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/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig index b332ba5ddb596..d03161292973b 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig @@ -126,36 +126,36 @@ - - - - + + + + - - - + + - + {% if not collector.authenticated and collector.roles is empty %} +

User is not authenticated probably because they have no roles.

+ {% endif %} + + - {% if collector.supportsRoleHierarchy %} + {% if collector.supportsRoleHierarchy %} - {% endif %} + {% endif %} - {% if collector.token %} + {% if collector.token %} - {% endif %} + {% endif %}
PropertyValue
PropertyValue
Roles - {{ collector.roles is empty ? 'none' : profiler_dump(collector.roles, maxDepth=1) }} +
Roles + {{ collector.roles is empty ? 'none' : profiler_dump(collector.roles, maxDepth=1) }} - {% if not collector.authenticated and collector.roles is empty %} -

User is not authenticated probably because they have no roles.

- {% endif %} -
Inherited Roles {{ collector.inheritedRoles is empty ? 'none' : profiler_dump(collector.inheritedRoles, maxDepth=1) }}
Token {{ profiler_dump(collector.token) }}
{% elseif collector.enabled %} @@ -195,47 +195,47 @@

Configuration

- - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + {% if collector.authenticatorManagerEnabled %} - - - - - - - - - - - - - - - - - - + + + {% else %} - - + + - {% if collector.authenticatorManagerEnabled %} - - - - - {% else %} - - - - - {% endif %} + {% endif %}
KeyValue
KeyValue
provider{{ collector.firewall.provider ?: '(none)' }}
context{{ collector.firewall.context ?: '(none)' }}
entry_point{{ collector.firewall.entry_point ?: '(none)' }}
user_checker{{ collector.firewall.user_checker ?: '(none)' }}
access_denied_handler{{ collector.firewall.access_denied_handler ?: '(none)' }}
access_denied_url{{ collector.firewall.access_denied_url ?: '(none)' }}
provider{{ collector.firewall.provider ?: '(none)' }}
context{{ collector.firewall.context ?: '(none)' }}
entry_point{{ collector.firewall.entry_point ?: '(none)' }}
user_checker{{ collector.firewall.user_checker ?: '(none)' }}
access_denied_handler{{ collector.firewall.access_denied_handler ?: '(none)' }}authenticators{{ collector.firewall.authenticators is empty ? '(none)' : profiler_dump(collector.firewall.authenticators, maxDepth=1) }}
access_denied_url{{ collector.firewall.access_denied_url ?: '(none)' }}listeners{{ collector.firewall.listeners is empty ? '(none)' : profiler_dump(collector.firewall.listeners, maxDepth=1) }}
authenticators{{ collector.firewall.authenticators is empty ? '(none)' : profiler_dump(collector.firewall.authenticators, maxDepth=1) }}
listeners{{ collector.firewall.listeners is empty ? '(none)' : profiler_dump(collector.firewall.listeners, maxDepth=1) }}
{% endif %} @@ -344,104 +344,105 @@ - - - - + + + + - {% for voter in collector.voters %} - - - - - {% endfor %} + {% for voter in collector.voters %} + + + + + {% endfor %}
#Voter class
#Voter class
{{ loop.index }}{{ profiler_dump(voter) }}
{{ loop.index }}{{ profiler_dump(voter) }}
{% endif %} {% if collector.accessDecisionLog|default([]) is not empty %} -

Access decision log

- - - - - - - - - - - - - - - - - - {% for decision in collector.accessDecisionLog %} - - - - - - - - - + + + {% endfor %} + +
#ResultAttributesObject
{{ loop.index }} - {{ decision.result - ? 'GRANTED' - : 'DENIED' - }} - - {% if decision.attributes|length == 1 %} - {% set attribute = decision.attributes|first %} - {% if attribute.expression is defined %} - Expression: 
{{ attribute.expression }}
- {% elseif attribute.type == 'string' %} - {{ attribute }} - {% else %} - {{ profiler_dump(attribute) }} - {% endif %} - {% else %} - {{ profiler_dump(decision.attributes) }} - {% endif %} - 		{{ profiler_dump(decision.seek('object')) }}
- {% if decision.voter_details is not empty %} - {% set voter_details_id = 'voter-details-' ~ loop.index %} -
- - - {% for voter_detail in decision.voter_details %} - - - {% if collector.voterStrategy == constant('Symfony\\Component\\Security\\Core\\Authorization\\AccessDecisionManager::STRATEGY_UNANIMOUS') %} +

Access decision log

+ +
{{ profiler_dump(voter_detail['class']) }}
+ + + + + + + + + + + + + + + + {% for decision in collector.accessDecisionLog %} + + + + + + + + + - - {% endfor %} - -
#ResultAttributesObject
{{ loop.index }} + {{ decision.result + ? 'GRANTED' + : 'DENIED' + }} + + {% if decision.attributes|length == 1 %} + {% set attribute = decision.attributes|first %} + {% if attribute.expression is defined %} + Expression: 
{{ attribute.expression }}
+ {% elseif attribute.type == 'string' %} + {{ attribute }} + {% else %} + {{ profiler_dump(attribute) }} + {% endif %} + {% else %} + {{ profiler_dump(decision.attributes) }} + {% endif %} + 		{{ profiler_dump(decision.seek('object')) }}
+ {% if decision.voter_details is not empty %} + {% set voter_details_id = 'voter-details-' ~ loop.index %} +
+ + + {% for voter_detail in decision.voter_details %} + + + {% if collector.voterStrategy == constant('Symfony\\Component\\Security\\Core\\Authorization\\AccessDecisionManager::STRATEGY_UNANIMOUS') %} + {% endif %} + - - {% endfor %} - -
{{ profiler_dump(voter_detail['class']) }}attribute {{ voter_detail['attributes'][0] }} + {% if voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_GRANTED') %} + ACCESS GRANTED + {% elseif voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_ABSTAIN') %} + ACCESS ABSTAIN + {% elseif voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %} + ACCESS DENIED + {% else %} + unknown ({{ voter_detail['vote'].access }}) {% endif %} - - {% if voter_detail['vote'] == 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') %} - ACCESS ABSTAIN - {% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %} - ACCESS DENIED - {% else %} - unknown ({{ voter_detail['vote'] }}) - {% endif %} -
-
- Show voter details - {% endif %} -
-
+ 		{{ voter_detail['vote'].message|default('~') }}
+ + Show voter details + {% endif %} + + + {% endfor %} + + + {% endif %} 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..6848d6ab93535 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -0,0 +1,118 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authorization; + +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\Voter; +use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; + +/** + * An AccessDecision is returned by an AccessDecisionManager and contains the access verdict and all the related votes. + * + * @author Dany Maillard + */ +final class AccessDecision +{ + /** @var int One of the VoterInterface::ACCESS_* constants */ + private $access; + + /** @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; + } + + public function getAccess(): int + { + return $this->access; + } + + public function isGranted(): bool + { + return VoterInterface::ACCESS_GRANTED === $this->access; + } + + public function isAbstain(): bool + { + return VoterInterface::ACCESS_ABSTAIN === $this->access; + } + + public function isDenied(): bool + { + return VoterInterface::ACCESS_DENIED === $this->access; + } + + /** + * @param Vote[] $votes + */ + public static function createGranted(array $votes = []): self + { + return new self(VoterInterface::ACCESS_GRANTED, $votes); + } + + /** + * @param Vote[] $votes + */ + public static function createDenied(array $votes = []): self + { + return new self(VoterInterface::ACCESS_DENIED, $votes); + } + + /** + * @return Vote[] + */ + public function getVotes(): array + { + return $this->votes; + } + + /** + * @return Vote[] + */ + public function getGrantedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_GRANTED); + } + + /** + * @return Vote[] + */ + public function getAbstainedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_ABSTAIN); + } + + /** + * @return Vote[] + */ + public function getDeniedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_DENIED); + } + + /** + * @return Vote[] + */ + private function getVotesByAccess(int $access): array + { + return array_filter($this->votes, static function (Vote $vote) use ($access): bool { + return $vote->getAccess() === $access; + }); + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 82f9e0ae827d3..2e9d9c2ae052f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.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; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; @@ -54,13 +55,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 Symfony 5.4, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/) { + 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); // Special case for AccessListener, do not remove the right side of the condition before 6.0 @@ -68,7 +78,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 +87,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 +128,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 +171,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 +210,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.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); + } + + return null; } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index b807861109a9e..f5bea7c90d1af 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -17,6 +17,8 @@ * AccessDecisionManagerInterface makes authorization decisions. * * @author Fabien Potencier + * + * @method AccessDecision getDecision(TokenInterface $token, array $attributes, object $object = null) */ interface AccessDecisionManagerInterface { @@ -27,6 +29,8 @@ interface AccessDecisionManagerInterface * @param mixed $object The object to secure * * @return bool + * + * @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/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index 2f26dff0f9f56..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; @@ -67,25 +67,27 @@ public function __construct(TokenStorageInterface $tokenStorage, /*AccessDecisio */ 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->authenticationManager && ($this->alwaysAuthenticate || !$token->isAuthenticated())) { + $this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token)); + } + + if (!method_exists($this->accessDecisionManager, 'getDecision')) { + 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(); } - return $this->accessDecisionManager->decide($token, [$attribute], $subject); + 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..7e447567b4205 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 Symfony 5.4, use {@see getDecision()} instead. */ public function decide(TokenInterface $token, array $attributes, $object = null/*, bool $allowMultipleAttributes = false*/): bool { + trigger_deprecation('symfony/security-core', '5.4', '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.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); + } + $currentLogIndex = \count($this->currentLog) - 1; $this->currentLog[$currentLogIndex]['voterDetails'][] = [ 'voter' => $voter, diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php index 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..2ee2396612b57 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -33,11 +33,17 @@ 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) { - $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.4', '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 = new Vote($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..a0df7b2e5e7b1 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -0,0 +1,91 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authorization\Voter; + +/** + * A Vote is returned by a Voter and contains the access (granted, abstain or denied). + * It can also contain a message explaining the vote decision. + * + * @author Dany Maillard + */ +final class Vote +{ + /** @var int One of the VoterInterface::ACCESS_* constants */ + private $access; + private $message; + private $context; + + /** + * @param int $access One of the VoterInterface::ACCESS_* constants + */ + public function __construct(int $access, string $message = '', array $context = []) + { + $this->access = $access; + $this->message = $message; + $this->context = $context; + } + + public function getAccess(): int + { + return $this->access; + } + + public function isGranted(): bool + { + return VoterInterface::ACCESS_GRANTED === $this->access; + } + + public function isAbstain(): bool + { + return VoterInterface::ACCESS_ABSTAIN === $this->access; + } + + public function isDenied(): bool + { + return VoterInterface::ACCESS_DENIED === $this->access; + } + + public static function create(int $access, string $message = '', array $context = []): self + { + return new self($access, $message, $context); + } + + public static function createGranted(string $message = '', array $context = []): self + { + return new self(VoterInterface::ACCESS_GRANTED, $message, $context); + } + + public static function createAbstain(string $message = '', array $context = []): self + { + return new self(VoterInterface::ACCESS_ABSTAIN, $message, $context); + } + + public static function createDenied(string $message = '', array $context = []): self + { + return new self(VoterInterface::ACCESS_DENIED, $message, $context); + } + + public function setMessage(string $message) + { + $this->message = $message; + } + + public function getMessage(): string + { + return $this->message; + } + + public function getContext(): array + { + return $this->context; + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index 7044d9d0f8a86..087c2b3ae9bb9 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,54 @@ 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.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(); + } + + if ($decision->isGranted()) { // grant access as soon as at least one attribute returns a positive response - return self::ACCESS_GRANTED; + return $decision; } + + $vote->setMessage($vote->getMessage().trim(' '.$decision->getMessage())); } return $vote; } + /** + * Creates a granted vote. + */ + protected function grant(string $message = '', array $context = []): Vote + { + return Vote::createGranted($message, $context); + } + + /** + * Creates an abstained vote. + */ + protected function abstain(string $message = '', array $context = []): Vote + { + return Vote::createAbstain($message, $context); + } + + /** + * Creates a denied vote. + */ + protected function deny(string $message = '', array $context = []): Vote + { + return Vote::createDenied($message, $context); + } + /** * 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 */ abstract protected function supports(string $attribute, $subject); @@ -75,7 +106,7 @@ abstract protected function supports(string $attribute, $subject); * * @param mixed $subject * - * @return bool + * @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); } 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..35272767d9a22 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.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); + } + $this->vote = $vote; } @@ -52,6 +62,11 @@ public function getAttributes(): array } public function getVote(): int + { + return $this->vote->getAccess(); + } + + public function getVoteDecision(): Vote { return $this->vote; } diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index 0e59dc4077a91..f9338b0558d0e 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,7 @@ class AccessDeniedException extends RuntimeException { private $attributes = []; private $subject; + private ?AccessDecision $accessDecision; public function __construct(string $message = 'Access Denied.', \Throwable $previous = null) { @@ -57,4 +61,29 @@ 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; + } + + $messages = array_map(static function (Vote $vote) { + return $vote->getMessage(); + }, $accessDecision->getDeniedVotes()); + + $this->message .= ' '.rtrim(' '.implode(' ', $messages)); + } + + /** + * Gets the access decision. + */ + public function getAccessDecision(): ?AccessDecision + { + return $this->accessDecision; + } } diff --git a/src/Symfony/Component/Security/Core/Security.php b/src/Symfony/Component/Security/Core/Security.php index 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..4735943c0c943 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('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']); } @@ -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/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 160b921b3075c..0f3b05666d177 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 d4f89396d3372..47035c804033a 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -12,31 +12,33 @@ 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; 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 { + use ExpectDeprecationTrait; + /** * @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,7 +50,7 @@ public function testDecideLog(array $expectedLog, array $attributes, $object, ar }) ; - $adm->decide($token, $attributes, $object); + $adm->getDecision($token, $attributes, $object); $this->assertEquals($expectedLog, $adm->getDecisionLog()); } @@ -62,115 +64,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 +215,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 +226,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 +241,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,36 +259,45 @@ public function testAccessDecisionManagerCalledByVoter() }); $token = $this->createMock(TokenInterface::class); - $sut->decide($token, ['attr1'], null); - $sut->decide($token, ['attr2'], $obj = new \stdClass()); + $sut->getDecision($token, ['attr1'], null); + $sut->getDecision($token, ['attr2'], $obj = new \stdClass()); $this->assertEquals([ [ 'attributes' => ['attr1'], 'object' => null, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr1'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => AccessDecision::createGranted([ + Vote::createGranted(), + ]), ], [ 'attributes' => ['attr2'], 'object' => null, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => Vote::createAbstain()], + ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createGranted(), + ]), ], [ 'attributes' => ['attr2'], 'object' => $obj, 'voterDetails' => [ - ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_DENIED], - ['voter' => $voter3, 'attributes' => ['attr2'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter1, 'attributes' => ['attr2'], 'vote' => Vote::createAbstain()], + ['voter' => $voter2, 'attributes' => ['attr2'], 'vote' => Vote::createDenied()], + ['voter' => $voter3, 'attributes' => ['attr2'], 'vote' => Vote::createGranted()], ], - 'result' => true, + 'result' => AccessDecision::createGranted([ + Vote::createAbstain(), + Vote::createDenied(), + Vote::createGranted(), + ]), ], ], $sut->getDecisionLog()); } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php index 32633ac68d17a..0aa0c169530ce 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 @@ -30,26 +30,26 @@ 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() { 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..9d6db8c8c51eb 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.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']); - $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..2f508e16eae23 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; @@ -22,6 +23,8 @@ 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; @@ -33,6 +36,11 @@ class AccessListenerTest extends TestCase { + use ExpectDeprecationTrait; + + /** + * @group legacy + */ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() { $this->expectException(AccessDeniedException::class); @@ -46,6 +54,69 @@ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess() ->willReturn([['foo' => 'bar'], null]) ; + $token = $this->createMock(TokenInterface::class); + $token + ->expects($this->any()) + ->method('isAuthenticated') + ->willReturn(true) + ; + + $notAuthenticatedToken = $this->createMock(TokenInterface::class); + $notAuthenticatedToken + ->expects($this->any()) + ->method('isAuthenticated') + ->willReturn(false) + ; + + $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()) + ; + +// $authManager = $this->createMock(AuthenticationManagerInterface::class); +// $authManager +// ->expects($this->once()) +// ->method('authenticate') +// ->with($this->equalTo($notAuthenticatedToken)) +// ->willReturn($token) +// ; + + $listener = new AccessListener( + $tokenStorage, + $accessDecisionManager, + $accessMap, + false + ); + + $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]) + ; + $token = new class() extends AbstractToken { public function isAuthenticated(): bool { @@ -85,10 +156,61 @@ public function getCredentials() $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) + ; + + $tokenStorage = $this->createMock(TokenStorageInterface::class); + $tokenStorage + ->expects($this->any()) + ->method('getToken') + ->willReturn($notAuthenticatedToken) + ; + + $accessDecisionManager = $this->createMock(AccessDecisionManager::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, + false + ); + + $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); + } + /** * @group legacy */ - public function testHandleWhenTheTokenIsNotAuthenticated() + public function testLegacyHandleWhenTheTokenIsNotAuthenticated() { $request = new Request(); @@ -182,7 +304,7 @@ public function testHandleWhenThereIsNoAccessMapEntryMatchingTheRequest() $listener = new AccessListener( $tokenStorage, - $this->createMock(AccessDecisionManagerInterface::class), + $this->createMock(AccessDecisionManager::class), $accessMap, false ); @@ -210,7 +332,7 @@ public function testHandleWhenAccessMapReturnsEmptyAttributes() $listener = new AccessListener( $tokenStorage, - $this->createMock(AccessDecisionManagerInterface::class), + $this->createMock(AccessDecisionManager::class), $accessMap, false ); @@ -220,40 +342,68 @@ public function testHandleWhenAccessMapReturnsEmptyAttributes() $listener(new LazyResponseEvent($event)); } - /** - * @group legacy - */ - public function testLegacyHandleWhenTheSecurityTokenStorageHasNoToken() + 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()) + ->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, + $accessDecisionManager, + $accessMap, + false + ); + + $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); + } + + public function testHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptionOnTokenIsFalse() + { + $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(AccessDecisionManagerInterface::class), + $accessDecisionManager, $accessMap, - $this->createMock(AuthenticationManagerInterface::class) + false ); $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); } - public function testHandleWhenTheSecurityTokenStorageHasNoToken() + /** + * @group legacy + */ + public function testLegacyHandleWhenTheSecurityTokenStorageHasNoTokenAndExceptionOnTokenIsFalse() { $this->expectException(AccessDeniedException::class); $tokenStorage = new TokenStorage(); @@ -276,13 +426,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(); @@ -294,11 +445,11 @@ public function testHandleWhenPublicAccessIsAllowed() ->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::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, @@ -312,7 +463,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(); @@ -324,11 +475,11 @@ public function testHandleWhenPublicAccessWhileAuthenticated() ->willReturn([[AuthenticatedVoter::PUBLIC_ACCESS], null]) ; - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::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, @@ -357,12 +508,12 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd() $tokenStorage = new TokenStorage(); $tokenStorage->setToken($authenticatedToken); - $accessDecisionManager = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager = $this->createMock(AccessDecisionManager::class); $accessDecisionManager ->expects($this->once()) - ->method('decide') - ->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), true) - ->willReturn(true) + ->method('getDecision') + ->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request)) + ->willReturn(AccessDecision::createGranted()) ; $listener = new AccessListener( diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php index 0338af0017e8a..7d648f19ba03b 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\UserCheckerInterface; use Symfony\Component\Security\Http\Event\SwitchUserEvent; use Symfony\Component\Security\Http\Firewall\SwitchUserListener; @@ -49,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); } @@ -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);