diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index af453619b5ab8..8a8742821a2c6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php @@ -35,6 +35,7 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\RouterInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\User\UserInterface; @@ -202,6 +203,23 @@ protected function isGranted(mixed $attribute, mixed $subject = null): bool return $this->container->get('security.authorization_checker')->isGranted($attribute, $subject); } + /** + * Checks if the attribute is granted against the current authentication token and optionally supplied subject. + * + * @throws \LogicException + */ + protected function getAccessDecision(mixed $attribute, mixed $subject = null): AccessDecision + { + 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".'); + } + + $accessDecision = null; + $decision = $this->container->get('security.authorization_checker')->isGranted($attribute, $subject, $accessDecision); + + return null === $accessDecision ? new AccessDecision($decision) : $accessDecision; + } + /** * Throws an exception unless the attribute is granted against the current authentication token and optionally * supplied subject. @@ -210,11 +228,13 @@ protected function isGranted(mixed $attribute, mixed $subject = null): bool */ protected function denyAccessUnlessGranted(mixed $attribute, mixed $subject = null, string $message = 'Access Denied.'): void { - if (!$this->isGranted($attribute, $subject)) { + $decision = $this->getAccessDecision($attribute, $subject); + + if ($decision->isDenied()) { $exception = $this->createAccessDeniedException($message); $exception->setAttributes([$attribute]); $exception->setSubject($subject); - + $exception->setAccessDecision($decision); throw $exception; } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php index 55a3639848c32..d3f03d9a3b5a9 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php @@ -40,7 +40,9 @@ use Symfony\Component\Routing\RouterInterface; 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\AuthorizationCheckerInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\User\InMemoryUser; use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface; @@ -362,7 +364,14 @@ public function testdenyAccessUnlessGranted() $this->expectException(AccessDeniedException::class); - $controller->denyAccessUnlessGranted('foo'); + try { + $controller->denyAccessUnlessGranted('foo'); + } catch (AccessDeniedException $exception) { + $this->assertFalse($exception->getAccessDecision()->getAccess()); + $this->assertEmpty($exception->getAccessDecision()->getVotes()); + $this->assertEmpty($exception->getAccessDecision()->getMessage()); + throw $exception; + } } /** @@ -644,4 +653,33 @@ public function testSendEarlyHints() $this->assertSame('; rel="preload"; as="stylesheet",; rel="preload"; as="script"', $response->headers->get('Link')); } + + public function testdenyAccessUnlessGrantedWithAccessDecisionObject() + { + $authorizationChecker = new class implements AuthorizationCheckerInterface { + public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecision &$accessDecision = null): bool + { + $accessDecision = new AccessDecision(false, [new Vote(-1)], 'access denied'); + + return $accessDecision->getAccess(); + } + }; + + $container = new Container(); + $container->set('security.authorization_checker', $authorizationChecker); + + $controller = $this->createController(); + $controller->setContainer($container); + + $this->expectException(AccessDeniedException::class); + + try { + $controller->denyAccessUnlessGranted('foo'); + } catch (AccessDeniedException $exception) { + $this->assertFalse($exception->getAccessDecision()->getAccess()); + $this->assertCount(1, $exception->getAccessDecision()->getVotes()); + $this->assertSame('access denied', $exception->getAccessDecision()->getMessage()); + throw $exception; + } + } } diff --git a/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php b/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php index f3c1cd1fe34af..a24398ae6e51f 100644 --- a/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php +++ b/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php @@ -20,9 +20,12 @@ use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Role\RoleHierarchyInterface; use Symfony\Component\Security\Http\Firewall\SwitchUserListener; use Symfony\Component\Security\Http\FirewallMapInterface; @@ -138,6 +141,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep // collect voter details $decisionLog = $this->accessDecisionManager->getDecisionLog(); + foreach ($decisionLog as $key => $log) { $decisionLog[$key]['voter_details'] = []; foreach ($log['voterDetails'] as $voterDetail) { @@ -146,10 +150,14 @@ public function collect(Request $request, Response $response, ?\Throwable $excep $decisionLog[$key]['voter_details'][] = [ 'class' => $classData, 'attributes' => $voterDetail['attributes'], // Only displayed for unanimous strategy - 'vote' => $voterDetail['vote'], + 'vote' => $voterDetail['vote'] instanceof VoteInterface ? $voterDetail['vote'] : new Vote($voterDetail['vote']), ]; } unset($decisionLog[$key]['voterDetails']); + + if (!$decisionLog[$key]['result'] instanceof AccessDecision) { + $decisionLog[$key]['result'] = new AccessDecision($decisionLog[$key]['result']); + } } $this->data['access_decision_log'] = $decisionLog; diff --git a/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php b/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php index 54eac4384542a..81d934958199c 100644 --- a/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php +++ b/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php @@ -31,7 +31,9 @@ public function __construct( public function onVoterVote(VoteEvent $event): void { - $this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $event->getVote()); + $voteObj = null; + $vote = $event->getVote($voteObj); + $this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $voteObj ?? $vote); } 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 635d61e2dd2c8..5f00689179978 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig @@ -351,11 +351,11 @@ {% else %} - - - - - + + + + + {% set previous_event = (collector.listeners|first) %} @@ -393,10 +393,10 @@ - - - - + + + + {% for i, authenticator in collector.authenticators %} - + + @@ -526,6 +527,7 @@ + @@ -534,7 +536,7 @@ + @@ -570,14 +573,17 @@ {% endif %} diff --git a/src/Symfony/Bundle/SecurityBundle/Security.php b/src/Symfony/Bundle/SecurityBundle/Security.php index 0cb23c7601b0b..8c4457adc2188 100644 --- a/src/Symfony/Bundle/SecurityBundle/Security.php +++ b/src/Symfony/Bundle/SecurityBundle/Security.php @@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; 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\Authorization\UserAuthorizationCheckerInterface; use Symfony\Component\Security\Core\Exception\LogicException; @@ -58,10 +59,10 @@ public function getUser(): ?UserInterface /** * Checks if the attributes are granted against the current authentication token and optionally supplied subject. */ - public function isGranted(mixed $attributes, mixed $subject = null): bool + public function isGranted(mixed $attributes, mixed $subject = null, ?AccessDecision $accessDecision = null): bool { return $this->container->get('security.authorization_checker') - ->isGranted($attributes, $subject); + ->isGranted($attributes, $subject, $accessDecision); } public function getToken(): ?TokenInterface diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php index 21161d281eb92..856e9c13ae7ff 100644 --- a/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php +++ b/src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php @@ -26,8 +26,11 @@ use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; 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\TraceableAccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Role\RoleHierarchy; use Symfony\Component\Security\Core\User\InMemoryUser; @@ -225,13 +228,102 @@ public function testCollectCollectsDecisionLogWhenStrategyIsAffirmative() { $voter1 = new DummyVoter(); $voter2 = new DummyVoter(); + $voter3 = new DummyVoterWithObject(); + + $decoratedVoter = function (VoterInterface $voter) { + return new TraceableVoter( + $voter, new class implements EventDispatcherInterface { + public function dispatch(object $event, ?string $eventName = null): object + { + return new \stdClass(); + } + } + ); + }; + + $strategy = MainConfiguration::STRATEGY_AFFIRMATIVE; + + $accessDecisionManager = $this->createMock(TraceableAccessDecisionManager::class); + + $accessDecisionManager + ->method('getStrategy') + ->willReturn($strategy); + + $accessDecisionManager + ->method('getVoters') + ->willReturn([ + $decoratedVoter($voter1), + $decoratedVoter($voter2), + $decoratedVoter($voter3), + ]); + + $accessDecisionManager + ->method('getDecisionLog') + ->willReturn([[ + 'attributes' => ['view'], + 'object' => new \stdClass(), + 'result' => new AccessDecision(true), + 'voterDetails' => [ + ['voter' => $voter1, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN], + ['voter' => $voter2, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN], + ['voter' => $voter3, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_ABSTAIN)], + ], + ]]); + + $dataCollector = new SecurityDataCollector(null, null, null, $accessDecisionManager, null, null, true); + + $dataCollector->collect(new Request(), new Response()); + + $actualDecisionLog = $dataCollector->getAccessDecisionLog(); + + $expectedDecisionLog = [[ + 'attributes' => ['view'], + 'object' => new \stdClass(), + 'result' => new AccessDecision(true), + 'voter_details' => [ + ['class' => $voter1::class, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_ABSTAIN)], + ['class' => $voter2::class, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_ABSTAIN)], + ['class' => $voter3::class, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_ABSTAIN)], + ], + ]]; + + $this->assertEquals($actualDecisionLog, $expectedDecisionLog, 'Wrong value returned by getAccessDecisionLog'); + + $actualVoterClasses = array_map(static function (ClassStub $classStub): string { + return (string) $classStub; + }, $dataCollector->getVoters()); - $decoratedVoter1 = new TraceableVoter($voter1, new class implements EventDispatcherInterface { - public function dispatch(object $event, ?string $eventName = null): object - { - return new \stdClass(); - } - }); + $expectedVoterClasses = [ + $voter1::class, + $voter2::class, + $voter3::class, + ]; + + $this->assertSame( + $actualVoterClasses, + $expectedVoterClasses, + 'Wrong value returned by getVoters' + ); + + $this->assertSame($dataCollector->getVoterStrategy(), $strategy, 'Wrong value returned by getVoterStrategy'); + } + + public function testCollectCollectsDecisionLogWhenStrategyIsAffirmativeWithVoterMessage() + { + $voter1 = new DummyVoter(); + $voter2 = new DummyVoter(); + $voter3 = new DummyVoterWithObject(); + + $decoratedVoter = function (VoterInterface $voter) { + return new TraceableVoter( + $voter, new class implements EventDispatcherInterface { + public function dispatch(object $event, ?string $eventName = null): object + { + return new \stdClass(); + } + } + ); + }; $strategy = MainConfiguration::STRATEGY_AFFIRMATIVE; @@ -244,8 +336,9 @@ public function dispatch(object $event, ?string $eventName = null): object $accessDecisionManager ->method('getVoters') ->willReturn([ - $decoratedVoter1, - $decoratedVoter1, + $decoratedVoter($voter1), + $decoratedVoter($voter2), + $decoratedVoter($voter3), ]); $accessDecisionManager @@ -257,6 +350,7 @@ public function dispatch(object $event, ?string $eventName = null): object 'voterDetails' => [ ['voter' => $voter1, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN], ['voter' => $voter2, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN], + ['voter' => $voter3, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_ABSTAIN, ['voter message'])], ], ]]); @@ -269,10 +363,11 @@ public function dispatch(object $event, ?string $eventName = null): object $expectedDecisionLog = [[ 'attributes' => ['view'], 'object' => new \stdClass(), - 'result' => true, + 'result' => new AccessDecision(true), 'voter_details' => [ - ['class' => $voter1::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN], - ['class' => $voter2::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN], + ['class' => $voter1::class, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_ABSTAIN)], + ['class' => $voter2::class, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_ABSTAIN)], + ['class' => $voter3::class, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_ABSTAIN, ['voter message'])], ], ]]; @@ -285,6 +380,7 @@ public function dispatch(object $event, ?string $eventName = null): object $expectedVoterClasses = [ $voter1::class, $voter2::class, + $voter3::class, ]; $this->assertSame( @@ -300,13 +396,18 @@ public function testCollectCollectsDecisionLogWhenStrategyIsUnanimous() { $voter1 = new DummyVoter(); $voter2 = new DummyVoter(); - - $decoratedVoter1 = new TraceableVoter($voter1, new class implements EventDispatcherInterface { - public function dispatch(object $event, ?string $eventName = null): object - { - return new \stdClass(); - } - }); + $voter3 = new DummyVoterWithObject(); + + $decoratedVoter = function (VoterInterface $voter) { + return new TraceableVoter( + $voter, new class implements EventDispatcherInterface { + public function dispatch(object $event, ?string $eventName = null): object + { + return new \stdClass(); + } + } + ); + }; $strategy = MainConfiguration::STRATEGY_UNANIMOUS; @@ -319,8 +420,9 @@ public function dispatch(object $event, ?string $eventName = null): object $accessDecisionManager ->method('getVoters') ->willReturn([ - $decoratedVoter1, - $decoratedVoter1, + $decoratedVoter($voter1), + $decoratedVoter($voter2), + $decoratedVoter($voter3), ]); $accessDecisionManager @@ -335,6 +437,8 @@ public function dispatch(object $event, ?string $eventName = null): object ['voter' => $voter1, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_DENIED], ['voter' => $voter2, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_GRANTED], ['voter' => $voter2, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter3, 'attributes' => ['edit'], 'vote' => new Vote(VoterInterface::ACCESS_DENIED)], + ['voter' => $voter3, 'attributes' => ['edit'], 'vote' => new Vote(VoterInterface::ACCESS_GRANTED)], ], ], [ @@ -358,21 +462,23 @@ public function dispatch(object $event, ?string $eventName = null): object [ 'attributes' => ['view', 'edit'], 'object' => new \stdClass(), - 'result' => false, + 'result' => new AccessDecision(false), 'voter_details' => [ - ['class' => $voter1::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_DENIED], - ['class' => $voter1::class, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_DENIED], - ['class' => $voter2::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_GRANTED], - ['class' => $voter2::class, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['class' => $voter1::class, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_DENIED)], + ['class' => $voter1::class, 'attributes' => ['edit'], 'vote' => new Vote(VoterInterface::ACCESS_DENIED)], + ['class' => $voter2::class, 'attributes' => ['view'], 'vote' => new Vote(VoterInterface::ACCESS_GRANTED)], + ['class' => $voter2::class, 'attributes' => ['edit'], 'vote' => new Vote(VoterInterface::ACCESS_GRANTED)], + ['class' => $voter3::class, 'attributes' => ['edit'], 'vote' => new Vote(VoterInterface::ACCESS_DENIED)], + ['class' => $voter3::class, 'attributes' => ['edit'], 'vote' => new Vote(VoterInterface::ACCESS_GRANTED)], ], ], [ 'attributes' => ['update'], 'object' => new \stdClass(), - 'result' => true, + 'result' => new AccessDecision(true), 'voter_details' => [ - ['class' => $voter1::class, 'attributes' => ['update'], 'vote' => VoterInterface::ACCESS_GRANTED], - ['class' => $voter2::class, 'attributes' => ['update'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['class' => $voter1::class, 'attributes' => ['update'], 'vote' => new Vote(VoterInterface::ACCESS_GRANTED)], + ['class' => $voter2::class, 'attributes' => ['update'], 'vote' => new Vote(VoterInterface::ACCESS_GRANTED)], ], ], ]; @@ -386,6 +492,7 @@ public function dispatch(object $event, ?string $eventName = null): object $expectedVoterClasses = [ $voter1::class, $voter2::class, + $voter3::class, ]; $this->assertSame( @@ -465,3 +572,10 @@ public function vote(TokenInterface $token, mixed $subject, array $attributes): { } } + +final class DummyVoterWithObject implements VoterInterface +{ + public function vote(TokenInterface $token, mixed $subject, array $attributes, ?VoteInterface $vote = null): int + { + } +} diff --git a/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php b/src/Symfony/Bundle/SecurityBundle/Tests/EventListener/VoteListenerTest.php index d262effae29ac..9328d0ae2a03a 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; @@ -37,4 +38,23 @@ public function testOnVoterVote() $sut = new VoteListener($traceableAccessDecisionManager); $sut->onVoterVote(new VoteEvent($voter, 'mysubject', ['myattr1', 'myattr2'], VoterInterface::ACCESS_GRANTED)); } + + public function testOnVoterVoteWithObject() + { + $voter = $this->createMock(VoterInterface::class); + + $traceableAccessDecisionManager = $this + ->getMockBuilder(TraceableAccessDecisionManager::class) + ->disableOriginalConstructor() + ->onlyMethods(['addVoterVote']) + ->getMock(); + + $traceableAccessDecisionManager + ->expects($this->once()) + ->method('addVoterVote') + ->with($voter, ['myattr1', 'myattr2'], new Vote(VoterInterface::ACCESS_GRANTED)); + + $sut = new VoteListener($traceableAccessDecisionManager); + $sut->onVoterVote(new VoteEvent($voter, 'mysubject', ['myattr1', 'myattr2'], new Vote(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..403320494e2b1 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -0,0 +1,61 @@ + + * + * 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\VoteInterface; + +/** + * An AccessDecision is returned by an AccessDecisionManager and contains the access verdict and all the related votes. + * + * @author Dany Maillard + * @author Roman JOLY + */ +final class AccessDecision +{ + /** + * @param VoteInterface[]|int[] $votes + */ + public function __construct( + private readonly bool $access, + private readonly array $votes = [], + private readonly string $message = '', + ) { + } + + public function getAccess(): bool + { + return $this->access; + } + + public function isGranted(): bool + { + return true === $this->access; + } + + public function isDenied(): bool + { + return false === $this->access; + } + + public function getMessage(): string + { + return $this->message; + } + + /** + * @return VoteInterface[]|int[] + */ + public function getVotes(): array + { + return $this->votes; + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 3e42c4bf0af98..9a473fae544b0 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -13,8 +13,10 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; +use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionVoteObjectStrategyInterface; use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; @@ -49,30 +51,34 @@ public function __construct( /** * @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array */ - public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool + public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false, ?AccessDecision &$accessDecision = null): bool { // Special case for AccessListener, do not remove the right side of the condition before 6.0 if (\count($attributes) > 1 && !$allowMultipleAttributes) { throw new InvalidArgumentException(\sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__)); } - - return $this->strategy->decide( - $this->collectResults($token, $attributes, $object) + $decision = $this->strategy->decide( + $this->collectResults($token, $attributes, $object, $this->strategy instanceof AccessDecisionVoteObjectStrategyInterface), + $accessDecision, ); + + return $decision; } /** - * @return \Traversable + * @return \Traversable */ - private function collectResults(TokenInterface $token, array $attributes, mixed $object): \Traversable + private function collectResults(TokenInterface $token, array $attributes, mixed $object, bool $returnAsObject): \Traversable { foreach ($this->getVoters($attributes, $object) as $voter) { - $result = $voter->vote($token, $object, $attributes); - if (!\is_int($result) || !(self::VALID_VOTES[$result] ?? false)) { + $vote = null; + $result = $voter->vote($token, $object, $attributes, $vote); + + if (!($vote instanceof VoteInterface) && (!\is_int($result) || !(self::VALID_VOTES[$result] ?? false))) { throw new \LogicException(\sprintf('"%s::vote()" must return one of "%s" constants ("ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN"), "%s" returned.', get_debug_type($voter), VoterInterface::class, var_export($result, true))); } - yield $result; + yield $returnAsObject && null !== $vote ? $vote : $result; } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index f25c7e1bef9b3..1eccc1ee04ff0 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php @@ -26,5 +26,5 @@ interface AccessDecisionManagerInterface * @param array $attributes An array of attributes associated with the method being invoked * @param mixed $object The object to secure */ - public function decide(TokenInterface $token, array $attributes, mixed $object = null): bool; + public function decide(TokenInterface $token, array $attributes, mixed $object = null/* , bool $allowMultipleAttributes = false, ?AccessDecision &$accessDecision = null */): bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index c748697c494f9..e0371e414243f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -30,14 +30,16 @@ public function __construct( ) { } - final public function isGranted(mixed $attribute, mixed $subject = null): bool + final public function isGranted(mixed $attribute, mixed $subject = null, ?AccessDecision &$accessDecision = null): bool { $token = $this->tokenStorage->getToken(); if (!$token || !$token->getUser()) { $token = new NullToken(); } + $accessDecision = null; + $decision = $this->accessDecisionManager->decide($token, [$attribute], $subject, false, $accessDecision); - return $this->accessDecisionManager->decide($token, [$attribute], $subject); + return $decision; } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php index 6f5a6022178ba..ce19df5784058 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php @@ -23,5 +23,5 @@ interface AuthorizationCheckerInterface * * @param mixed $attribute A single attribute to vote on (can be of any type, string and instance of Expression are supported by the core) */ - public function isGranted(mixed $attribute, mixed $subject = null): bool; + public function isGranted(mixed $attribute, mixed $subject = null/* , ?AccessDecision &$accessDecision = null */): bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php index 00238378a30fa..4e80f36c38666 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionStrategyInterface.php @@ -11,6 +11,9 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; + /** * A strategy for turning a stream of votes into a final decision. * @@ -19,7 +22,7 @@ interface AccessDecisionStrategyInterface { /** - * @param \Traversable $results + * @param \Traversable $results */ - public function decide(\Traversable $results): bool; + public function decide(\Traversable $results/* , ?AccessDecision &$accessDecision */): bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionVoteObjectStrategyInterface.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionVoteObjectStrategyInterface.php new file mode 100644 index 0000000000000..e4471d82df5a0 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AccessDecisionVoteObjectStrategyInterface.php @@ -0,0 +1,28 @@ + + * + * 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\Strategy; + +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; + +/** + * A strategy for turning a stream of votes into a final decision. + * + * @author Alexander M. Turek + */ +interface AccessDecisionVoteObjectStrategyInterface extends AccessDecisionStrategyInterface +{ + /** + * @param \Traversable $results + */ + public function decide(\Traversable $results, ?AccessDecision &$accessDecision = null): bool; +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php index fb316fd31e027..be42ef811a823 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/AffirmativeStrategy.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -22,19 +24,28 @@ * @author Fabien Potencier * @author Alexander M. Turek */ -final class AffirmativeStrategy implements AccessDecisionStrategyInterface, \Stringable +final class AffirmativeStrategy implements AccessDecisionVoteObjectStrategyInterface, \Stringable { public function __construct( private bool $allowIfAllAbstainDecisions = false, ) { } - public function decide(\Traversable $results): bool + public function decide(\Traversable $results, ?AccessDecision &$accessDecision = null): bool { $deny = 0; + $allVotes = []; + foreach ($results as $result) { + $allVotes[] = $result; + if ($result instanceof VoteInterface) { + $result = $result->getAccess(); + } + if (VoterInterface::ACCESS_GRANTED === $result) { - return true; + $accessDecision = new AccessDecision(true, $allVotes); + + return $accessDecision->getAccess(); } if (VoterInterface::ACCESS_DENIED === $result) { @@ -43,10 +54,14 @@ public function decide(\Traversable $results): bool } if ($deny > 0) { - return false; + $accessDecision = new AccessDecision(false, $allVotes); + + return $accessDecision->getAccess(); } - return $this->allowIfAllAbstainDecisions; + $accessDecision = new AccessDecision($this->allowIfAllAbstainDecisions, $allVotes); + + return $accessDecision->getAccess(); } public function __toString(): string diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php index bff56513830f3..6946ed4909cb7 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -30,7 +32,7 @@ * @author Fabien Potencier * @author Alexander M. Turek */ -final class ConsensusStrategy implements AccessDecisionStrategyInterface, \Stringable +final class ConsensusStrategy implements AccessDecisionVoteObjectStrategyInterface, \Stringable { public function __construct( private bool $allowIfAllAbstainDecisions = false, @@ -38,11 +40,18 @@ public function __construct( ) { } - public function decide(\Traversable $results): bool + public function decide(\Traversable $results, ?AccessDecision &$accessDecision = null): bool { $grant = 0; $deny = 0; + $allVotes = []; + foreach ($results as $result) { + $allVotes[] = $result; + if ($result instanceof VoteInterface) { + $result = $result->getAccess(); + } + if (VoterInterface::ACCESS_GRANTED === $result) { ++$grant; } elseif (VoterInterface::ACCESS_DENIED === $result) { @@ -51,18 +60,26 @@ public function decide(\Traversable $results): bool } if ($grant > $deny) { - return true; + $accessDecision = new AccessDecision(true, $allVotes); + + return $accessDecision->getAccess(); } if ($deny > $grant) { - return false; + $accessDecision = new AccessDecision(false, $allVotes); + + return $accessDecision->getAccess(); } if ($grant > 0) { - return $this->allowIfEqualGrantedDeniedDecisions; + $accessDecision = new AccessDecision($this->allowIfEqualGrantedDeniedDecisions, $allVotes); + + return $accessDecision->getAccess(); } - return $this->allowIfAllAbstainDecisions; + $accessDecision = new AccessDecision($this->allowIfAllAbstainDecisions, $allVotes); + + return $accessDecision->getAccess(); } public function __toString(): string diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php index d7f566adbf19d..9632b8411ecb3 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -23,26 +25,39 @@ * @author Fabien Potencier * @author Alexander M. Turek */ -final class PriorityStrategy implements AccessDecisionStrategyInterface, \Stringable +final class PriorityStrategy implements AccessDecisionVoteObjectStrategyInterface, \Stringable { public function __construct( private bool $allowIfAllAbstainDecisions = false, ) { } - public function decide(\Traversable $results): bool + public function decide(\Traversable $results, ?AccessDecision &$accessDecision = null): bool { + $allVotes = []; + foreach ($results as $result) { + $allVotes[] = $result; + if ($result instanceof VoteInterface) { + $result = $result->getAccess(); + } + if (VoterInterface::ACCESS_GRANTED === $result) { - return true; + $accessDecision = new AccessDecision(true, $allVotes); + + return $accessDecision->getAccess(); } if (VoterInterface::ACCESS_DENIED === $result) { - return false; + $accessDecision = new AccessDecision(false, $allVotes); + + return $accessDecision->getAccess(); } } - return $this->allowIfAllAbstainDecisions; + $accessDecision = new AccessDecision($this->allowIfAllAbstainDecisions, $allVotes); + + return $accessDecision->getAccess(); } public function __toString(): string diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php index d47d8994f86c4..ba048e86fb2d6 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php +++ b/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php @@ -11,6 +11,8 @@ namespace Symfony\Component\Security\Core\Authorization\Strategy; +use Symfony\Component\Security\Core\Authorization\AccessDecision; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -22,19 +24,28 @@ * @author Fabien Potencier * @author Alexander M. Turek */ -final class UnanimousStrategy implements AccessDecisionStrategyInterface, \Stringable +final class UnanimousStrategy implements AccessDecisionVoteObjectStrategyInterface, \Stringable { public function __construct( private bool $allowIfAllAbstainDecisions = false, ) { } - public function decide(\Traversable $results): bool + public function decide(\Traversable $results, ?AccessDecision &$accessDecision = null): bool { $grant = 0; + $allVotes = []; + foreach ($results as $result) { + $allVotes[] = $result; + if ($result instanceof VoteInterface) { + $result = $result->getAccess(); + } + if (VoterInterface::ACCESS_DENIED === $result) { - return false; + $accessDecision = new AccessDecision(false, $allVotes); + + return $accessDecision->getAccess(); } if (VoterInterface::ACCESS_GRANTED === $result) { @@ -44,10 +55,14 @@ public function decide(\Traversable $results): bool // no deny votes if ($grant > 0) { - return true; + $accessDecision = new AccessDecision(true, $allVotes); + + return $accessDecision->getAccess(); } - return $this->allowIfAllAbstainDecisions; + $accessDecision = new AccessDecision($this->allowIfAllAbstainDecisions, $allVotes); + + return $accessDecision->getAccess(); } public function __toString(): string diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index 0b82eb3a6d96d..663ca11ed57b6 100644 --- a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php @@ -13,6 +13,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -45,7 +46,7 @@ public function __construct( } } - public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool + public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false, ?AccessDecision &$accessDecision = null): bool { $currentDecisionLog = [ 'attributes' => $attributes, @@ -55,7 +56,7 @@ public function decide(TokenInterface $token, array $attributes, mixed $object = $this->currentLog[] = &$currentDecisionLog; - $result = $this->manager->decide($token, $attributes, $object, $allowMultipleAttributes); + $result = $this->manager->decide($token, $attributes, $object, $allowMultipleAttributes, $accessDecision); $currentDecisionLog['result'] = $result; @@ -67,10 +68,10 @@ public function decide(TokenInterface $token, array $attributes, mixed $object = /** * Adds voter vote and class to the voter details. * - * @param array $attributes attributes used for the vote - * @param int $vote vote of the voter + * @param array $attributes attributes used for the vote + * @param VoteInterface|int $vote vote of the voter */ - public function addVoterVote(VoterInterface $voter, array $attributes, int $vote): void + public function addVoterVote(VoterInterface $voter, array $attributes, VoteInterface|int $vote): void { $currentLogIndex = \count($this->currentLog) - 1; $this->currentLog[$currentLogIndex]['voterDetails'][] = [ diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index 1abc7c704fb59..281924f2a29a8 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -30,11 +30,11 @@ public function __construct( ) { } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function vote(TokenInterface $token, mixed $subject, array $attributes, ?VoteInterface &$vote = null): int { - $result = $this->voter->vote($token, $subject, $attributes); + $result = $this->voter->vote($token, $subject, $attributes, $vote); - $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $result), 'debug.security.authorization.vote'); + $this->eventDispatcher->dispatch(new VoteEvent($this->voter, $subject, $attributes, $vote ?? $result), '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..b26aef74c4207 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Vote.php @@ -0,0 +1,62 @@ + + * + * 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 and some messages. + * + * @author Roman JOLY + */ +class Vote implements VoteInterface +{ + private const VALID_VOTES = [ + VoterInterface::ACCESS_GRANTED => true, + VoterInterface::ACCESS_DENIED => true, + VoterInterface::ACCESS_ABSTAIN => true, + ]; + + public function __construct( + private int $access, + private string|array $messages = [], + ) { + if (!\in_array($access, [VoterInterface::ACCESS_GRANTED, VoterInterface::ACCESS_ABSTAIN, VoterInterface::ACCESS_DENIED], true)) { + throw new \LogicException(\sprintf('"$access" must return one of "%s" constants ("ACCESS_GRANTED", "ACCESS_DENIED" or "ACCESS_ABSTAIN"), "%s" returned.', VoterInterface::class, $access)); + } + $this->setMessages($messages); + } + + public function getAccess(): int + { + return $this->access; + } + + /** + * @return string[] + */ + public function getMessages(): array + { + return $this->messages; + } + + /** + * @throws \InvalidArgumentException + */ + public function setMessages(string|array $messages): void + { + $this->messages = (array) $messages; + foreach ($this->messages as $message) { + if (!\is_string($message)) { + throw new \InvalidArgumentException(\sprintf('Message must be string, "%s" given.', get_debug_type($message))); + } + } + } +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/VoteInterface.php b/src/Symfony/Component/Security/Core/Authorization/Voter/VoteInterface.php new file mode 100644 index 0000000000000..a79a8e1a93917 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/VoteInterface.php @@ -0,0 +1,27 @@ + + * + * 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 VoteInterface implemented object can be returned by a Voter instead simple int for add some data, messages or other. + * + * @author Roman JOLY + */ +interface VoteInterface +{ + public function getAccess(): int; + + /** + * @return string[] + */ + public function getMessages(): array; +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php index 5255c88e6ec0f..644959fd5e1c5 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php @@ -35,5 +35,5 @@ interface VoterInterface * * @return self::ACCESS_* */ - public function vote(TokenInterface $token, mixed $subject, array $attributes): int; + public function vote(TokenInterface $token, mixed $subject, array $attributes/* , ?VoteInterface &$vote = null */): int; } diff --git a/src/Symfony/Component/Security/Core/CHANGELOG.md b/src/Symfony/Component/Security/Core/CHANGELOG.md index 290aa8b25e566..1cdf7a0090c23 100644 --- a/src/Symfony/Component/Security/Core/CHANGELOG.md +++ b/src/Symfony/Component/Security/Core/CHANGELOG.md @@ -9,6 +9,7 @@ CHANGELOG * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user * Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`, erase credentials e.g. using `__serialize()` instead + * Add the ability for voter to return decision reason by passing a `Vote` object 7.2 --- diff --git a/src/Symfony/Component/Security/Core/Event/VoteEvent.php b/src/Symfony/Component/Security/Core/Event/VoteEvent.php index edc66b3667ec2..28be0f41e0007 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\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Contracts\EventDispatcher\Event; @@ -27,7 +28,7 @@ public function __construct( private VoterInterface $voter, private mixed $subject, private array $attributes, - private int $vote, + private VoteInterface|int $vote, ) { } @@ -46,8 +47,10 @@ public function getAttributes(): array return $this->attributes; } - public function getVote(): int + public function getVote(?VoteInterface &$vote = null): int { - return $this->vote; + $vote = $this->vote; + + return $this->vote instanceof VoteInterface ? $this->vote->getAccess() : $this->vote; } } diff --git a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php index 93c3869470d05..04273bf303363 100644 --- a/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php +++ b/src/Symfony/Component/Security/Core/Exception/AccessDeniedException.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Exception; use Symfony\Component\HttpKernel\Attribute\WithHttpStatus; +use Symfony\Component\Security\Core\Authorization\AccessDecision; /** * AccessDeniedException is thrown when the account has not the required role. @@ -23,6 +24,7 @@ class AccessDeniedException extends RuntimeException { private array $attributes = []; private mixed $subject = null; + private ?AccessDecision $accessDecision = null; public function __construct(string $message = 'Access Denied.', ?\Throwable $previous = null, int $code = 403) { @@ -48,4 +50,17 @@ public function setSubject(mixed $subject): void { $this->subject = $subject; } + + /** + * Sets an access decision and appends the denied reasons to the exception message. + */ + public function setAccessDecision(AccessDecision $accessDecision): void + { + $this->accessDecision = $accessDecision; + } + + public function getAccessDecision(): ?AccessDecision + { + return $this->accessDecision; + } } diff --git a/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php b/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php index 792e777915400..552bdabb54c51 100644 --- a/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php +++ b/src/Symfony/Component/Security/Core/Test/AccessDecisionStrategyTestCase.php @@ -14,8 +14,11 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -31,12 +34,19 @@ abstract class AccessDecisionStrategyTestCase extends TestCase * @param VoterInterface[] $voters */ #[DataProvider('provideStrategyTests')] - final public function testDecide(AccessDecisionStrategyInterface $strategy, array $voters, bool $expected) + final public function testDecide(AccessDecisionStrategyInterface $strategy, array $voters, bool $expected, array $votesExpected) { $token = $this->createMock(TokenInterface::class); $manager = new AccessDecisionManager($voters, $strategy); - $this->assertSame($expected, $manager->decide($token, ['ROLE_FOO'])); + $accessDecision = null; + $decision = $manager->decide($token, ['ROLE_FOO'], null, false, $accessDecision); + + $this->assertSame($expected, $decision); + + $this->assertInstanceOf(AccessDecision::class, $accessDecision); + $this->assertSame($expected, $accessDecision->getAccess()); + $this->assertEquals($votesExpected, $accessDecision->getVotes()); } /** @@ -77,4 +87,21 @@ public function vote(TokenInterface $token, $subject, array $attributes): int } }; } + + final protected static function getVoterWithVoteObject(int $vote): VoterInterface + { + return new class($vote) implements VoterInterface { + public function __construct( + private int $vote, + ) { + } + + public function vote(TokenInterface $token, $subject, array $attributes, ?VoteInterface &$vote = null): int + { + $vote = new Vote($this->vote); + + return $this->vote; + } + }; + } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index f0a4978860df9..8c32ebe3603b2 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -14,10 +14,15 @@ use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; +use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionVoteObjectStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; +use Symfony\Component\Security\Core\Tests\Fixtures\DummyVoterWithObject; class AccessDecisionManagerTest extends TestCase { @@ -36,6 +41,7 @@ public function testVoterCalls() $voters = [ $this->getExpectedVoter(VoterInterface::ACCESS_DENIED), $this->getExpectedVoter(VoterInterface::ACCESS_GRANTED), + $this->getExpectedVoter(VoterInterface::ACCESS_DENIED), $this->getUnexpectedVoter(), ]; @@ -52,6 +58,10 @@ public function decide(\Traversable $results): bool case 1: Assert::assertSame(VoterInterface::ACCESS_GRANTED, $result); + break; + case 2: + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result); + return true; } } @@ -65,6 +75,147 @@ public function decide(\Traversable $results): bool $this->assertTrue($manager->decide($token, ['ROLE_FOO'])); } + public function testVoterCallsWithAccessDecisionObject() + { + $token = $this->createMock(TokenInterface::class); + + $voters = [ + $this->getExpectedVoter(VoterInterface::ACCESS_DENIED), + $this->getExpectedVoter(VoterInterface::ACCESS_GRANTED), + new DummyVoterWithObject(new Vote(VoterInterface::ACCESS_DENIED)), + $this->getUnexpectedVoter(), + ]; + + $strategy = new class implements AccessDecisionVoteObjectStrategyInterface { + public function decide(\Traversable $results, ?AccessDecision &$accessDecision = null): bool + { + $i = 0; + foreach ($results as $result) { + switch ($i++) { + case 0: + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result); + + break; + case 1: + Assert::assertSame(VoterInterface::ACCESS_GRANTED, $result); + + break; + case 2: + Assert::assertInstanceOf(VoteInterface::class, $result); + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result->getAccess()); + $accessDecision = new AccessDecision(true, [$result]); + + return true; + } + } + + return false; + } + }; + + $manager = new AccessDecisionManager($voters, $strategy); + $accessDecision = null; + + $decision = $manager->decide($token, ['ROLE_FOO'], null, false, $accessDecision); + $this->assertInstanceOf(AccessDecision::class, $accessDecision); + $this->assertTrue($decision); + $this->assertTrue($accessDecision->getAccess()); + $this->assertEmpty($accessDecision->getMessage()); + } + + public function testVoterCallsWithAccessDecisionObjectAndStrategyWithoutObject() + { + $token = $this->createMock(TokenInterface::class); + + $voters = [ + $this->getExpectedVoter(VoterInterface::ACCESS_DENIED), + $this->getExpectedVoter(VoterInterface::ACCESS_GRANTED), + new DummyVoterWithObject(new Vote(VoterInterface::ACCESS_DENIED)), + $this->getUnexpectedVoter(), + ]; + + $strategy = new class implements AccessDecisionStrategyInterface { + public function decide(\Traversable $results, ?AccessDecision &$accessDecision = null): bool + { + $i = 0; + foreach ($results as $result) { + switch ($i++) { + case 0: + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result); + + break; + case 1: + Assert::assertSame(VoterInterface::ACCESS_GRANTED, $result); + + break; + case 2: + Assert::assertNotInstanceOf(VoteInterface::class, $result); + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result); + + return true; + } + } + + return false; + } + }; + + $manager = new AccessDecisionManager($voters, $strategy); + $accessDecision = null; + + $decision = $manager->decide($token, ['ROLE_FOO'], null, false, $accessDecision); + $this->assertNull($accessDecision); + $this->assertTrue($decision); + } + + public function testVoterCallsWithAccessDecisionObjectFromStrategy() + { + $token = $this->createMock(TokenInterface::class); + + $voters = [ + $this->getExpectedVoter(VoterInterface::ACCESS_DENIED), + $this->getExpectedVoter(VoterInterface::ACCESS_GRANTED), + new DummyVoterWithObject(new Vote(VoterInterface::ACCESS_DENIED)), + $this->getUnexpectedVoter(), + ]; + + $strategy = new class implements AccessDecisionVoteObjectStrategyInterface { + public function decide(\Traversable $results, ?AccessDecision &$accessDecision = null): bool + { + $i = 0; + foreach ($results as $result) { + switch ($i++) { + case 0: + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result); + + break; + case 1: + Assert::assertSame(VoterInterface::ACCESS_GRANTED, $result); + + break; + case 2: + Assert::assertInstanceOf(VoteInterface::class, $result); + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result->getAccess()); + $accessDecision = new AccessDecision(true, [$result], 'message from strategy'); + + return true; + } + } + + return new AccessDecision(false, [], 'message from strategy'); + } + }; + + $manager = new AccessDecisionManager($voters, $strategy); + + $accessDecision = null; + $decision = $manager->decide($token, ['ROLE_FOO'], null, false, $accessDecision); + $this->assertInstanceOf(AccessDecision::class, $accessDecision); + $this->assertTrue($decision); + $this->assertTrue($accessDecision->getAccess()); + $this->assertSame('message from strategy', $accessDecision->getMessage()); + } + public function testCacheableVoters() { $token = $this->createMock(TokenInterface::class); @@ -270,12 +421,12 @@ public function vote(TokenInterface $token, $subject, array $attributes) }; } - private function getExpectedVoter(int $vote): VoterInterface + private function getExpectedVoter(VoteInterface|int $vote): VoterInterface { $voter = $this->createMock(VoterInterface::class); $voter->expects($this->once()) ->method('vote') - ->willReturn($vote); + ->willReturn($vote instanceof VoteInterface ? $vote->getAccess() : $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 36b048c8976d1..9082eb472bb70 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -15,7 +15,9 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; +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\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationChecker; use Symfony\Component\Security\Core\User\InMemoryUser; @@ -77,4 +79,79 @@ public function testIsGrantedWithObjectAttribute() $this->tokenStorage->setToken($token); $this->assertTrue($this->authorizationChecker->isGranted($attribute)); } + + public function testIsGrantedWithAccessDecisionObject() + { + $attribute = new \stdClass(); + + $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + + $accessDecisionManager = new class implements AccessDecisionManagerInterface { + public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false, ?AccessDecision &$accessDecision = null): bool + { + $accessDecision = new AccessDecision(true); + + return $accessDecision->getAccess(); + } + }; + + $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $accessDecisionManager); + + $this->tokenStorage->setToken($token); + + $accessDecision = null; + $decision = $authorizationChecker->isGranted($attribute, $token, $accessDecision); + $this->assertInstanceOf(AccessDecision::class, $accessDecision); + $this->assertTrue($decision); + $this->assertTrue($accessDecision->getAccess()); + $this->assertEmpty($accessDecision->getMessage()); + } + + public function testIsGrantedWithoutAccessDecisionObject() + { + $attribute = new \stdClass(); + + $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + + $accessDecisionManager = new class implements AccessDecisionManagerInterface { + public function decide(TokenInterface $token, array $attributes, mixed $object = null): bool + { + return true; + } + }; + + $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $accessDecisionManager); + + $this->tokenStorage->setToken($token); + + $accessDecision = null; + $decision = $authorizationChecker->isGranted($attribute, $token, $accessDecision); + $this->assertNull($accessDecision); + $this->assertTrue($decision); + } + + public function testIsGrantedWithAccessDecisionObjectFromADM() + { + $attribute = new \stdClass(); + + $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + + $accessDecisionManager = new class implements AccessDecisionManagerInterface { + public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false, ?AccessDecision &$accessDecision = null): bool + { + $accessDecision = new AccessDecision(true, [], 'from accessDecisionManager'); + + return $accessDecision->getAccess(); + } + }; + + $authorizationChecker = new AuthorizationChecker($this->tokenStorage, $accessDecisionManager); + $this->tokenStorage->setToken($token); + + $accessDecision = null; + $decision = $authorizationChecker->isGranted($attribute, $token, $accessDecision); + $this->assertTrue($decision); + $this->assertTrue($accessDecision->getAccess()); + $this->assertSame('from accessDecisionManager', $accessDecision->getMessage()); + } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php index b467a920b0f67..21e45482331c3 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php @@ -12,6 +12,7 @@ namespace Authorization\Strategy; use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Test\AccessDecisionStrategyTestCase; class AffirmativeStrategyTest extends AccessDecisionStrategyTestCase @@ -20,13 +21,56 @@ public static function provideStrategyTests(): iterable { $strategy = new AffirmativeStrategy(); - yield [$strategy, self::getVoters(1, 0, 0), true]; - yield [$strategy, self::getVoters(1, 2, 0), true]; - yield [$strategy, self::getVoters(0, 1, 0), false]; - yield [$strategy, self::getVoters(0, 0, 1), false]; + yield [$strategy, self::getVoters(1, 0, 0), true, [1]]; + yield [$strategy, self::getVoters(1, 2, 0), true, [1]]; + yield [$strategy, self::getVoters(0, 1, 0), false, [-1]]; + yield [$strategy, self::getVoters(0, 0, 1), false, [0]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], true, [ + new Vote(0), + -1, + 0, + new Vote(1), + ]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(0), + ], false, [ + new Vote(0), + -1, + 0, + new Vote(0), + ]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(1), + self::getVoter(0), + self::getVoterWithVoteObject(-1), + ], true, [ + new Vote(0), + 1, + ]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(-1), + ], true, [ + new Vote(1), + ]]; $strategy = new AffirmativeStrategy(true); - yield [$strategy, self::getVoters(0, 0, 1), true]; + yield [$strategy, self::getVoters(0, 0, 1), true, [0]]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php index bde6fb0d624b7..975b404ff8fa8 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php @@ -12,6 +12,7 @@ namespace Authorization\Strategy; use Symfony\Component\Security\Core\Authorization\Strategy\ConsensusStrategy; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Test\AccessDecisionStrategyTestCase; class ConsensusStrategyTest extends AccessDecisionStrategyTestCase @@ -20,21 +21,57 @@ public static function provideStrategyTests(): iterable { $strategy = new ConsensusStrategy(); - yield [$strategy, self::getVoters(1, 0, 0), true]; - yield [$strategy, self::getVoters(1, 2, 0), false]; - yield [$strategy, self::getVoters(2, 1, 0), true]; - yield [$strategy, self::getVoters(0, 0, 1), false]; + yield [$strategy, self::getVoters(1, 0, 0), true, [1]]; + yield [$strategy, self::getVoters(1, 2, 0), false, [1, -1, -1]]; + yield [$strategy, self::getVoters(2, 1, 0), true, [1, 1, -1]]; + yield [$strategy, self::getVoters(0, 0, 1), false, [0]]; - yield [$strategy, self::getVoters(2, 2, 0), true]; - yield [$strategy, self::getVoters(2, 2, 1), true]; + yield [$strategy, self::getVoters(2, 2, 0), true, [1, 1, -1, -1]]; + yield [$strategy, self::getVoters(2, 2, 1), true, [1, 1, -1, -1, 0]]; $strategy = new ConsensusStrategy(true); - yield [$strategy, self::getVoters(0, 0, 1), true]; + yield [$strategy, self::getVoters(0, 0, 1), true, [0]]; $strategy = new ConsensusStrategy(false, false); - yield [$strategy, self::getVoters(2, 2, 0), false]; - yield [$strategy, self::getVoters(2, 2, 1), false]; + yield [$strategy, self::getVoters(2, 2, 0), false, [1, 1, -1, -1]]; + yield [$strategy, self::getVoters(2, 2, 1), false, [1, 1, -1, -1, 0]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], true, [ + new Vote(1), + -1, + 0, + new Vote(1), + ]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(-1), + self::getVoterWithVoteObject(1), + ], false, [ + new Vote(1), + -1, + -1, + new Vote(1), + ]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(-1), + self::getVoterWithVoteObject(0), + ], false, [ + new Vote(1), + -1, + -1, + new Vote(0), + ]]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php index aef3aaf0b27e3..0708b5f543cd6 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php @@ -12,6 +12,7 @@ namespace Authorization\Strategy; use Symfony\Component\Security\Core\Authorization\Strategy\PriorityStrategy; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Test\AccessDecisionStrategyTestCase; @@ -26,19 +27,40 @@ public static function provideStrategyTests(): iterable self::getVoter(VoterInterface::ACCESS_GRANTED), self::getVoter(VoterInterface::ACCESS_DENIED), self::getVoter(VoterInterface::ACCESS_DENIED), - ], true]; + ], true, [0, 1]]; yield [$strategy, [ self::getVoter(VoterInterface::ACCESS_ABSTAIN), self::getVoter(VoterInterface::ACCESS_DENIED), self::getVoter(VoterInterface::ACCESS_GRANTED), self::getVoter(VoterInterface::ACCESS_GRANTED), - ], false]; + ], false, [0, -1]]; - yield [$strategy, self::getVoters(0, 0, 2), false]; + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], true, [new Vote(1)]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], false, [new Vote(0), -1]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(-1), + self::getVoter(1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], false, [new Vote(-1)]]; + + yield [$strategy, self::getVoters(0, 0, 2), false, [0, 0]]; $strategy = new PriorityStrategy(true); - yield [$strategy, self::getVoters(0, 0, 2), true]; + yield [$strategy, self::getVoters(0, 0, 2), true, [0, 0]]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php index e00a50e3186ba..74b0882449435 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php @@ -12,6 +12,7 @@ namespace Authorization\Strategy; use Symfony\Component\Security\Core\Authorization\Strategy\UnanimousStrategy; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Test\AccessDecisionStrategyTestCase; class UnanimousStrategyTest extends AccessDecisionStrategyTestCase @@ -20,14 +21,40 @@ public static function provideStrategyTests(): iterable { $strategy = new UnanimousStrategy(); - yield [$strategy, self::getVoters(1, 0, 0), true]; - yield [$strategy, self::getVoters(1, 0, 1), true]; - yield [$strategy, self::getVoters(1, 1, 0), false]; - - yield [$strategy, self::getVoters(0, 0, 2), false]; - - $strategy = new UnanimousStrategy(true); - - yield [$strategy, self::getVoters(0, 0, 2), true]; + yield [$strategy, self::getVoters(1, 0, 0), true, [1]]; + yield [$strategy, self::getVoters(1, 0, 1), true, [1, 0]]; + yield [$strategy, self::getVoters(1, 1, 0), false, [1, -1]]; + + yield [$strategy, self::getVoters(0, 0, 2), false, [0, 0]]; + + $strategy = new UnanimousStrategy(true, []); + + yield [$strategy, self::getVoters(0, 0, 2), true, [0, 0]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], false, [new Vote(1), -1]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(-1), + self::getVoter(1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], false, [new Vote(-1)]]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(0), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], true, [ + new Vote(0), + 0, + 0, + new Vote(1) + ]]; } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php index 8797d74d79f0c..515393caf028d 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/TraceableAccessDecisionManagerTest.php @@ -16,8 +16,10 @@ use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Tests\Fixtures\DummyVoter; +use Symfony\Component\Security\Core\Tests\Fixtures\DummyVoterWithObject; class TraceableAccessDecisionManagerTest extends TestCase { @@ -54,6 +56,7 @@ public static function provideObjectsAndLogs(): \Generator { $voter1 = new DummyVoter(); $voter2 = new DummyVoter(); + $voter3 = new DummyVoterWithObject(); yield [ [[ @@ -169,6 +172,27 @@ public static function provideObjectsAndLogs(): \Generator ], false, ]; + + yield [ + [[ + 'attributes' => ['ATTRIBUTE_1'], + 'object' => null, + 'result' => true, + 'voterDetails' => [ + ['voter' => $voter1, 'attributes' => ['ATTRIBUTE_1'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter2, 'attributes' => ['ATTRIBUTE_1'], 'vote' => VoterInterface::ACCESS_GRANTED], + ['voter' => $voter3, 'attributes' => ['ATTRIBUTE_1'], 'vote' => new Vote(VoterInterface::ACCESS_GRANTED)], + ], + ]], + ['ATTRIBUTE_1'], + null, + [ + [$voter1, VoterInterface::ACCESS_GRANTED], + [$voter2, VoterInterface::ACCESS_GRANTED], + [$voter3, new Vote(VoterInterface::ACCESS_GRANTED)], + ], + true, + ]; } /** 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 1d8c86490de4e..2039f19954abd 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php @@ -15,8 +15,10 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Event\VoteEvent; +use Symfony\Component\Security\Core\Tests\Fixtures\DummyVoterWithObject; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; class TraceableVoterTest extends TestCase @@ -53,6 +55,28 @@ public function testVote() $this->assertSame(VoterInterface::ACCESS_DENIED, $result); } + public function testVoteWithObject() + { + $voter = $this->createMock(VoterInterface::class); + + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $token = $this->createStub(TokenInterface::class); + + $voter = new DummyVoterWithObject(new Vote(VoterInterface::ACCESS_DENIED)); + + $eventDispatcher + ->expects($this->once()) + ->method('dispatch') + ->with(new VoteEvent($voter, 'anysubject', ['attr1'], new Vote(VoterInterface::ACCESS_DENIED)), 'debug.security.authorization.vote'); + + $sut = new TraceableVoter($voter, $eventDispatcher); + $vote = null; + $result = $sut->vote($token, 'anysubject', ['attr1'], $vote); + + $this->assertSame(VoterInterface::ACCESS_DENIED, $result); + $this->assertSame(VoterInterface::ACCESS_DENIED, $vote->getAccess()); + } + public function testSupportsAttributeOnCacheable() { $voter = $this->createMock(CacheableVoterInterface::class); diff --git a/src/Symfony/Component/Security/Core/Tests/Fixtures/DummyVoterWithObject.php b/src/Symfony/Component/Security/Core/Tests/Fixtures/DummyVoterWithObject.php new file mode 100644 index 0000000000000..c39515e57f6df --- /dev/null +++ b/src/Symfony/Component/Security/Core/Tests/Fixtures/DummyVoterWithObject.php @@ -0,0 +1,29 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Core\Tests\Fixtures; + +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; +use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; + +final class DummyVoterWithObject implements VoterInterface +{ + public function __construct(private ?VoteInterface $vote = null) + { + } + + public function vote(TokenInterface $token, $subject, array $attributes, ?VoteInterface &$vote = null): int + { + $vote = $this->vote; + return $vote->getAccess(); + } +} diff --git a/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php b/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php index c511cf04e2398..c202f7e511a09 100644 --- a/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php +++ b/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php @@ -18,6 +18,7 @@ use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent; use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\HttpKernel\KernelEvents; +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\Exception\RuntimeException; @@ -59,7 +60,10 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo } } - if (!$this->authChecker->isGranted($attribute->attribute, $subject)) { + $accessDecision = null; + $decision = $this->authChecker->isGranted($attribute->attribute, $subject, $accessDecision); + + if (!$decision) { $message = $attribute->message ?: \sprintf('Access Denied by #[IsGranted(%s)] on controller', $this->getIsGrantedString($attribute)); if ($statusCode = $attribute->statusCode) { @@ -69,6 +73,7 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo $accessDeniedException = new AccessDeniedException($message, code: $attribute->exceptionCode ?? 403); $accessDeniedException->setAttributes($attribute->attribute); $accessDeniedException->setSubject($subject); + $accessDeniedException->setAccessDecision($accessDecision ?? new AccessDecision($decision)); throw $accessDeniedException; } diff --git a/src/Symfony/Component/Security/Http/Firewall/AccessListener.php b/src/Symfony/Component/Security/Http/Firewall/AccessListener.php index f04de5a9fcd50..7dd0799daed82 100644 --- a/src/Symfony/Component/Security/Http/Firewall/AccessListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/AccessListener.php @@ -15,6 +15,7 @@ use Symfony\Component\HttpKernel\Event\RequestEvent; use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Exception\AccessDeniedException; @@ -73,16 +74,24 @@ public function authenticate(RequestEvent $event): void $token = $this->tokenStorage->getToken() ?? new NullToken(); - if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) { - throw $this->createAccessDeniedException($request, $attributes); + $accessDecision = null; + $decision = $this->accessDecisionManager->decide($token, $attributes, $request, true, $accessDecision); + + if(! $accessDecision instanceof AccessDecision) { + $accessDecision = new AccessDecision($decision); + } + + if ($accessDecision->isDenied()) { + throw $this->createAccessDeniedException($request, $attributes, $accessDecision); } } - private function createAccessDeniedException(Request $request, array $attributes): AccessDeniedException + private function createAccessDeniedException(Request $request, array $attributes, AccessDecision $accessDecision): AccessDeniedException { $exception = new AccessDeniedException(); $exception->setAttributes($attributes); $exception->setSubject($request); + $exception->setAccessDecision($accessDecision); return $exception; } diff --git a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php index ffdad52eef207..bcfc0b62d83ed 100644 --- a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php +++ b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php @@ -19,6 +19,7 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; @@ -154,10 +155,17 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn throw $e; } - if (false === $this->accessDecisionManager->decide($token, [$this->role], $user)) { + $accessDecision = null; + $decision = $this->accessDecisionManager->decide($token, [$this->role], $user, false, $accessDecision); + if(! $accessDecision instanceof AccessDecision) { + $accessDecision = new AccessDecision($decision); + } + + + if (!$decision) { $exception = new AccessDeniedException(); $exception->setAttributes($this->role); - + $exception->setAccessDecision($accessDecision); throw $exception; } diff --git a/src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php b/src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php index 2d03b7ac357ea..af2607e51c219 100644 --- a/src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/EventListener/IsGrantedAttributeListenerTest.php @@ -18,6 +18,7 @@ use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent; use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\HttpKernel\HttpKernelInterface; +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\Http\EventListener\IsGrantedAttributeListener; @@ -434,4 +435,36 @@ public function testAccessDeniedExceptionWithExceptionCode() $listener->onKernelControllerArguments($event); } + + public function testAccessDeniedExceptionWithExceptionCodeAndWithObject() + { + $authChecker = new class() implements AuthorizationCheckerInterface { + public function isGranted(mixed $attribute, mixed $subject = null , ?AccessDecision &$accessDecision = null ): bool + { + $accessDecision = new AccessDecision(false, [], 'User denied'); + return $accessDecision->getAccess(); + } + }; + + $event = new ControllerArgumentsEvent( + $this->createMock(HttpKernelInterface::class), + [new IsGrantedAttributeMethodsController(), 'exceptionCodeInAccessDeniedException'], + [], + new Request(), + null + ); + + $listener = new IsGrantedAttributeListener($authChecker); + + $this->expectException(AccessDeniedException::class); + $this->expectExceptionMessage('Exception Code'); + $this->expectExceptionCode(10010); + + try { + $listener->onKernelControllerArguments($event); + } catch (AccessDeniedException $exception) { + $this->assertSame('User denied', $exception->getAccessDecision()->getMessage()); + 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 5decf414251f9..831702a0e0c75 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -19,7 +19,9 @@ use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Exception\AccessDeniedException; @@ -278,4 +280,55 @@ public function testConstructWithTrueExceptionOnNoToken() new AccessListener($tokenStorage, $this->createMock(AccessDecisionManagerInterface::class), $accessMap, true); } + + public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccessWithAccessDecisionObject() + { + $request = new Request(); + + $accessMap = $this->createMock(AccessMapInterface::class); + $accessMap + ->expects($this->any()) + ->method('getPatterns') + ->with($this->equalTo($request)) + ->willReturn([['foo' => 'bar'], null]) + ; + + $token = new class extends AbstractToken { + public function getCredentials(): mixed + { + } + }; + + $tokenStorage = $this->createMock(TokenStorageInterface::class); + $tokenStorage + ->expects($this->any()) + ->method('getToken') + ->willReturn($token) + ; + + $accessDecisionManager = new class implements AccessDecisionManagerInterface { + function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false, ?AccessDecision &$accessDecision = null): bool + { + $accessDecision = new AccessDecision(false,[], 'not allowed'); + return $accessDecision->getAccess(); + } + }; + + $listener = new AccessListener( + $tokenStorage, + $accessDecisionManager, + $accessMap + ); + + $this->expectException(AccessDeniedException::class); + + try { + $listener(new RequestEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST)); + } catch (AccessDeniedException $exception) { + $this->assertFalse($exception->getAccessDecision()->getAccess()); + $this->assertSame('not allowed', $exception->getAccessDecision()->getMessage()); + throw $exception; + } + + } } diff --git a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php index 114d0db979e46..4ca2028f7aa71 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/SwitchUserListenerTest.php @@ -21,7 +21,9 @@ use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken; 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\AccessDecisionManagerInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException; use Symfony\Component\Security\Core\User\InMemoryUser; @@ -373,4 +375,35 @@ public function testSwitchUserRefreshesOriginalToken() $listener = new SwitchUserListener($this->tokenStorage, $userProvider, $this->userChecker, 'provider123', $this->accessDecisionManager, null, '_switch_user', 'ROLE_ALLOWED_TO_SWITCH', $dispatcher); $listener($this->event); } + + public function testSwitchUserIsDisallowedWithObject() + { + $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'); + + $accessDecision = null; + $accessDecisionManager = new class implements AccessDecisionManagerInterface { + function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false, ?AccessDecision &$accessDecision = null): bool + { + $accessDecision = new AccessDecision(false, [new Vote(false)], 'User unable to switch'); + return $accessDecision->getAccess(); + } + }; + + $this->expectException(AccessDeniedException::class); + + try{ + $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $accessDecisionManager); + $listener($this->event); + } catch (AccessDeniedException $exception) { + $this->assertFalse($exception->getAccessDecision()->getAccess()); + $this->assertSame('User unable to switch', $exception->getAccessDecision()->getMessage()); + $this->assertCount(1, $exception->getAccessDecision()->getVotes()); + + throw $exception; + } + } }
ListenerDurationResponse
ListenerDurationResponse
StatusAuthenticator
StatusAuthenticator
Result Attributes ObjectMessage
{{ loop.index }} - {{ decision.result + {{ decision.result.access ? 'GRANTED' : 'DENIED' }} @@ -554,6 +556,7 @@ {% endif %} {{ profiler_dump(decision.seek('object')) }}{{ decision.result.message }}
attribute {{ voter_detail['attributes'][0] }} - {% 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 %} + {% if voter_detail['vote'].messages is not empty %} + : {{ voter_detail['vote'].messages | join(', ') }} {% endif %}