Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit be530b6

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
[Security] Add ability for voters to explain their vote
1 parent 6a6ebac commit be530b6
Copy full SHA for be530b6

37 files changed

+489
-189
lines changed

‎UPGRADE-7.3.md

Copy file name to clipboardExpand all lines: UPGRADE-7.3.md
+26-6Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,19 @@ Ldap
1616
Security
1717
--------
1818

19-
* Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`,
19+
* Deprecate `UserInterface::eraseCredentials()` and `TokenInterface::eraseCredentials()`;
2020
erase credentials e.g. using `__serialize()` instead
2121

22-
*Before*
22+
Before:
23+
2324
```php
2425
public function eraseCredentials(): void
2526
{
2627
}
2728
```
2829

29-
*After*
30+
After:
31+
3032
```php
3133
#[\Deprecated]
3234
public function eraseCredentials(): void
@@ -43,19 +45,36 @@ Security
4345
}
4446
```
4547

48+
* Add argument `$vote` to `VoterInterface::vote()` and to `Voter::voteOnAttribute()`;
49+
it should be used to report the reason of a vote. E.g:
50+
51+
```php
52+
protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool
53+
{
54+
$vote ??= new Vote();
55+
56+
$vote->reasons[] = 'A brief explanation of why access is granted or denied, as appropriate.';
57+
}
58+
```
59+
60+
* Add argument `$accessDecision` to `AccessDecisionManagerInterface::decide()` and `AuthorizationCheckerInterface::isGranted()`;
61+
it should be used to report the reason of a decision, including all the related votes.
62+
4663
Console
4764
-------
4865

49-
* Omitting parameter types in callables configured via `Command::setCode` method is deprecated
66+
* Omitting parameter types in callables configured via `Command::setCode()` method is deprecated
67+
68+
Before:
5069

51-
*Before*
5270
```php
5371
$command->setCode(function ($input, $output) {
5472
// ...
5573
});
5674
```
5775

58-
*After*
76+
After:
77+
5978
```php
6079
use Symfony\Component\Console\Input\InputInterface;
6180
use Symfony\Component\Console\Output\OutputInterface;
@@ -119,6 +138,7 @@ Validator
119138
}
120139
}
121140
```
141+
122142
* Deprecate passing an array of options to the constructors of the constraint classes, pass each option as a dedicated argument instead
123143

124144
Before:

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

Copy file name to clipboardExpand all lines: src/Symfony/Bridge/Twig/Extension/SecurityExtension.php
+9-4Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Controller/AbstractController.php
+33-5Lines 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/FrameworkBundle/Tests/Controller/AbstractControllerTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Tests/Controller/AbstractControllerTest.php
+26-2Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@
4040
use Symfony\Component\Routing\RouterInterface;
4141
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
4242
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
43+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
4344
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
45+
use Symfony\Component\Security\Core\Authorization\Voter\Vote;
46+
use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface;
4447
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
4548
use Symfony\Component\Security\Core\User\InMemoryUser;
4649
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
@@ -352,7 +355,19 @@ public function testIsGranted()
352355
public function testdenyAccessUnlessGranted()
353356
{
354357
$authorizationChecker = $this->createMock(AuthorizationCheckerInterface::class);
355-
$authorizationChecker->expects($this->once())->method('isGranted')->willReturn(false);
358+
$authorizationChecker
359+
->expects($this->once())
360+
->method('isGranted')
361+
->willReturnCallback(function ($attribute, $subject, ?AccessDecision $accessDecision = null) {
362+
if (class_exists(AccessDecision::class)) {
363+
$this->assertInstanceOf(AccessDecision::class, $accessDecision);
364+
$accessDecision->votes[] = $vote = new Vote();
365+
$vote->result = VoterInterface::ACCESS_DENIED;
366+
$vote->reasons[] = 'Why should I.';
367+
}
368+
369+
return false;
370+
});
356371

357372
$container = new Container();
358373
$container->set('security.authorization_checker', $authorizationChecker);
@@ -361,8 +376,17 @@ public function testdenyAccessUnlessGranted()
361376
$controller->setContainer($container);
362377

363378
$this->expectException(AccessDeniedException::class);
379+
$this->expectExceptionMessage('Access Denied.'.(class_exists(AccessDecision::class) ? ' Why should I.' : ''));
364380

365-
$controller->denyAccessUnlessGranted('foo');
381+
try {
382+
$controller->denyAccessUnlessGranted('foo');
383+
} catch (AccessDeniedException $e) {
384+
if (class_exists(AccessDecision::class)) {
385+
$this->assertFalse($e->getAccessDecision()->isGranted);
386+
}
387+
388+
throw $e;
389+
}
366390
}
367391

368392
/**

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php
+2Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/EventListener/VoteListener.php
+1-1Lines 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/config/security_debug.php

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/config/security_debug.php
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
->args([
2323
service('debug.security.access.decision_manager.inner'),
2424
])
25+
->tag('kernel.reset', ['method' => 'reset', 'on_invalid' => 'ignore'])
2526

2627
->set('debug.security.voter.vote_listener', VoteListener::class)
2728
->args([

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Resources/views/Collector/security.html.twig
+6-3Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Security.php
+5-4Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/SecurityBundle/Tests/DataCollector/SecurityDataCollectorTest.php
+10-9Lines 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
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.