diff --git a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php index 82f9e0ae827d3..e9ef8e2b41912 100644 --- a/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php +++ b/src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Authorization; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; use Symfony\Component\Security\Core\Exception\InvalidArgumentException; @@ -29,6 +30,8 @@ class AccessDecisionManager implements AccessDecisionManagerInterface public const STRATEGY_PRIORITY = 'priority'; private $voters; + private $votersCacheAttributes; + private $votersCacheObject; private $strategy; private $allowIfAllAbstainDecisions; private $allowIfEqualGrantedDeniedDecisions; @@ -80,7 +83,7 @@ public function decide(TokenInterface $token, array $attributes, $object = null/ private function decideAffirmative(TokenInterface $token, array $attributes, $object = null): bool { $deny = 0; - foreach ($this->voters as $voter) { + foreach ($this->getVoters($attributes, $object) as $voter) { $result = $voter->vote($token, $object, $attributes); if (VoterInterface::ACCESS_GRANTED === $result) { @@ -119,7 +122,7 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje { $grant = 0; $deny = 0; - foreach ($this->voters as $voter) { + foreach ($this->getVoters($attributes, $object) as $voter) { $result = $voter->vote($token, $object, $attributes); if (VoterInterface::ACCESS_GRANTED === $result) { @@ -155,7 +158,7 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje private function decideUnanimous(TokenInterface $token, array $attributes, $object = null): bool { $grant = 0; - foreach ($this->voters as $voter) { + foreach ($this->getVoters($attributes, $object) as $voter) { foreach ($attributes as $attribute) { $result = $voter->vote($token, $object, [$attribute]); @@ -188,7 +191,7 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje */ private function decidePriority(TokenInterface $token, array $attributes, $object = null) { - foreach ($this->voters as $voter) { + foreach ($this->getVoters($attributes, $object) as $voter) { $result = $voter->vote($token, $object, $attributes); if (VoterInterface::ACCESS_GRANTED === $result) { @@ -206,4 +209,48 @@ private function decidePriority(TokenInterface $token, array $attributes, $objec return $this->allowIfAllAbstainDecisions; } + + private function getVoters(array $attributes, $object = null): iterable + { + $keyAttributes = []; + foreach ($attributes as $attribute) { + $keyAttributes[] = \is_string($attribute) ? $attribute : null; + } + // use `get_class` to handle anonymous classes + $keyObject = \is_object($object) ? \get_class($object) : get_debug_type($object); + foreach ($this->voters as $key => $voter) { + if (!$voter instanceof CacheableVoterInterface) { + yield $voter; + continue; + } + + $supports = true; + // The voter supports the attributes if it supports at least one attribute of the list + foreach ($keyAttributes as $keyAttribute) { + if (null === $keyAttribute) { + $supports = true; + } elseif (!isset($this->votersCacheAttributes[$keyAttribute][$key])) { + $this->votersCacheAttributes[$keyAttribute][$key] = $supports = $voter->supportsAttribute($keyAttribute); + } else { + $supports = $this->votersCacheAttributes[$keyAttribute][$key]; + } + if ($supports) { + break; + } + } + if (!$supports) { + continue; + } + + if (!isset($this->votersCacheObject[$keyObject][$key])) { + $this->votersCacheObject[$keyObject][$key] = $supports = $voter->supportsType($keyObject); + } else { + $supports = $this->votersCacheObject[$keyObject][$key]; + } + if (!$supports) { + continue; + } + yield $voter; + } + } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php index 577d509cd7826..aa1ec90d9aa24 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php @@ -24,7 +24,7 @@ * @author Fabien Potencier * @author Johannes M. Schmitt */ -class AuthenticatedVoter implements VoterInterface +class AuthenticatedVoter implements CacheableVoterInterface { public const IS_AUTHENTICATED_FULLY = 'IS_AUTHENTICATED_FULLY'; public const IS_AUTHENTICATED_REMEMBERED = 'IS_AUTHENTICATED_REMEMBERED'; @@ -116,4 +116,23 @@ public function vote(TokenInterface $token, $subject, array $attributes) return $result; } + + public function supportsAttribute(string $attribute): bool + { + return \in_array($attribute, [ + self::IS_AUTHENTICATED_FULLY, + self::IS_AUTHENTICATED_REMEMBERED, + self::IS_AUTHENTICATED_ANONYMOUSLY, + self::IS_AUTHENTICATED, + self::IS_ANONYMOUS, + self::IS_IMPERSONATOR, + self::IS_REMEMBERED, + self::PUBLIC_ACCESS, + ], true); + } + + public function supportsType(string $subjectType): bool + { + return true; + } } diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/CacheableVoterInterface.php b/src/Symfony/Component/Security/Core/Authorization/Voter/CacheableVoterInterface.php new file mode 100644 index 0000000000000..875aad6601a1a --- /dev/null +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/CacheableVoterInterface.php @@ -0,0 +1,30 @@ + + * + * 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; + +/** + * Let voters expose the attributes and types they care about. + * + * By returning false to either `supportsAttribute` or `supportsType`, the + * voter will never be called for the specified attribute or subject. + * + * @author Jérémy Derussé + */ +interface CacheableVoterInterface extends VoterInterface +{ + public function supportsAttribute(string $attribute): bool; + + /** + * @param string $subjectType The type of the subject inferred by `get_class` or `get_debug_type` + */ + public function supportsType(string $subjectType): bool; +} diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php index 661acab20f5cc..f1d993a619346 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php @@ -18,7 +18,7 @@ * * @author Fabien Potencier */ -class RoleVoter implements VoterInterface +class RoleVoter implements CacheableVoterInterface { private $prefix; @@ -55,6 +55,16 @@ public function vote(TokenInterface $token, $subject, array $attributes) return $result; } + public function supportsAttribute(string $attribute): bool + { + return str_starts_with($attribute, $this->prefix); + } + + public function supportsType(string $subjectType): bool + { + return true; + } + protected function extractRoles(TokenInterface $token) { return $token->getRoleNames(); diff --git a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php index 48f7651256710..f606f1d275d36 100644 --- a/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php +++ b/src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php @@ -22,7 +22,7 @@ * * @internal */ -class TraceableVoter implements VoterInterface +class TraceableVoter implements CacheableVoterInterface { private $voter; private $eventDispatcher; @@ -46,4 +46,14 @@ public function getDecoratedVoter(): VoterInterface { return $this->voter; } + + public function supportsAttribute(string $attribute): bool + { + return !$this->voter instanceof CacheableVoterInterface || $this->voter->supportsAttribute($attribute); + } + + public function supportsType(string $subjectType): bool + { + return !$this->voter instanceof CacheableVoterInterface || $this->voter->supportsType($subjectType); + } } diff --git a/src/Symfony/Component/Security/Core/CHANGELOG.md b/src/Symfony/Component/Security/Core/CHANGELOG.md index 4ae5309c857b3..034d634a5d02a 100644 --- a/src/Symfony/Component/Security/Core/CHANGELOG.md +++ b/src/Symfony/Component/Security/Core/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG 5.4 --- + * Add a `CacheableVoterInterface` for voters that vote only on identified attributes and subjects * Deprecate `AuthenticationEvents::AUTHENTICATION_FAILURE`, use the `LoginFailureEvent` instead * Deprecate `AnonymousToken`, as the related authenticator was deprecated in 5.3 * Deprecate `Token::getCredentials()`, tokens should no longer contain credentials (as they represent authenticated sessions) diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php index 375fb6d6d49ef..cef99a65b2e75 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php @@ -15,6 +15,7 @@ use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Authorization\AccessDecisionManager; +use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class AccessDecisionManagerTest extends TestCase @@ -120,6 +121,143 @@ public function provideStrategies() yield [AccessDecisionManager::STRATEGY_PRIORITY]; } + public function testCacheableVoters() + { + $token = $this->createMock(TokenInterface::class); + $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter + ->expects($this->once()) + ->method('supportsAttribute') + ->with('foo') + ->willReturn(true); + $voter + ->expects($this->once()) + ->method('supportsType') + ->with('string') + ->willReturn(true); + $voter + ->expects($this->once()) + ->method('vote') + ->with($token, 'bar', ['foo']) + ->willReturn(VoterInterface::ACCESS_GRANTED); + + $manager = new AccessDecisionManager([$voter]); + $this->assertTrue($manager->decide($token, ['foo'], 'bar')); + } + + public function testCacheableVotersIgnoresNonStringAttributes() + { + $token = $this->createMock(TokenInterface::class); + $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter + ->expects($this->never()) + ->method('supportsAttribute'); + $voter + ->expects($this->once()) + ->method('supportsType') + ->with('string') + ->willReturn(true); + $voter + ->expects($this->once()) + ->method('vote') + ->with($token, 'bar', [1337]) + ->willReturn(VoterInterface::ACCESS_GRANTED); + + $manager = new AccessDecisionManager([$voter]); + $this->assertTrue($manager->decide($token, [1337], 'bar')); + } + + public function testCacheableVotersWithMultipleAttributes() + { + $token = $this->createMock(TokenInterface::class); + $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter + ->expects($this->exactly(2)) + ->method('supportsAttribute') + ->withConsecutive(['foo'], ['bar']) + ->willReturnOnConsecutiveCalls(false, true); + $voter + ->expects($this->once()) + ->method('supportsType') + ->with('string') + ->willReturn(true); + $voter + ->expects($this->once()) + ->method('vote') + ->with($token, 'bar', ['foo', 'bar']) + ->willReturn(VoterInterface::ACCESS_GRANTED); + + $manager = new AccessDecisionManager([$voter]); + $this->assertTrue($manager->decide($token, ['foo', 'bar'], 'bar', true)); + } + + public function testCacheableVotersWithEmptyAttributes() + { + $token = $this->createMock(TokenInterface::class); + $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter + ->expects($this->never()) + ->method('supportsAttribute'); + $voter + ->expects($this->once()) + ->method('supportsType') + ->with('string') + ->willReturn(true); + $voter + ->expects($this->once()) + ->method('vote') + ->with($token, 'bar', []) + ->willReturn(VoterInterface::ACCESS_GRANTED); + + $manager = new AccessDecisionManager([$voter]); + $this->assertTrue($manager->decide($token, [], 'bar')); + } + + public function testCacheableVotersSupportsMethodsCalledOnce() + { + $token = $this->createMock(TokenInterface::class); + $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter + ->expects($this->once()) + ->method('supportsAttribute') + ->with('foo') + ->willReturn(true); + $voter + ->expects($this->once()) + ->method('supportsType') + ->with('string') + ->willReturn(true); + $voter + ->expects($this->exactly(2)) + ->method('vote') + ->with($token, 'bar', ['foo']) + ->willReturn(VoterInterface::ACCESS_GRANTED); + + $manager = new AccessDecisionManager([$voter]); + $this->assertTrue($manager->decide($token, ['foo'], 'bar')); + $this->assertTrue($manager->decide($token, ['foo'], 'bar')); + } + + public function testCacheableVotersNotCalled() + { + $token = $this->createMock(TokenInterface::class); + $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $voter + ->expects($this->once()) + ->method('supportsAttribute') + ->with('foo') + ->willReturn(false); + $voter + ->expects($this->never()) + ->method('supportsType'); + $voter + ->expects($this->never()) + ->method('vote'); + + $manager = new AccessDecisionManager([$voter]); + $this->assertFalse($manager->decide($token, ['foo'], 'bar')); + } + protected function getVoters($grants, $denies, $abstains) { $voters = []; diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php index 32633ac68d17a..96890ab5264b8 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php @@ -83,6 +83,40 @@ public function getLegacyVoteTests() ]; } + /** + * @dataProvider provideAttributes + */ + public function testSupportsAttribute(string $attribute, bool $expected) + { + $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); + + $this->assertSame($expected, $voter->supportsAttribute($attribute)); + } + + public function provideAttributes() + { + yield [AuthenticatedVoter::IS_AUTHENTICATED_FULLY, true]; + yield [AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED, true]; + yield [AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY, true]; + yield [AuthenticatedVoter::IS_ANONYMOUS, true]; + yield [AuthenticatedVoter::IS_AUTHENTICATED, true]; + yield [AuthenticatedVoter::IS_IMPERSONATOR, true]; + yield [AuthenticatedVoter::IS_REMEMBERED, true]; + yield [AuthenticatedVoter::PUBLIC_ACCESS, true]; + + yield ['', false]; + yield ['foo', false]; + } + + public function testSupportsType() + { + $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); + + $this->assertTrue($voter->supportsType(get_debug_type('foo'))); + $this->assertTrue($voter->supportsType(get_debug_type(null))); + $this->assertTrue($voter->supportsType(get_debug_type(new \StdClass()))); + } + protected function getToken($authenticated) { $user = new InMemoryUser('wouter', '', ['ROLE_USER']); diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php index 43f802481e413..912368caa02e3 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/RoleVoterTest.php @@ -13,7 +13,9 @@ use PHPUnit\Framework\TestCase; use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; +use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolver; use Symfony\Component\Security\Core\Authentication\Token\AbstractToken; +use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter; use Symfony\Component\Security\Core\Authorization\Voter\RoleVoter; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; @@ -58,6 +60,35 @@ public function testDeprecatedRolePreviousAdmin() $voter->vote($this->getTokenWithRoleNames(['ROLE_USER', 'ROLE_PREVIOUS_ADMIN']), null, ['ROLE_PREVIOUS_ADMIN']); } + /** + * @dataProvider provideAttributes + */ + public function testSupportsAttribute(string $prefix, string $attribute, bool $expected) + { + $voter = new RoleVoter($prefix); + + $this->assertSame($expected, $voter->supportsAttribute($attribute)); + } + + public function provideAttributes() + { + yield ['ROLE_', 'ROLE_foo', true]; + yield ['ROLE_', 'ROLE_', true]; + yield ['FOO_', 'FOO_bar', true]; + + yield ['ROLE_', '', false]; + yield ['ROLE_', 'foo', false]; + } + + public function testSupportsType() + { + $voter = new AuthenticatedVoter(new AuthenticationTrustResolver()); + + $this->assertTrue($voter->supportsType(get_debug_type('foo'))); + $this->assertTrue($voter->supportsType(get_debug_type(null))); + $this->assertTrue($voter->supportsType(get_debug_type(new \StdClass()))); + } + protected function getTokenWithRoleNames(array $roles) { $token = $this->createMock(AbstractToken::class); diff --git a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php index 53e6a277453f5..d0f8ae08f97db 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authorization/Voter/TraceableVoterTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\TestCase; 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\VoterInterface; use Symfony\Component\Security\Core\Event\VoteEvent; @@ -51,4 +52,56 @@ public function testVote() $this->assertSame(VoterInterface::ACCESS_DENIED, $result); } + + public function testSupportsAttributeOnCacheable() + { + $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass(); + + $voter + ->expects($this->once()) + ->method('supportsAttribute') + ->with('foo') + ->willReturn(false); + + $sut = new TraceableVoter($voter, $eventDispatcher); + + $this->assertFalse($sut->supportsAttribute('foo')); + } + + public function testSupportsTypeOnCacheable() + { + $voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass(); + $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass(); + + $voter + ->expects($this->once()) + ->method('supportsType') + ->with('foo') + ->willReturn(false); + + $sut = new TraceableVoter($voter, $eventDispatcher); + + $this->assertFalse($sut->supportsType('foo')); + } + + public function testSupportsAttributeOnNonCacheable() + { + $voter = $this->getMockBuilder(VoterInterface::class)->getMockForAbstractClass(); + $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass(); + + $sut = new TraceableVoter($voter, $eventDispatcher); + + $this->assertTrue($sut->supportsAttribute('foo')); + } + + public function testSupportsTypeOnNonCacheable() + { + $voter = $this->getMockBuilder(VoterInterface::class)->getMockForAbstractClass(); + $eventDispatcher = $this->getMockBuilder(EventDispatcherInterface::class)->getMockForAbstractClass(); + + $sut = new TraceableVoter($voter, $eventDispatcher); + + $this->assertTrue($sut->supportsType('foo')); + } }