diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index 4818d379312a3..29ffc35757ca5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php @@ -232,6 +232,7 @@ protected function denyAccessUnlessGranted($attributes, $subject = null, string $exception = $this->createAccessDeniedException($message); $exception->setAttributes($attributes); $exception->setSubject($subject); + $exception->setAccessDecision($this->container->get('security.authorization_checker')->getLastAccessDecision()); throw $exception; } diff --git a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig index 6b0819513fa04..f11be0e1bf3bd 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig @@ -346,16 +346,17 @@ attribute {{ voter_detail['attributes'][0] }} {% endif %} - {% if voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_GRANTED') %} + {% if voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_GRANTED') %} ACCESS GRANTED - {% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_ABSTAIN') %} + {% elseif voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_ABSTAIN') %} ACCESS ABSTAIN - {% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %} + {% elseif voter_detail['vote'].access == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %} ACCESS DENIED {% else %} - unknown ({{ voter_detail['vote'] }}) + unknown ({{ voter_detail['vote'].access }}) {% endif %} + {{ voter_detail['vote'].reason|default('~') }} {% endfor %} diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php new file mode 100644 index 0000000000000..87ed37217a6db --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -0,0 +1,93 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authorization; + +use Symfony\Component\Security\Core\Authorization\Voter\AccessTrait; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\Voter; +use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; + +/** + * An AccessDecision is returned by an AccessDecisionManager and contains the access verdict and all the related votes. + * + * @author Dany Maillard + */ +final class AccessDecision +{ + use AccessTrait; + + /** @var Vote[] */ + private $votes = []; + + /** + * @param int $access One of the VoterInterface::ACCESS_* constants + * @param Vote[] $votes + */ + private function __construct(int $access, array $votes = []) + { + $this->access = $access; + $this->votes = $votes; + } + + /** + * @param Vote[] $votes + */ + public static function createGranted(array $votes = []): self + { + return new self(VoterInterface::ACCESS_GRANTED, $votes); + } + + /** + * @param Vote[] $votes + */ + public static function createDenied(array $votes = []): self + { + return new self(VoterInterface::ACCESS_DENIED, $votes); + } + + /** + * @return Vote[] + */ + public function getVotes(): array + { + return $this->votes; + } + + /** + * @return Vote[] + */ + public function getGrantedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_GRANTED); + } + + /** + * @return Vote[] + */ + public function getAbstainedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_ABSTAIN); + } + + /** + * @return Vote[] + */ + public function getDeniedVotes(): array + { + return $this->getVotesByAccess(Voter::ACCESS_DENIED); + } + + private function getVotesByAccess(int $access): array + { + return array_filter($this->votes, function (Vote $vote) use ($access) { 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 8da5b06818ada..1c9ef362d1535 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; @@ -72,26 +73,27 @@ 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); + $votes[] = $vote = $this->vote($voter, $token, $object, $attributes); - if (VoterInterface::ACCESS_GRANTED === $result) { - return true; + if ($vote->isGranted()) { + return AccessDecision::createGranted($votes); } - if (VoterInterface::ACCESS_DENIED === $result) { + if ($vote->isDenied()) { ++$deny; } } if ($deny > 0) { - return false; + return AccessDecision::createDenied($votes); } - return $this->allowIfAllAbstainDecisions; + return $this->decideIfAllAbstainDecisions(); } /** @@ -108,33 +110,37 @@ 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); + $votes[] = $vote = $this->vote($voter, $token, $object, $attributes); - if (VoterInterface::ACCESS_GRANTED === $result) { + if ($vote->isGranted()) { ++$grant; - } elseif (VoterInterface::ACCESS_DENIED === $result) { + } elseif ($vote->isDenied()) { ++$deny; } } 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() + : AccessDecision::createDenied() + ; } - return $this->allowIfAllAbstainDecisions; + return $this->decideIfAllAbstainDecisions(); } /** @@ -143,18 +149,19 @@ 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]); + $votes[] = $vote = $this->vote($voter, $token, $object, [$attribute]); - if (VoterInterface::ACCESS_DENIED === $result) { - return false; + if ($vote->isDenied()) { + return AccessDecision::createDenied($votes); } - if (VoterInterface::ACCESS_GRANTED === $result) { + if ($vote->isGranted()) { ++$grant; } } @@ -162,10 +169,10 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje // no deny votes if ($grant > 0) { - return true; + return AccessDecision::createGranted($votes); } - return $this->allowIfAllAbstainDecisions; + return $this->decideIfAllAbstainDecisions(); } /** @@ -191,4 +198,22 @@ private function decidePriority(TokenInterface $token, array $attributes, $objec return $this->allowIfAllAbstainDecisions; } + + private function decideIfAllAbstainDecisions(): AccessDecision + { + return $this->allowIfAllAbstainDecisions + ? AccessDecision::createGranted() + : AccessDecision::createDenied() + ; + } + + private function vote(VoterInterface $voter, TokenInterface $token, $subject, array $attributes): Vote + { + if (\is_int($vote = $voter->vote($token, $subject, $attributes))) { + trigger_deprecation('symfony/security', 5.1, 'Returning an int from the "%s::vote()" method is deprecated. Return a "%s" object instead.', \get_class($this->voter), Vote::class); + $vote = Vote::create($vote); + } + + return $vote; + } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index 97eb4f9d41a55..11d01979c8b5e 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -26,7 +26,7 @@ interface AccessDecisionManagerInterface * @param array $attributes An array of attributes associated with the method being invoked * @param object $object The object to secure * - * @return bool true if the access is granted, false otherwise + * @return bool|AccessDecision Returning a boolean is deprecated since Symfony 5.1. Return an AccessDecision object 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 9036bba029d2e..790f76b728316 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -29,6 +29,8 @@ class AuthorizationChecker implements AuthorizationCheckerInterface private $accessDecisionManager; private $authenticationManager; private $alwaysAuthenticate; + /** @var AccessDecision */ + private $lastAccessDecision; public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, AccessDecisionManagerInterface $accessDecisionManager, bool $alwaysAuthenticate = false) { @@ -53,6 +55,19 @@ final public function isGranted($attribute, $subject = null): bool $this->tokenStorage->setToken($token = $this->authenticationManager->authenticate($token)); } - return $this->accessDecisionManager->decide($token, [$attribute], $subject); + $this->lastAccessDecision = $this->accessDecisionManager->decide($token, [$attribute], $subject); + + if (\is_bool($this->lastAccessDecision)) { + trigger_deprecation('symfony/security', 5.1, 'Returning a boolean from the "%s::decide()" method is deprecated. Return an "%s" object instead', \get_class($this->accessDecisionManager), AccessDecision::class); + + return $this->lastAccessDecision; + } + + return $this->lastAccessDecision->isGranted(); + } + + public function getLastAccessDecision(): AccessDecision + { + return $this->lastAccessDecision; } } diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index 7690b3e2264bc..e6d6931eebd28 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; /** @@ -48,7 +49,7 @@ public function __construct(AccessDecisionManagerInterface $manager) /** * {@inheritdoc} */ - public function decide(TokenInterface $token, array $attributes, $object = null): bool + public function decide(TokenInterface $token, array $attributes, $object = null) { $currentDecisionLog = [ 'attributes' => $attributes, @@ -71,9 +72,9 @@ 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 Vote $vote vote of the voter */ - public function addVoterVote(VoterInterface $voter, array $attributes, int $vote) + public function addVoterVote(VoterInterface $voter, array $attributes, Vote $vote) { $currentLogIndex = \count($this->currentLog) - 1; $this->currentLog[$currentLogIndex]['voterDetails'][] = [ diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php new file mode 100644 index 0000000000000..c008d7cf0c4d0 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/AccessTrait.php @@ -0,0 +1,41 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authorization\Voter; + +/** + * @author Dany Maillard + */ +trait AccessTrait +{ + /** @var int One of the VoterInterface::ACCESS_* constants */ + protected $access; + + public function getAccess(): int + { + return $this->access; + } + + public function isGranted(): bool + { + return VoterInterface::ACCESS_GRANTED === $this->access; + } + + public function isAbstain(): bool + { + return VoterInterface::ACCESS_ABSTAIN === $this->access; + } + + public function isDenied(): bool + { + return VoterInterface::ACCESS_DENIED === $this->access; + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index 48f7651256710..210b585851b6d 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -33,9 +33,9 @@ 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); + $result = \is_int($vote = $this->voter->vote($token, $subject, $attributes)) ? Vote::create($vote) : $vote; $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $result), 'debug.security.authorization.vote'); diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php new file mode 100644 index 0000000000000..6a7e734b50caf --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -0,0 +1,71 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Authorization\Voter; + +/** + * A Vote is returned by a Voter and contains the access (granted, abstain or denied). It can also contains a reason + * explaining the why of the access which has been decided. + * + * @author Dany Maillard + */ +final class Vote +{ + use AccessTrait; + + private $reason; + private $parameters; + + /** + * @param int $access One of the VoterInterface::ACCESS_* constants + */ + private function __construct(int $access, string $reason = '', array $parameters = []) + { + $this->access = $access; + $this->reason = $reason; + $this->parameters = $parameters; + } + + public static function create(int $access, string $reason = '', array $parameters = []): self + { + return new self($access, $reason, $parameters); + } + + public static function createGranted(string $reason, array $parameters = []): self + { + return new self(VoterInterface::ACCESS_GRANTED, $reason, $parameters); + } + + public static function createAbstrain(string $reason, array $parameters = []): self + { + return new self(VoterInterface::ACCESS_ABSTAIN, $reason, $parameters); + } + + public static function createDenied(string $reason, array $parameters = []): self + { + return new self(VoterInterface::ACCESS_DENIED, $reason, $parameters); + } + + public function merge(self $vote): void + { + $this->reason .= trim(' '.$vote->getReason()); + } + + public function getReason(): string + { + return $this->reason; + } + + public function getParameters(): array + { + return $this->parameters; + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index 1ef95d0e979dd..3556113700fcd 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) { if (!$this->supports($attribute, $subject)) { @@ -35,17 +36,44 @@ 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)) { + $v = \is_bool($v = $this->voteOnAttribute($attribute, $subject, $token)) ? Vote::create($v) : $v; // BC layer + if ($v->isGranted()) { // grant access as soon as at least one attribute returns a positive response - return self::ACCESS_GRANTED; + return $v; + } else { + $vote->merge($v); } } return $vote; } + /** + * Creates a granted vote. + */ + public function grant(string $reason = '', array $parameters = []): Vote + { + return Vote::createGranted($reason, $parameters); + } + + /** + * Creates an abstained vote. + */ + public function abstain(string $reason = '', array $parameters = []): Vote + { + return Vote::createAbstrain($reason, $parameters); + } + + /** + * Creates a denied vote. + */ + public function deny(string $reason = '', array $parameters = []): Vote + { + return Vote::createDenied($reason, $parameters); + } + /** * Determines if the attribute and subject are supported by this voter. * @@ -62,7 +90,7 @@ abstract protected function supports(string $attribute, $subject); * * @param mixed $subject * - * @return bool + * @return bool|Vote Returning a boolean is deprecated since Symfony 5.1. Return a Vote object instead. */ abstract protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token); } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php index 76c1968f5e2e1..8bd7d352ef548 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..aaa56f96eb2b4 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,7 +29,7 @@ final class VoteEvent extends Event private $attributes; private $vote; - public function __construct(VoterInterface $voter, $subject, array $attributes, int $vote) + public function __construct(VoterInterface $voter, $subject, array $attributes, Vote $vote) { $this->voter = $voter; $this->subject = $subject; @@ -51,7 +52,7 @@ public function getAttributes(): array return $this->attributes; } - public function getVote(): int + public function getVote(): Vote { return $this->vote; } diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index 0e59dc4077a91..881431e9abf6c 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -11,6 +11,9 @@ namespace Symfony\Component\Security\Core\Exception; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; + /** * AccessDeniedException is thrown when the account has not the required role. * @@ -20,6 +23,8 @@ class AccessDeniedException extends RuntimeException { private $attributes = []; private $subject; + /** @var AccessDecision */ + private $accessDecision; public function __construct(string $message = 'Access Denied.', \Throwable $previous = null) { @@ -57,4 +62,19 @@ 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; + $reasons = array_map(function (Vote $vote) { return $vote->getReason(); }, $this->accessDecision->getDeniedVotes()); + $this->message .= rtrim(' '.implode(' ', $reasons)); + } + + public function getAccessDecision(): AccessDecision + { + return $this->accessDecision; + } }