diff --git a/src/Symfony/Bridge/Twig/Extension/SecurityExtension.php b/src/Symfony/Bridge/Twig/Extension/SecurityExtension.php index 9bf346caefc37..9b0d3969121cc 100644 --- a/src/Symfony/Bridge/Twig/Extension/SecurityExtension.php +++ b/src/Symfony/Bridge/Twig/Extension/SecurityExtension.php @@ -45,6 +45,9 @@ public function isGranted(mixed $role, mixed $object = null, ?string $field = nu } try { + /** + * @var bool + */ return $this->securityChecker->isGranted($role, $object); } catch (AuthenticationCredentialsNotFoundException) { return false; diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php index af453619b5ab8..5ebd13599483a 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,22 @@ 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".'); + } + + $decision = $this->container->get('security.authorization_checker')->isGranted($attribute, $subject, AccessDecision::RETURN_AS_OBJECT); + + return $decision instanceof AccessDecision ? $decision : new AccessDecision($decision); + } + /** * Throws an exception unless the attribute is granted against the current authentication token and optionally * supplied subject. @@ -210,10 +227,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..5099f77cc0132 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,29 @@ public function testSendEarlyHints() $this->assertSame('; rel="preload"; as="stylesheet",; rel="preload"; as="script"', $response->headers->get('Link')); } + + public function testdenyAccessUnlessGrantedWithAccessDecisionObject() + { + $authorizationChecker = $this->createMock(AuthorizationCheckerInterface::class); + $authorizationChecker->expects($this->once()) + ->method('isGranted') + ->willReturn(new AccessDecision(false, [new Vote(-1)], 'access denied')); + + $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..33f1590926aa7 100644 --- a/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php +++ b/src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php @@ -31,7 +31,7 @@ public function __construct( public function onVoterVote(VoteEvent $event): void { - $this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $event->getVote()); + $this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $event->getVote(true)); } 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..2bc82f78bcdaf 100644 --- a/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig +++ b/src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig @@ -518,7 +518,8 @@ - + + @@ -526,6 +527,7 @@ Result Attributes Object + Message @@ -534,7 +536,7 @@ {{ loop.index }} - {{ decision.result + {{ decision.result.access ? 'GRANTED' : 'DENIED' }} @@ -554,6 +556,7 @@ {% endif %} {{ profiler_dump(decision.seek('object')) }} + {{ decision.result.message }} @@ -570,14 +573,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 %} + {% if voter_detail['vote'].messages is not empty %} + : {{ voter_detail['vote'].messages | join(', ') }} {% endif %} diff --git a/src/Symfony/Bundle/SecurityBundle/Security.php b/src/Symfony/Bundle/SecurityBundle/Security.php index 0cb23c7601b0b..c0d8fee1f7700 100644 --- a/src/Symfony/Bundle/SecurityBundle/Security.php +++ b/src/Symfony/Bundle/SecurityBundle/Security.php @@ -58,10 +58,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, $asObject = false): bool { return $this->container->get('security.authorization_checker') - ->isGranted($attributes, $subject); + ->isGranted($attributes, $subject, $asObject); } 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..d9bb091cac4f7 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, bool $asObject = false): VoteInterface + { + } +} 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..a2d96f96f1a23 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecision.php @@ -0,0 +1,63 @@ + + * + * 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 +{ + public const RETURN_AS_OBJECT = true; + + /** + * @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..e3144a7f5fcc3 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -15,6 +15,7 @@ use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; @@ -49,26 +50,32 @@ 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, bool $asObject = false): AccessDecision|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, $asObject), $asObject ); + + if (!($decision instanceof AccessDecision) && $asObject) { + return new AccessDecision($decision); + } + + 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 $asObject): \Traversable { foreach ($this->getVoters($attributes, $object) as $voter) { - $result = $voter->vote($token, $object, $attributes); - if (!\is_int($result) || !(self::VALID_VOTES[$result] ?? false)) { + $result = $voter->vote($token, $object, $attributes, $asObject); + if (!($result 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))); } diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManagerInterface.php index f25c7e1bef9b3..963da93955b85 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): AccessDecision|bool; } diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php index c748697c494f9..2f989d3974373 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/AuthorizationChecker.php @@ -30,7 +30,7 @@ public function __construct( ) { } - final public function isGranted(mixed $attribute, mixed $subject = null): bool + final public function isGranted(mixed $attribute, mixed $subject = null, bool $asObject = false): AccessDecision|bool { $token = $this->tokenStorage->getToken(); @@ -38,6 +38,8 @@ final public function isGranted(mixed $attribute, mixed $subject = null): bool $token = new NullToken(); } - return $this->accessDecisionManager->decide($token, [$attribute], $subject); + $decision = $this->accessDecisionManager->decide($token, [$attribute], $subject, false, $asObject); + + return (!$asObject || $decision instanceof AccessDecision) ? $decision : new AccessDecision($decision); } } diff --git a/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php b/src/Symfony/Component/Security/Core/Authorization/AuthorizationCheckerInterface.php index 6f5a6022178ba..ecda1bfff388e 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/* , bool $asObject = false */): AccessDecision|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..dd5b55f9cd157 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|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..74103f9914eb2 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; /** @@ -29,12 +31,19 @@ public function __construct( ) { } - public function decide(\Traversable $results): bool + public function decide(\Traversable $results, $asObject = false): AccessDecision|bool { $deny = 0; + $allVotes = []; + foreach ($results as $result) { + $allVotes[] = $result; + if ($result instanceof VoteInterface) { + $result = $result->getAccess(); + } + if (VoterInterface::ACCESS_GRANTED === $result) { - return true; + return $this->getReturn(true, $asObject, $allVotes); } if (VoterInterface::ACCESS_DENIED === $result) { @@ -43,14 +52,19 @@ public function decide(\Traversable $results): bool } if ($deny > 0) { - return false; + return $this->getReturn(false, $asObject, $allVotes); } - return $this->allowIfAllAbstainDecisions; + return $this->getReturn($this->allowIfAllAbstainDecisions, $asObject, $allVotes); } public function __toString(): string { return 'affirmative'; } + + private function getReturn(bool $result, bool $asObject, array $votes): AccessDecision|bool + { + return $asObject ? new AccessDecision($result, $votes) : $result; + } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/ConsensusStrategy.php index bff56513830f3..c6bd245f5027b 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; /** @@ -38,11 +40,18 @@ public function __construct( ) { } - public function decide(\Traversable $results): bool + public function decide(\Traversable $results, $asObject = false): AccessDecision|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,22 +60,27 @@ public function decide(\Traversable $results): bool } if ($grant > $deny) { - return true; + return $this->getReturn(true, $asObject, $allVotes); } if ($deny > $grant) { - return false; + return $this->getReturn(false, $asObject, $allVotes); } if ($grant > 0) { - return $this->allowIfEqualGrantedDeniedDecisions; + return $this->getReturn($this->allowIfEqualGrantedDeniedDecisions, $asObject, $allVotes); } - return $this->allowIfAllAbstainDecisions; + return $this->getReturn($this->allowIfAllAbstainDecisions, $asObject, $allVotes); } public function __toString(): string { return 'consensus'; } + + private function getReturn(bool $result, bool $asObject, array $votes): AccessDecision|bool + { + return $asObject ? new AccessDecision($result, $votes) : $result; + } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/PriorityStrategy.php index d7f566adbf19d..f7c0a1a1afd4a 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; /** @@ -30,23 +32,35 @@ public function __construct( ) { } - public function decide(\Traversable $results): bool + public function decide(\Traversable $results, $asObject = false): AccessDecision|bool { + $allVotes = []; + foreach ($results as $result) { + $allVotes[] = $result; + if ($result instanceof VoteInterface) { + $result = $result->getAccess(); + } + if (VoterInterface::ACCESS_GRANTED === $result) { - return true; + return $this->getReturn(true, $asObject, $allVotes); } if (VoterInterface::ACCESS_DENIED === $result) { - return false; + return $this->getReturn(false, $asObject, $allVotes); } } - return $this->allowIfAllAbstainDecisions; + return $this->getReturn($this->allowIfAllAbstainDecisions, $asObject, $allVotes); } public function __toString(): string { return 'priority'; } + + private function getReturn(bool $result, bool $asObject, array $votes): AccessDecision|bool + { + return $asObject ? new AccessDecision($result, $votes) : $result; + } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php b/src/Symfony/Component/Security/Core/Authorization/Strategy/UnanimousStrategy.php index d47d8994f86c4..133f8ceee480b 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; /** @@ -29,12 +31,19 @@ public function __construct( ) { } - public function decide(\Traversable $results): bool + public function decide(\Traversable $results, $asObject = false): AccessDecision|bool { $grant = 0; + $allVotes = []; + foreach ($results as $result) { + $allVotes[] = $result; + if ($result instanceof VoteInterface) { + $result = $result->getAccess(); + } + if (VoterInterface::ACCESS_DENIED === $result) { - return false; + return $this->getReturn(false, $asObject, $allVotes); } if (VoterInterface::ACCESS_GRANTED === $result) { @@ -44,14 +53,19 @@ public function decide(\Traversable $results): bool // no deny votes if ($grant > 0) { - return true; + return $this->getReturn(true, $asObject, $allVotes); } - return $this->allowIfAllAbstainDecisions; + return $this->getReturn($this->allowIfAllAbstainDecisions, $asObject, $allVotes); } public function __toString(): string { return 'unanimous'; } + + private function getReturn(bool $result, bool $asObject, array $votes): AccessDecision|bool + { + return $asObject ? new AccessDecision($result, $votes) : $result; + } } diff --git a/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/TraceableAccessDecisionManager.php index 0b82eb3a6d96d..9e4910b2a9091 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, bool $asObject = false): AccessDecision|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, $asObject); $currentDecisionLog['result'] = $result; @@ -70,7 +71,7 @@ public function decide(TokenInterface $token, array $attributes, mixed $object = * @param array $attributes attributes used for the vote * @param 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/UserAuthorizationChecker.php b/src/Symfony/Component/Security/Core/Authorization/UserAuthorizationChecker.php index f5ba7b8846e03..baa6e799e6016 100644 --- a/src/Symfony/Component/Security/Core/Authorization/UserAuthorizationChecker.php +++ b/src/Symfony/Component/Security/Core/Authorization/UserAuthorizationChecker.php @@ -26,6 +26,9 @@ public function __construct( public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null): bool { + /** + * @var bool + */ return $this->accessDecisionManager->decide(new UserAuthorizationCheckerToken($user), [$attribute], $subject); } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index 1abc7c704fb59..53b245c0462e4 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -30,9 +30,9 @@ public function __construct( ) { } - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function vote(TokenInterface $token, mixed $subject, array $attributes, bool $asObject = false): VoteInterface|int { - $result = $this->voter->vote($token, $subject, $attributes); + $result = $this->voter->vote($token, $subject, $attributes, $asObject); $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..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/Voter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php index 1f76a42eaf1b8..e91c8dae9451f 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/Voter.php @@ -24,7 +24,7 @@ */ abstract class Voter implements VoterInterface, CacheableVoterInterface { - public function vote(TokenInterface $token, mixed $subject, array $attributes): int + public function vote(TokenInterface $token, mixed $subject, array $attributes): VoteInterface|int { // abstain vote by default in case none of the attributes are supported $vote = self::ACCESS_ABSTAIN; diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterInterface.php index 5255c88e6ec0f..c3b065b093085 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|int; } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/VoterWithObjects.php b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterWithObjects.php new file mode 100644 index 0000000000000..f89695ea19828 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/VoterWithObjects.php @@ -0,0 +1,40 @@ + + * + * 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; + +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; + +/** + * Voter is an abstract default implementation of a voter. + * + * @author Roman Marintšenko + * @author Grégoire Pineau + * + * @template TAttribute of string + * @template TSubject of mixed + * + * @extends Voter + */ +abstract class VoterWithObjects extends Voter +{ + public function vote(TokenInterface $token, mixed $subject, array $attributes, bool $asObject = false): VoteInterface|int + { + $vote = $this->doVote($token, $subject, $attributes); + + return $asObject ? $vote : $vote->getAccess(); + } + + public function doVote(TokenInterface $token, mixed $subject, array $attributes): VoteInterface + { + throw new \LogicException(\sprintf('"vote" or "doVote" methods must be implemented in "%s"', static::class)); + } +} diff --git a/src/Symfony/Component/Security/Core/CHANGELOG.md b/src/Symfony/Component/Security/Core/CHANGELOG.md index 3cc738ce5b93c..8becbaafd392e 100644 --- a/src/Symfony/Component/Security/Core/CHANGELOG.md +++ b/src/Symfony/Component/Security/Core/CHANGELOG.md @@ -7,6 +7,7 @@ CHANGELOG * Add `UserAuthorizationChecker::isGrantedForUser()` to test user authorization without relying on the session. For example, users not currently logged in, or while processing a message from a message queue. * Add `OfflineTokenInterface` to mark tokens that do not represent the currently logged-in user + * 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..b8482bfe3308d 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,12 @@ public function getAttributes(): array return $this->attributes; } - public function getVote(): int + public function getVote($asObject = false): VoteInterface|int { + if ($this->vote instanceof VoteInterface && !$asObject) { + return $this->vote->getAccess(); + } + return $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..ecb13c5ccecdf 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; /** @@ -37,6 +40,11 @@ final public function testDecide(AccessDecisionStrategyInterface $strategy, arra $manager = new AccessDecisionManager($voters, $strategy); $this->assertSame($expected, $manager->decide($token, ['ROLE_FOO'])); + + $decision = $manager->decide($token, ['ROLE_FOO'], null, false, true); + + $this->assertInstanceOf(AccessDecision::class, $decision); + $this->assertSame($expected, $decision->getAccess()); } /** @@ -77,4 +85,19 @@ 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, bool $asObject = false): VoteInterface|int + { + return $asObject ? new Vote($this->vote) : $this->vote; + } + }; + } } diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index f0a4978860df9..49ac9f5b4961b 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -14,9 +14,12 @@ use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; +use Symfony\Component\Security\Core\Authorization\Voter\VoteInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class AccessDecisionManagerTest extends TestCase @@ -36,6 +39,7 @@ public function testVoterCalls() $voters = [ $this->getExpectedVoter(VoterInterface::ACCESS_DENIED), $this->getExpectedVoter(VoterInterface::ACCESS_GRANTED), + $this->getExpectedVoter(new Vote(VoterInterface::ACCESS_DENIED)), $this->getUnexpectedVoter(), ]; @@ -52,6 +56,11 @@ public function decide(\Traversable $results): bool case 1: Assert::assertSame(VoterInterface::ACCESS_GRANTED, $result); + break; + case 2: + Assert::assertInstanceOf(Vote::class, $result); + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result->getAccess()); + return true; } } @@ -65,6 +74,96 @@ 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), + $this->getExpectedVoter(new Vote(VoterInterface::ACCESS_DENIED)), + $this->getUnexpectedVoter(), + ]; + + $strategy = new class implements AccessDecisionStrategyInterface { + public function decide(\Traversable $results): 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(Vote::class, $result); + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result->getAccess()); + + return true; + } + } + + return false; + } + }; + + $manager = new AccessDecisionManager($voters, $strategy); + + $decision = $manager->decide($token, ['ROLE_FOO'], null, false, true); + $this->assertInstanceOf(AccessDecision::class, $decision); + $this->assertTrue($decision->getAccess()); + $this->assertEmpty($decision->getMessage()); + } + + public function testVoterCallsWithAccessDecisionObjectFromStrategy() + { + $token = $this->createMock(TokenInterface::class); + + $voters = [ + $this->getExpectedVoter(VoterInterface::ACCESS_DENIED), + $this->getExpectedVoter(VoterInterface::ACCESS_GRANTED), + $this->getExpectedVoter(new Vote(VoterInterface::ACCESS_DENIED)), + $this->getUnexpectedVoter(), + ]; + + $strategy = new class implements AccessDecisionStrategyInterface { + public function decide(\Traversable $results): AccessDecision + { + $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(Vote::class, $result); + Assert::assertSame(VoterInterface::ACCESS_DENIED, $result->getAccess()); + + return new AccessDecision(true, [], 'message from strategy'); + } + } + + return new AccessDecision(false, [], 'message from strategy'); + } + }; + + $manager = new AccessDecisionManager($voters, $strategy); + + $decision = $manager->decide($token, ['ROLE_FOO'], null, false, true); + $this->assertInstanceOf(AccessDecision::class, $decision); + $this->assertTrue($decision->getAccess()); + $this->assertSame('message from strategy', $decision->getMessage()); + } + public function testCacheableVoters() { $token = $this->createMock(TokenInterface::class); @@ -270,7 +369,7 @@ 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()) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php index 36b048c8976d1..447ea647f2340 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AuthorizationCheckerTest.php @@ -16,6 +16,7 @@ use Symfony\Component\Security\Core\Authentication\Token\NullToken; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\AuthorizationChecker; use Symfony\Component\Security\Core\User\InMemoryUser; @@ -77,4 +78,42 @@ 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']); + + $this->accessDecisionManager + ->expects($this->once()) + ->method('decide') + ->with($this->identicalTo($token), $this->identicalTo([$attribute])) + ->willReturn(true); + $this->tokenStorage->setToken($token); + + $decision = $this->authorizationChecker->isGranted($attribute, $token, true); + $this->assertInstanceOf(AccessDecision::class, $decision); + $this->assertTrue($decision->getAccess()); + $this->assertEmpty($decision->getMessage()); + } + + public function testIsGrantedWithAccessDecisionObjectFromADM() + { + $attribute = new \stdClass(); + + $token = new UsernamePasswordToken(new InMemoryUser('username', 'password', ['ROLE_USER']), 'provider', ['ROLE_USER']); + + $this->accessDecisionManager + ->expects($this->once()) + ->method('decide') + ->with($this->identicalTo($token), $this->identicalTo([$attribute])) + ->willReturn(new AccessDecision(true, [], 'from accessDecisionManager')); + $this->tokenStorage->setToken($token); + + $decision = $this->authorizationChecker->isGranted($attribute, $token, true); + $this->assertInstanceOf(AccessDecision::class, $decision); + $this->assertTrue($decision->getAccess()); + $this->assertSame('from accessDecisionManager', $decision->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..60175fe4f4a42 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/AffirmativeStrategyTest.php @@ -25,6 +25,27 @@ public static function provideStrategyTests(): iterable yield [$strategy, self::getVoters(0, 1, 0), false]; yield [$strategy, self::getVoters(0, 0, 1), false]; + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], true]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(0), + ], false]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(1), + self::getVoter(0), + self::getVoterWithVoteObject(-1), + ], true]; + $strategy = new AffirmativeStrategy(true); yield [$strategy, self::getVoters(0, 0, 1), true]; 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..dbd5e01aa419b 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/ConsensusStrategyTest.php @@ -36,5 +36,26 @@ public static function provideStrategyTests(): iterable yield [$strategy, self::getVoters(2, 2, 0), false]; yield [$strategy, self::getVoters(2, 2, 1), false]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], true]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(-1), + self::getVoterWithVoteObject(1), + ], false]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(-1), + self::getVoterWithVoteObject(0), + ], false]; } } 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..905eadaada6da 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/PriorityStrategyTest.php @@ -35,6 +35,27 @@ public static function provideStrategyTests(): iterable self::getVoter(VoterInterface::ACCESS_GRANTED), ], false]; + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], true]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], false]; + + yield [$strategy, [ + self::getVoterWithVoteObject(-1), + self::getVoter(1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], false]; + yield [$strategy, self::getVoters(0, 0, 2), false]; $strategy = new PriorityStrategy(true); 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..7935712b8ba2c 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Strategy/UnanimousStrategyTest.php @@ -29,5 +29,26 @@ public static function provideStrategyTests(): iterable $strategy = new UnanimousStrategy(true); yield [$strategy, self::getVoters(0, 0, 2), true]; + + yield [$strategy, [ + self::getVoterWithVoteObject(1), + self::getVoter(-1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], false]; + + yield [$strategy, [ + self::getVoterWithVoteObject(-1), + self::getVoter(1), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], false]; + + yield [$strategy, [ + self::getVoterWithVoteObject(0), + self::getVoter(0), + self::getVoter(0), + self::getVoterWithVoteObject(1), + ], true]; } } 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..1ed53b253e39b 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php @@ -15,6 +15,7 @@ use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Event\VoteEvent; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; @@ -53,6 +54,30 @@ 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 + ->expects($this->once()) + ->method('vote') + ->with($token, 'anysubject', ['attr1']) + ->willReturn(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); + $result = $sut->vote($token, 'anysubject', ['attr1'], true); + + $this->assertSame(VoterInterface::ACCESS_DENIED, $result->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..6880bd2560174 --- /dev/null +++ b/src/Symfony/Component/Security/Core/Tests/Fixtures/DummyVoterWithObject.php @@ -0,0 +1,23 @@ + + * + * 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 vote(TokenInterface $token, $subject, array $attributes, $asObject = true): VoteInterface|int + { + } +} diff --git a/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php b/src/Symfony/Component/Security/Http/EventListener/IsGrantedAttributeListener.php index c511cf04e2398..2ea2f271cbc46 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,13 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo } } - if (!$this->authChecker->isGranted($attribute->attribute, $subject)) { + $decision = $this->authChecker->isGranted($attribute->attribute, $subject, true); + + if(!$decision instanceof AccessDecision) { + $decision = new AccessDecision($decision); + } + + if (!$decision->isGranted()) { $message = $attribute->message ?: \sprintf('Access Denied by #[IsGranted(%s)] on controller', $this->getIsGrantedString($attribute)); if ($statusCode = $attribute->statusCode) { @@ -69,6 +76,7 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo $accessDeniedException = new AccessDeniedException($message, code: $attribute->exceptionCode ?? 403); $accessDeniedException->setAttributes($attribute->attribute); $accessDeniedException->setSubject($subject); + $accessDeniedException->setAccessDecision($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..10d9f25dff7aa 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,23 @@ public function authenticate(RequestEvent $event): void $token = $this->tokenStorage->getToken() ?? new NullToken(); - if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) { - throw $this->createAccessDeniedException($request, $attributes); + $decision = $this->accessDecisionManager->decide($token, $attributes, $request, true, AccessDecision::RETURN_AS_OBJECT); + + if(! $decision instanceof AccessDecision) { + $decision = new AccessDecision($decision); + } + + if ($decision->isDenied()) { + throw $this->createAccessDeniedException($request, $attributes, $decision); } } - private function createAccessDeniedException(Request $request, array $attributes): AccessDeniedException + private function createAccessDeniedException(Request $request, array $attributes, AccessDecision $accessDecision): AccessDeniedException { $exception = new AccessDeniedException(); $exception->setAttributes($attributes); $exception->setSubject($request); + $exception->setAccessDecision($accessDecision); return $exception; } diff --git a/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php b/src/Symfony/Component/Security/Http/Firewall/SwitchUserListener.php index ffdad52eef207..47341fbd909e6 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,9 +155,15 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn throw $e; } - if (false === $this->accessDecisionManager->decide($token, [$this->role], $user)) { + $decision = $this->accessDecisionManager->decide($token, [$this->role], $user, false, true); + if(! $decision instanceof AccessDecision) { + $decision = new AccessDecision($decision); + } + + if ($decision->isDenied()) { $exception = new AccessDeniedException(); $exception->setAttributes($this->role); + $exception->setAccessDecision($decision); 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..833bc99bf8ead 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,33 @@ public function testAccessDeniedExceptionWithExceptionCode() $listener->onKernelControllerArguments($event); } + + public function testAccessDeniedExceptionWithExceptionCodeAndWithObject() + { + $authChecker = $this->createMock(AuthorizationCheckerInterface::class); + $authChecker->expects($this->any()) + ->method('isGranted') + ->willReturn(new AccessDecision(false, [], 'User denied')); + + $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 f8c0b62e4ec51..b369a667e6c4f 100644 --- a/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php @@ -20,6 +20,7 @@ use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage; use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\Authorization\AccessDecision; use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface; use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Exception\AccessDeniedException; @@ -282,4 +283,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 = $this->createMock(AccessDecisionManagerInterface::class); + $accessDecisionManager + ->expects($this->once()) + ->method('decide') + ->with($this->equalTo($token), $this->equalTo(['foo' => 'bar']), $this->equalTo($request)) + ->willReturn(new AccessDecision(false,[], 'not allowed')) + ; + + $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..83d868977189b 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,30 @@ 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'); + + $this->accessDecisionManager->expects($this->once()) + ->method('decide')->with($token, ['ROLE_ALLOWED_TO_SWITCH']) + ->willReturn(new AccessDecision(false, [new Vote(false)], 'User unable to switch')); + + $this->expectException(AccessDeniedException::class); + + try{ + $listener = new SwitchUserListener($this->tokenStorage, $this->userProvider, $this->userChecker, 'provider123', $this->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; + } + } }