Skip to content

Commit e896c7b

Browse files
jderussejderusse
committed
Cache voters that will always abstain
1 parent c0bf036 commit e896c7b

File tree

9 files changed

+360
-7
lines changed

9 files changed

+360
-7
lines changed

‎src/Symfony/Component/Security/Core/Authorization/AccessDecisionManager.php

+34-4
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Security\Core\Authorization;
1313

1414
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
15+
use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface;
1516
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
1617
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
1718

@@ -29,6 +30,8 @@ class AccessDecisionManager implements AccessDecisionManagerInterface
2930
public const STRATEGY_PRIORITY = 'priority';
3031

3132
private $voters;
33+
private $votersCacheAttributes;
34+
private $votersCacheObject;
3235
private $strategy;
3336
private $allowIfAllAbstainDecisions;
3437
private $allowIfEqualGrantedDeniedDecisions;
@@ -80,7 +83,7 @@ public function decide(TokenInterface $token, array $attributes, $object = null/
8083
private function decideAffirmative(TokenInterface $token, array $attributes, $object = null): bool
8184
{
8285
$deny = 0;
83-
foreach ($this->voters as $voter) {
86+
foreach ($this->getVoters($attributes, $object) as $voter) {
8487
$result = $voter->vote($token, $object, $attributes);
8588

8689
if (VoterInterface::ACCESS_GRANTED === $result) {
@@ -119,7 +122,7 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje
119122
{
120123
$grant = 0;
121124
$deny = 0;
122-
foreach ($this->voters as $voter) {
125+
foreach ($this->getVoters($attributes, $object) as $voter) {
123126
$result = $voter->vote($token, $object, $attributes);
124127

125128
if (VoterInterface::ACCESS_GRANTED === $result) {
@@ -155,7 +158,7 @@ private function decideConsensus(TokenInterface $token, array $attributes, $obje
155158
private function decideUnanimous(TokenInterface $token, array $attributes, $object = null): bool
156159
{
157160
$grant = 0;
158-
foreach ($this->voters as $voter) {
161+
foreach ($this->getVoters($attributes, $object) as $voter) {
159162
foreach ($attributes as $attribute) {
160163
$result = $voter->vote($token, $object, [$attribute]);
161164

@@ -188,7 +191,7 @@ private function decideUnanimous(TokenInterface $token, array $attributes, $obje
188191
*/
189192
private function decidePriority(TokenInterface $token, array $attributes, $object = null)
190193
{
191-
foreach ($this->voters as $voter) {
194+
foreach ($this->getVoters($attributes, $object) as $voter) {
192195
$result = $voter->vote($token, $object, $attributes);
193196

194197
if (VoterInterface::ACCESS_GRANTED === $result) {
@@ -206,4 +209,31 @@ private function decidePriority(TokenInterface $token, array $attributes, $objec
206209

207210
return $this->allowIfAllAbstainDecisions;
208211
}
212+
213+
private function getVoters(array $attributes, $object = null): iterable
214+
{
215+
$keyAttributes = count($attributes) === 1 && is_string($attributes[0]) ? $attributes[0] : null;
216+
$keyObject = get_debug_type($object);
217+
218+
foreach ($this->voters as $key => $voter) {
219+
if (!isset($this->votersCacheAttributes[$keyAttributes][$key])) {
220+
$this->votersCacheAttributes[$keyAttributes][$key] = $supports = $keyAttributes === null || !$voter instanceof CacheableVoterInterface || $voter->supportsAttribute($keyAttributes);
221+
} else {
222+
$supports = $this->votersCacheAttributes[$keyAttributes][$key];
223+
}
224+
if (!$supports) {
225+
continue;
226+
}
227+
228+
if (!isset($this->votersCacheObject[$keyObject][$key])) {
229+
$this->votersCacheObject[$keyObject][$key] = $supports = !$voter instanceof CacheableVoterInterface || $voter->supportsType($keyObject);
230+
} else {
231+
$supports = $this->votersCacheObject[$keyObject][$key];
232+
}
233+
if (!$supports) {
234+
continue;
235+
}
236+
yield $voter;
237+
}
238+
}
209239
}

‎src/Symfony/Component/Security/Core/Authorization/Voter/AuthenticatedVoter.php

+20-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
* @author Fabien Potencier <fabien@symfony.com>
2525
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
2626
*/
27-
class AuthenticatedVoter implements VoterInterface
27+
class AuthenticatedVoter implements VoterInterface, CacheableVoterInterface
2828
{
2929
public const IS_AUTHENTICATED_FULLY = 'IS_AUTHENTICATED_FULLY';
3030
public const IS_AUTHENTICATED_REMEMBERED = 'IS_AUTHENTICATED_REMEMBERED';
@@ -116,4 +116,23 @@ public function vote(TokenInterface $token, $subject, array $attributes)
116116

117117
return $result;
118118
}
119+
120+
public function supportsAttribute(string $attribute): bool
121+
{
122+
return in_array($attribute, [
123+
self::IS_AUTHENTICATED_FULLY,
124+
self::IS_AUTHENTICATED_REMEMBERED,
125+
self::IS_AUTHENTICATED_ANONYMOUSLY,
126+
self::IS_AUTHENTICATED,
127+
self::IS_ANONYMOUS,
128+
self::IS_IMPERSONATOR,
129+
self::IS_REMEMBERED,
130+
self::PUBLIC_ACCESS,
131+
], true);
132+
}
133+
134+
public function supportsType(string $objectType): bool
135+
{
136+
return true;
137+
}
119138
}

‎src/Symfony/Component/Security/Core/Authorization/Voter/CacheableVoterInterface.php

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Core\Authorization\Voter;
13+
14+
/**
15+
* Marker interface for voters that know if they will always abstain for the
16+
* give attributes.
17+
*
18+
* By implementing this interface, the voter will never be called for the
19+
* specified attributes.
20+
*
21+
* @author Jérémy Derussé <jeremy@derusse.com>
22+
*/
23+
interface CacheableVoterInterface extends VoterInterface
24+
{
25+
public function supportsAttribute(string $attribute): bool;
26+
public function supportsType(string $objectType): bool;
27+
}

‎src/Symfony/Component/Security/Core/Authorization/Voter/RoleVoter.php

+11-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*
1919
* @author Fabien Potencier <fabien@symfony.com>
2020
*/
21-
class RoleVoter implements VoterInterface
21+
class RoleVoter implements VoterInterface, CacheableVoterInterface
2222
{
2323
private $prefix;
2424

@@ -55,6 +55,16 @@ public function vote(TokenInterface $token, $subject, array $attributes)
5555
return $result;
5656
}
5757

58+
public function supportsAttribute(string $attribute): bool
59+
{
60+
return str_starts_with($attribute, $this->prefix);
61+
}
62+
63+
public function supportsType(string $objectType): bool
64+
{
65+
return true;
66+
}
67+
5868
protected function extractRoles(TokenInterface $token)
5969
{
6070
return $token->getRoleNames();

‎src/Symfony/Component/Security/Core/Authorization/Voter/TraceableVoter.php

+11-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
*
2323
* @internal
2424
*/
25-
class TraceableVoter implements VoterInterface
25+
class TraceableVoter implements VoterInterface, CacheableVoterInterface
2626
{
2727
private $voter;
2828
private $eventDispatcher;
@@ -46,4 +46,14 @@ public function getDecoratedVoter(): VoterInterface
4646
{
4747
return $this->voter;
4848
}
49+
50+
public function supportsAttribute(string $attribute): bool
51+
{
52+
return !$this->voter instanceof CacheableVoterInterface || $this->voter->supportsAttribute($attribute);
53+
}
54+
55+
public function supportsType(string $objectType): bool
56+
{
57+
return !$this->voter instanceof CacheableVoterInterface || $this->voter->supportsType($objectType);
58+
}
4959
}

‎src/Symfony/Component/Security/Core/Tests/Authorization/AccessDecisionManagerTest.php

+136
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
1616
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
1717
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
18+
use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface;
1819
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
1920

2021
class AccessDecisionManagerTest extends TestCase
@@ -120,6 +121,141 @@ public function provideStrategies()
120121
yield [AccessDecisionManager::STRATEGY_PRIORITY];
121122
}
122123

124+
public function testCacheableVoters()
125+
{
126+
$token = $this->createMock(TokenInterface::class);
127+
$voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
128+
$voter
129+
->expects($this->once())
130+
->method('supportsAttribute')
131+
->with('foo')
132+
->willReturn(true);
133+
$voter
134+
->expects($this->once())
135+
->method('supportsType')
136+
->with('string')
137+
->willReturn(true);
138+
$voter
139+
->expects($this->once())
140+
->method('vote')
141+
->with($token, 'bar', ['foo'])
142+
->willReturn(VoterInterface::ACCESS_GRANTED);
143+
144+
$manager = new AccessDecisionManager([$voter]);
145+
$this->assertSame(true, $manager->decide($token, ['foo'], 'bar'));
146+
}
147+
148+
public function testCacheableVotersIgnoresNonStringAttributes()
149+
{
150+
$token = $this->createMock(TokenInterface::class);
151+
$voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
152+
$voter
153+
->expects($this->never())
154+
->method('supportsAttribute');
155+
$voter
156+
->expects($this->once())
157+
->method('supportsType')
158+
->with('string')
159+
->willReturn(true);
160+
$voter
161+
->expects($this->once())
162+
->method('vote')
163+
->with($token, 'bar', [1337])
164+
->willReturn(VoterInterface::ACCESS_GRANTED);
165+
166+
$manager = new AccessDecisionManager([$voter]);
167+
$this->assertSame(true, $manager->decide($token, [1337], 'bar'));
168+
}
169+
170+
public function testCacheableVotersIgnoresMultipleAttributes()
171+
{
172+
$token = $this->createMock(TokenInterface::class);
173+
$voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
174+
$voter
175+
->expects($this->never())
176+
->method('supportsAttribute');
177+
$voter
178+
->expects($this->once())
179+
->method('supportsType')
180+
->with('string')
181+
->willReturn(true);
182+
$voter
183+
->expects($this->once())
184+
->method('vote')
185+
->with($token, 'bar', ['foo', 'bar'])
186+
->willReturn(VoterInterface::ACCESS_GRANTED);
187+
188+
$manager = new AccessDecisionManager([$voter]);
189+
$this->assertSame(true, $manager->decide($token, ['foo', 'bar'], 'bar', true));
190+
}
191+
192+
public function testCacheableVotersIgnoresEmptyAttributes()
193+
{
194+
$token = $this->createMock(TokenInterface::class);
195+
$voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
196+
$voter
197+
->expects($this->never())
198+
->method('supportsAttribute');
199+
$voter
200+
->expects($this->once())
201+
->method('supportsType')
202+
->with('string')
203+
->willReturn(true);
204+
$voter
205+
->expects($this->once())
206+
->method('vote')
207+
->with($token, 'bar', [])
208+
->willReturn(VoterInterface::ACCESS_GRANTED);
209+
210+
$manager = new AccessDecisionManager([$voter]);
211+
$this->assertSame(true, $manager->decide($token, [], 'bar'));
212+
}
213+
214+
public function testCacheableVotersSupportsMethodsCalledOnce()
215+
{
216+
$token = $this->createMock(TokenInterface::class);
217+
$voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
218+
$voter
219+
->expects($this->once())
220+
->method('supportsAttribute')
221+
->with('foo')
222+
->willReturn(true);
223+
$voter
224+
->expects($this->once())
225+
->method('supportsType')
226+
->with('string')
227+
->willReturn(true);
228+
$voter
229+
->expects($this->exactly(2))
230+
->method('vote')
231+
->with($token, 'bar', ['foo'])
232+
->willReturn(VoterInterface::ACCESS_GRANTED);
233+
234+
$manager = new AccessDecisionManager([$voter]);
235+
$this->assertSame(true, $manager->decide($token, ['foo'], 'bar'));
236+
$this->assertSame(true, $manager->decide($token, ['foo'], 'bar'));
237+
}
238+
239+
public function testCacheableVotersNotCalled()
240+
{
241+
$token = $this->createMock(TokenInterface::class);
242+
$voter = $this->getMockBuilder(CacheableVoterInterface::class)->getMockForAbstractClass();
243+
$voter
244+
->expects($this->once())
245+
->method('supportsAttribute')
246+
->with('foo')
247+
->willReturn(false);
248+
$voter
249+
->expects($this->never())
250+
->method('supportsType');
251+
$voter
252+
->expects($this->never())
253+
->method('vote');
254+
255+
$manager = new AccessDecisionManager([$voter]);
256+
$this->assertSame(false, $manager->decide($token, ['foo'], 'bar'));
257+
}
258+
123259
protected function getVoters($grants, $denies, $abstains)
124260
{
125261
$voters = [];

‎src/Symfony/Component/Security/Core/Tests/Authorization/Voter/AuthenticatedVoterTest.php

+34
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,40 @@ public function getLegacyVoteTests()
8383
];
8484
}
8585

86+
/**
87+
* @dataProvider provideAttributes
88+
*/
89+
public function testSupportsAttribute(string $attribute, bool $expected)
90+
{
91+
$voter = new AuthenticatedVoter(new AuthenticationTrustResolver());
92+
93+
$this->assertSame($expected, $voter->supportsAttribute($attribute));
94+
}
95+
96+
public function provideAttributes()
97+
{
98+
yield [AuthenticatedVoter::IS_AUTHENTICATED_FULLY, true];
99+
yield [AuthenticatedVoter::IS_AUTHENTICATED_REMEMBERED, true];
100+
yield [AuthenticatedVoter::IS_AUTHENTICATED_ANONYMOUSLY, true];
101+
yield [AuthenticatedVoter::IS_ANONYMOUS, true];
102+
yield [AuthenticatedVoter::IS_AUTHENTICATED, true];
103+
yield [AuthenticatedVoter::IS_IMPERSONATOR, true];
104+
yield [AuthenticatedVoter::IS_REMEMBERED, true];
105+
yield [AuthenticatedVoter::PUBLIC_ACCESS, true];
106+
107+
yield ['', false];
108+
yield ['foo', false];
109+
}
110+
111+
public function testSupportsType()
112+
{
113+
$voter = new AuthenticatedVoter(new AuthenticationTrustResolver());
114+
115+
$this->assertTrue($voter->supportsType(get_debug_type('foo')));
116+
$this->assertTrue($voter->supportsType(get_debug_type(null)));
117+
$this->assertTrue($voter->supportsType(get_debug_type(new \StdClass())));
118+
}
119+
86120
protected function getToken($authenticated)
87121
{
88122
$user = new InMemoryUser('wouter', '', ['ROLE_USER']);

0 commit comments

Comments
 (0)