Skip to content

Commit 20e7b35

Browse files
nicolas-grekasnicolas-grekas
committed
[Security] Add ability for voters to explain their vote
1 parent 6a6ebac commit 20e7b35

34 files changed

+401
-163
lines changed

‎src/Symfony/Bridge/Twig/Extension/SecurityExtension.php‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Bridge\Twig\Extension;
1313

1414
use Symfony\Component\Security\Acl\Voter\FieldVote;
15+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
1516
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
1617
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
1718
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
@@ -34,7 +35,7 @@ public function __construct(
3435
) {
3536
}
3637

37-
public function isGranted(mixed $role, mixed $object = null, ?string $field = null): bool
38+
public function isGranted(mixed $role, mixed $object = null, ?string $field = null, ?AccessDecision $accessDecision = null): bool
3839
{
3940
if (null === $this->securityChecker) {
4041
return false;
@@ -49,13 +50,13 @@ public function isGranted(mixed $role, mixed $object = null, ?string $field = nu
4950
}
5051

5152
try {
52-
return $this->securityChecker->isGranted($role, $object);
53+
return $this->securityChecker->isGranted($role, $object, $accessDecision);
5354
} catch (AuthenticationCredentialsNotFoundException) {
5455
return false;
5556
}
5657
}
5758

58-
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null): bool
59+
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?string $field = null, ?AccessDecision $accessDecision = null): bool
5960
{
6061
if (!$this->userSecurityChecker) {
6162
throw new \LogicException(\sprintf('An instance of "%s" must be provided to use "%s()".', UserAuthorizationCheckerInterface::class, __METHOD__));
@@ -69,7 +70,11 @@ public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $s
6970
$subject = new FieldVote($subject, $field);
7071
}
7172

72-
return $this->userSecurityChecker->isGrantedForUser($user, $attribute, $subject);
73+
try {
74+
return $this->userSecurityChecker->isGrantedForUser($user, $attribute, $subject, $accessDecision);
75+
} catch (AuthenticationCredentialsNotFoundException) {
76+
return false;
77+
}
7378
}
7479

7580
public function getImpersonateExitUrl(?string $exitTo = null): string

‎src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
3636
use Symfony\Component\Routing\RouterInterface;
3737
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
38+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
3839
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
3940
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
4041
use Symfony\Component\Security\Core\User\UserInterface;
@@ -202,6 +203,21 @@ protected function isGranted(mixed $attribute, mixed $subject = null): bool
202203
return $this->container->get('security.authorization_checker')->isGranted($attribute, $subject);
203204
}
204205

206+
/**
207+
* Checks if the attribute is granted against the current authentication token and optionally supplied subject.
208+
*/
209+
protected function getAccessDecision(mixed $attribute, mixed $subject = null): AccessDecision
210+
{
211+
if (!$this->container->has('security.authorization_checker')) {
212+
throw new \LogicException('The SecurityBundle is not registered in your application. Try running "composer require symfony/security-bundle".');
213+
}
214+
215+
$accessDecision = new AccessDecision();
216+
$accessDecision->isGranted = $this->container->get('security.authorization_checker')->isGranted($attribute, $subject, $accessDecision);
217+
218+
return $accessDecision;
219+
}
220+
205221
/**
206222
* Throws an exception unless the attribute is granted against the current authentication token and optionally
207223
* supplied subject.
@@ -210,12 +226,24 @@ protected function isGranted(mixed $attribute, mixed $subject = null): bool
210226
*/
211227
protected function denyAccessUnlessGranted(mixed $attribute, mixed $subject = null, string $message = 'Access Denied.'): void
212228
{
213-
if (!$this->isGranted($attribute, $subject)) {
214-
$exception = $this->createAccessDeniedException($message);
215-
$exception->setAttributes([$attribute]);
216-
$exception->setSubject($subject);
229+
if (class_exists(AccessDecision::class)) {
230+
$accessDecision = $this->getAccessDecision($attribute, $subject);
231+
$isGranted = $accessDecision->isGranted;
232+
} else {
233+
$accessDecision = null;
234+
$isGranted = $this->isGranted($attribute, $subject);
235+
}
236+
237+
if (!$isGranted) {
238+
$e = $this->createAccessDeniedException(3 > \func_num_args() && $accessDecision ? $accessDecision->getMessage() : $message);
239+
$e->setAttributes([$attribute]);
240+
$e->setSubject($subject);
241+
242+
if ($accessDecision) {
243+
$e->setAccessDecision($accessDecision);
244+
}
217245

218-
throw $exception;
246+
throw $e;
219247
}
220248
}
221249

‎src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep
138138

139139
// collect voter details
140140
$decisionLog = $this->accessDecisionManager->getDecisionLog();
141+
141142
foreach ($decisionLog as $key => $log) {
142143
$decisionLog[$key]['voter_details'] = [];
143144
foreach ($log['voterDetails'] as $voterDetail) {
@@ -147,6 +148,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep
147148
'class' => $classData,
148149
'attributes' => $voterDetail['attributes'], // Only displayed for unanimous strategy
149150
'vote' => $voterDetail['vote'],
151+
'reasons' => $voterDetail['reasons'] ?? [],
150152
];
151153
}
152154
unset($decisionLog[$key]['voterDetails']);

‎src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public function __construct(
3131

3232
public function onVoterVote(VoteEvent $event): void
3333
{
34-
$this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $event->getVote());
34+
$this->traceableAccessDecisionManager->addVoterVote($event->getVoter(), $event->getAttributes(), $event->getVote(), $event->getReasons());
3535
}
3636

3737
public static function getSubscribedEvents(): array

‎src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,14 +571,17 @@
571571
{% endif %}
572572
<td class="font-normal text-small">
573573
{% if voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_GRANTED') %}
574-
ACCESS GRANTED
574+
GRANTED
575575
{% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_ABSTAIN') %}
576-
ACCESS ABSTAIN
576+
ABSTAIN
577577
{% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %}
578-
ACCESS DENIED
578+
DENIED
579579
{% else %}
580580
unknown ({{ voter_detail['vote'] }})
581581
{% endif %}
582+
{% if voter_detail['reasons'] is not empty %}
583+
<br>{{ voter_detail['reasons'] | join('<br>') }}
584+
{% endif %}
582585
</td>
583586
</tr>
584587
{% endfor %}

‎src/Symfony/Bundle/SecurityBundle/Security.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Symfony\Component\HttpFoundation\Response;
1818
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
1919
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
20+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
2021
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
2122
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
2223
use Symfony\Component\Security\Core\Exception\LogicException;
@@ -58,10 +59,10 @@ public function getUser(): ?UserInterface
5859
/**
5960
* Checks if the attributes are granted against the current authentication token and optionally supplied subject.
6061
*/
61-
public function isGranted(mixed $attributes, mixed $subject = null): bool
62+
public function isGranted(mixed $attributes, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
6263
{
6364
return $this->container->get('security.authorization_checker')
64-
->isGranted($attributes, $subject);
65+
->isGranted($attributes, $subject, $accessDecision);
6566
}
6667

6768
public function getToken(): ?TokenInterface
@@ -154,10 +155,10 @@ public function logout(bool $validateCsrfToken = true): ?Response
154155
*
155156
* This should be used over isGranted() when checking permissions against a user that is not currently logged in or while in a CLI context.
156157
*/
157-
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null): bool
158+
public function isGrantedForUser(UserInterface $user, mixed $attribute, mixed $subject = null, ?AccessDecision $accessDecision = null): bool
158159
{
159160
return $this->container->get('security.user_authorization_checker')
160-
->isGrantedForUser($user, $attribute, $subject);
161+
->isGrantedForUser($user, $attribute, $subject, $accessDecision);
161162
}
162163

163164
private function getAuthenticator(?string $authenticatorName, string $firewallName): AuthenticatorInterface

‎src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
2929
use Symfony\Component\Security\Core\Authorization\TraceableAccessDecisionManager;
3030
use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter;
31+
use Symfony\Component\Security\Core\Authorization\Voter\Vote;
3132
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
3233
use Symfony\Component\Security\Core\Role\RoleHierarchy;
3334
use Symfony\Component\Security\Core\User\InMemoryUser;
@@ -271,8 +272,8 @@ public function dispatch(object $event, ?string $eventName = null): object
271272
'object' => new \stdClass(),
272273
'result' => true,
273274
'voter_details' => [
274-
['class' => $voter1::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN],
275-
['class' => $voter2::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN],
275+
['class' => $voter1::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []],
276+
['class' => $voter2::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []],
276277
],
277278
]];
278279

@@ -360,19 +361,19 @@ public function dispatch(object $event, ?string $eventName = null): object
360361
'object' => new \stdClass(),
361362
'result' => false,
362363
'voter_details' => [
363-
['class' => $voter1::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_DENIED],
364-
['class' => $voter1::class, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_DENIED],
365-
['class' => $voter2::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_GRANTED],
366-
['class' => $voter2::class, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_GRANTED],
364+
['class' => $voter1::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_DENIED, 'reasons' => []],
365+
['class' => $voter1::class, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_DENIED, 'reasons' => []],
366+
['class' => $voter2::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []],
367+
['class' => $voter2::class, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []],
367368
],
368369
],
369370
[
370371
'attributes' => ['update'],
371372
'object' => new \stdClass(),
372373
'result' => true,
373374
'voter_details' => [
374-
['class' => $voter1::class, 'attributes' => ['update'], 'vote' => VoterInterface::ACCESS_GRANTED],
375-
['class' => $voter2::class, 'attributes' => ['update'], 'vote' => VoterInterface::ACCESS_GRANTED],
375+
['class' => $voter1::class, 'attributes' => ['update'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []],
376+
['class' => $voter2::class, 'attributes' => ['update'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []],
376377
],
377378
],
378379
];
@@ -461,7 +462,7 @@ private function getRoleHierarchy()
461462

462463
final class DummyVoter implements VoterInterface
463464
{
464-
public function vote(TokenInterface $token, mixed $subject, array $attributes): int
465+
public function vote(TokenInterface $token, mixed $subject, array $attributes, ?Vote $vote = null): int
465466
{
466467
}
467468
}

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

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
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;
13+
14+
use Symfony\Component\Security\Core\Authorization\Voter\Vote;
15+
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
16+
17+
/**
18+
* Contains the access verdict and all the related votes.
19+
*
20+
* @author Dany Maillard <danymaillard93b@gmail.com>
21+
* @author Roman JOLY <eltharin18@outlook.fr>
22+
* @author Nicolas Grekas <p@tchwork.com>
23+
*/
24+
class AccessDecision
25+
{
26+
/**
27+
* @var class-string<AccessDecisionStrategyInterface>|string|null
28+
*/
29+
public ?string $strategy = null;
30+
31+
public bool $isGranted;
32+
33+
/**
34+
* @var Vote[]
35+
*/
36+
public $votes = [];
37+
38+
public function getMessage(): string
39+
{
40+
$message = $this->isGranted ? 'Access Granted.' : 'Access Denied.';
41+
$access = $this->isGranted ? VoterInterface::ACCESS_GRANTED : VoterInterface::ACCESS_DENIED;
42+
43+
if ($this->votes) {
44+
foreach ($this->votes as $vote) {
45+
if ($vote->result !== $access) {
46+
continue;
47+
}
48+
foreach ($vote->reasons as $reason) {
49+
$message .= ' '.$reason;
50+
}
51+
}
52+
}
53+
54+
return $message;
55+
}
56+
}

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
use Symfony\Component\Security\Core\Authorization\Strategy\AccessDecisionStrategyInterface;
1616
use Symfony\Component\Security\Core\Authorization\Strategy\AffirmativeStrategy;
1717
use Symfony\Component\Security\Core\Authorization\Voter\CacheableVoterInterface;
18+
use Symfony\Component\Security\Core\Authorization\Voter\TraceableVoter;
19+
use Symfony\Component\Security\Core\Authorization\Voter\Vote;
1820
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
1921
use Symfony\Component\Security\Core\Exception\InvalidArgumentException;
2022

@@ -49,35 +51,49 @@ public function __construct(
4951
/**
5052
* @param bool $allowMultipleAttributes Whether to allow passing multiple values to the $attributes array
5153
*/
52-
public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool $allowMultipleAttributes = false): bool
54+
public function decide(TokenInterface $token, array $attributes, mixed $object = null, bool|AccessDecision|null $accessDecision = null, bool $allowMultipleAttributes = false): bool
5355
{
56+
if (\is_bool($accessDecision ??= new AccessDecision())) {
57+
$allowMultipleAttributes = $accessDecision;
58+
$accessDecision = new AccessDecision();
59+
}
60+
5461
// Special case for AccessListener, do not remove the right side of the condition before 6.0
5562
if (\count($attributes) > 1 && !$allowMultipleAttributes) {
5663
throw new InvalidArgumentException(\sprintf('Passing more than one Security attribute to "%s()" is not supported.', __METHOD__));
5764
}
5865

59-
return $this->strategy->decide(
60-
$this->collectResults($token, $attributes, $object)
66+
$accessDecision->strategy = $this->strategy instanceof \Stringable ? $this->strategy : get_debug_type($this->strategy);
67+
68+
return $accessDecision->isGranted = $this->strategy->decide(
69+
$this->collectResults($token, $attributes, $object, $accessDecision)
6170
);
6271
}
6372

6473
/**
65-
* @return \Traversable<int, int>
74+
* @return \Traversable<int, VoterInterface::ACCESS_*>
6675
*/
67-
private function collectResults(TokenInterface $token, array $attributes, mixed $object): \Traversable
76+
private function collectResults(TokenInterface $token, array $attributes, mixed $object, AccessDecision $accessDecision): \Traversable
6877
{
6978
foreach ($this->getVoters($attributes, $object) as $voter) {
70-
$result = $voter->vote($token, $object, $attributes);
79+
$vote = new Vote();
80+
$result = $voter->vote($token, $object, $attributes, $vote);
81+
7182
if (!\is_int($result) || !(self::VALID_VOTES[$result] ?? false)) {
7283
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)));
7384
}
7485

86+
$voter = $voter instanceof TraceableVoter ? $voter->getDecoratedVoter() : $voter;
87+
$vote->voter = $voter instanceof \Stringable ? $voter : get_debug_type($voter);
88+
$vote->result = $result;
89+
$accessDecision->votes[] = $vote;
90+
7591
yield $result;
7692
}
7793
}
7894

7995
/**
80-
* @return iterable<mixed, VoterInterface>
96+
* @return iterable<int, VoterInterface>
8197
*/
8298
private function getVoters(array $attributes, $object = null): iterable
8399
{

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,9 @@ interface AccessDecisionManagerInterface
2323
/**
2424
* Decides whether the access is possible or not.
2525
*
26-
* @param array $attributes An array of attributes associated with the method being invoked
27-
* @param mixed $object The object to secure
26+
* @param array $attributes An array of attributes associated with the method being invoked
27+
* @param mixed $object The object to secure
28+
* @param AccessDecision|null $accessDecision Should be used to explain the decision
2829
*/
29-
public function decide(TokenInterface $token, array $attributes, mixed $object = null): bool;
30+
public function decide(TokenInterface $token, array $attributes, mixed $object = null/* , ?AccessDecision $accessDecision = null */): bool;
3031
}

0 commit comments

Comments
 (0)