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

[Security] Add ability for voters to explain their vote #59771

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions 32 UPGRADE-7.3.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@ Ldap
Security
--------

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

*Before*
Before:

```php
public function eraseCredentials(): void
{
}
```

*After*
After:

```php
#[\Deprecated]
public function eraseCredentials(): void
Expand All @@ -43,19 +45,36 @@ Security
}
```

* Add argument `$vote` to `VoterInterface::vote()` and to `Voter::voteOnAttribute()`;
it should be used to report the reason of a vote. E.g:

```php
protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool
{
$vote ??= new Vote();

$vote->reasons[] = 'A brief explanation of why access is granted or denied, as appropriate.';
}
```

* Add argument `$accessDecision` to `AccessDecisionManagerInterface::decide()` and `AuthorizationCheckerInterface::isGranted()`;
it should be used to report the reason of a decision, including all the related votes.

Console
-------

* Omitting parameter types in callables configured via `Command::setCode` method is deprecated
* Omitting parameter types in callables configured via `Command::setCode()` method is deprecated

Before:

*Before*
```php
$command->setCode(function ($input, $output) {
// ...
});
```

*After*
After:

```php
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
Expand Down Expand Up @@ -119,6 +138,7 @@ Validator
}
}
```

* Deprecate passing an array of options to the constructors of the constraint classes, pass each option as a dedicated argument instead

Before:
Expand Down
13 changes: 9 additions & 4 deletions 13 src/Symfony/Bridge/Twig/Extension/SecurityExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Bridge\Twig\Extension;

use Symfony\Component\Security\Acl\Voter\FieldVote;
use Symfony\Component\Security\Core\Authorization\AccessDecision;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
Expand All @@ -34,7 +35,7 @@ public function __construct(
) {
}

public function isGranted(mixed $role, mixed $object = null, ?string $field = null): bool
public function isGranted(mixed $role, mixed $object = null, ?string $field = null, ?AccessDecision $accessDecision = null): bool
{
if (null === $this->securityChecker) {
return false;
Expand All @@ -49,13 +50,13 @@ public function isGranted(mixed $role, mixed $object = null, ?string $field = nu
}

try {
return $this->securityChecker->isGranted($role, $object);
return $this->securityChecker->isGranted($role, $object, $accessDecision);
} catch (AuthenticationCredentialsNotFoundException) {
return false;
}
}

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

return $this->userSecurityChecker->isGrantedForUser($user, $attribute, $subject);
try {
return $this->userSecurityChecker->isGrantedForUser($user, $attribute, $subject, $accessDecision);
} catch (AuthenticationCredentialsNotFoundException) {
return false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added for parity with isGranted() above

}
}

public function getImpersonateExitUrl(?string $exitTo = null): string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -202,6 +203,21 @@ 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.
*/
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".');
}

$accessDecision = new AccessDecision();
$accessDecision->isGranted = $this->container->get('security.authorization_checker')->isGranted($attribute, $subject, $accessDecision);

return $accessDecision;
}

/**
* Throws an exception unless the attribute is granted against the current authentication token and optionally
* supplied subject.
Expand All @@ -210,12 +226,24 @@ 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)) {
$exception = $this->createAccessDeniedException($message);
$exception->setAttributes([$attribute]);
$exception->setSubject($subject);
if (class_exists(AccessDecision::class)) {
$accessDecision = $this->getAccessDecision($attribute, $subject);
$isGranted = $accessDecision->isGranted;
} else {
$accessDecision = null;
$isGranted = $this->isGranted($attribute, $subject);
}

if (!$isGranted) {
$e = $this->createAccessDeniedException(3 > \func_num_args() && $accessDecision ? $accessDecision->getMessage() : $message);
$e->setAttributes([$attribute]);
$e->setSubject($subject);

if ($accessDecision) {
$e->setAccessDecision($accessDecision);
}

throw $exception;
throw $e;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@
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\Authorization\Voter\VoterInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Symfony\Component\Security\Core\User\InMemoryUser;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
Expand Down Expand Up @@ -352,7 +355,19 @@ public function testIsGranted()
public function testdenyAccessUnlessGranted()
{
$authorizationChecker = $this->createMock(AuthorizationCheckerInterface::class);
$authorizationChecker->expects($this->once())->method('isGranted')->willReturn(false);
$authorizationChecker
->expects($this->once())
->method('isGranted')
->willReturnCallback(function ($attribute, $subject, ?AccessDecision $accessDecision = null) {
if (class_exists(AccessDecision::class)) {
$this->assertInstanceOf(AccessDecision::class, $accessDecision);
$accessDecision->votes[] = $vote = new Vote();
$vote->result = VoterInterface::ACCESS_DENIED;
$vote->reasons[] = 'Why should I.';
}

return false;
});

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

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

$controller->denyAccessUnlessGranted('foo');
try {
$controller->denyAccessUnlessGranted('foo');
} catch (AccessDeniedException $e) {
if (class_exists(AccessDecision::class)) {
$this->assertFalse($e->getAccessDecision()->isGranted);
}

throw $e;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,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) {
Expand All @@ -147,6 +148,7 @@ public function collect(Request $request, Response $response, ?\Throwable $excep
'class' => $classData,
'attributes' => $voterDetail['attributes'], // Only displayed for unanimous strategy
'vote' => $voterDetail['vote'],
'reasons' => $voterDetail['reasons'] ?? [],
];
}
unset($decisionLog[$key]['voterDetails']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(), $event->getReasons());
}

public static function getSubscribedEvents(): array
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
->args([
service('debug.security.access.decision_manager.inner'),
])
->tag('kernel.reset', ['method' => 'reset', 'on_invalid' => 'ignore'])

->set('debug.security.voter.vote_listener', VoteListener::class)
->args([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -571,14 +571,19 @@
{% endif %}
<td class="font-normal text-small">
{% if voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_GRANTED') %}
ACCESS GRANTED
GRANTED
{% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_ABSTAIN') %}
ACCESS ABSTAIN
ABSTAIN
{% elseif voter_detail['vote'] == constant('Symfony\\Component\\Security\\Core\\Authorization\\Voter\\VoterInterface::ACCESS_DENIED') %}
ACCESS DENIED
DENIED
{% else %}
unknown ({{ voter_detail['vote'] }})
{% endif %}
{% if voter_detail['reasons'] is not empty %}
{% for voter_reason in voter_detail['reasons'] %}
<br>{{ voter_reason }}
{% endfor %}
{% endif %}
</td>
</tr>
{% endfor %}
Expand Down
9 changes: 5 additions & 4 deletions 9 src/Symfony/Bundle/SecurityBundle/Security.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Authorization\AccessDecision;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Authorization\UserAuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\LogicException;
Expand Down Expand Up @@ -58,10 +59,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, ?AccessDecision $accessDecision = null): bool
{
return $this->container->get('security.authorization_checker')
->isGranted($attributes, $subject);
->isGranted($attributes, $subject, $accessDecision);
}

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

private function getAuthenticator(?string $authenticatorName, string $firewallName): AuthenticatorInterface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
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\VoterInterface;
use Symfony\Component\Security\Core\Role\RoleHierarchy;
use Symfony\Component\Security\Core\User\InMemoryUser;
Expand Down Expand Up @@ -271,8 +272,8 @@ public function dispatch(object $event, ?string $eventName = null): object
'object' => new \stdClass(),
'result' => 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' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []],
['class' => $voter2::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_ABSTAIN, 'reasons' => []],
],
]];

Expand Down Expand Up @@ -360,19 +361,19 @@ public function dispatch(object $event, ?string $eventName = null): object
'object' => new \stdClass(),
'result' => 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' => VoterInterface::ACCESS_DENIED, 'reasons' => []],
['class' => $voter1::class, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_DENIED, 'reasons' => []],
['class' => $voter2::class, 'attributes' => ['view'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []],
['class' => $voter2::class, 'attributes' => ['edit'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []],
],
],
[
'attributes' => ['update'],
'object' => new \stdClass(),
'result' => 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' => VoterInterface::ACCESS_GRANTED, 'reasons' => []],
['class' => $voter2::class, 'attributes' => ['update'], 'vote' => VoterInterface::ACCESS_GRANTED, 'reasons' => []],
],
],
];
Expand Down Expand Up @@ -461,7 +462,7 @@ private function getRoleHierarchy()

final class DummyVoter implements VoterInterface
{
public function vote(TokenInterface $token, mixed $subject, array $attributes): int
public function vote(TokenInterface $token, mixed $subject, array $attributes, ?Vote $vote = null): int
{
}
}
Loading
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.