Skip to content

Commit 5d6d6ee

Browse files
nicolas-grekasnicolas-grekas
committed
[Security] Add ability for voters to explain their vote
1 parent 649e01c commit 5d6d6ee

File tree

5 files changed

+59
-52
lines changed

5 files changed

+59
-52
lines changed

‎EventListener/IsGrantedAttributeListener.php

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
1919
use Symfony\Component\HttpKernel\Exception\HttpException;
2020
use Symfony\Component\HttpKernel\KernelEvents;
21+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
2122
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
2223
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
2324
use Symfony\Component\Security\Core\Exception\RuntimeException;
@@ -58,19 +59,21 @@ public function onKernelControllerArguments(ControllerArgumentsEvent $event): vo
5859
$subject = $this->getIsGrantedSubject($subjectRef, $request, $arguments);
5960
}
6061
}
62+
$accessDecision = new AccessDecision();
6163

62-
if (!$this->authChecker->isGranted($attribute->attribute, $subject)) {
63-
$message = $attribute->message ?: \sprintf('Access Denied by #[IsGranted(%s)] on controller', $this->getIsGrantedString($attribute));
64+
if (!$accessDecision->isGranted = $this->authChecker->isGranted($attribute->attribute, $subject, $accessDecision)) {
65+
$message = $attribute->message ?: $accessDecision->getMessage();
6466

6567
if ($statusCode = $attribute->statusCode) {
6668
throw new HttpException($statusCode, $message, code: $attribute->exceptionCode ?? 0);
6769
}
6870

69-
$accessDeniedException = new AccessDeniedException($message, code: $attribute->exceptionCode ?? 403);
70-
$accessDeniedException->setAttributes($attribute->attribute);
71-
$accessDeniedException->setSubject($subject);
71+
$e = new AccessDeniedException($message, code: $attribute->exceptionCode ?? 403);
72+
$e->setAttributes($attribute->attribute);
73+
$e->setSubject($subject);
74+
$e->setAccessDecision($accessDecision);
7275

73-
throw $accessDeniedException;
76+
throw $e;
7477
}
7578
}
7679
}
@@ -97,23 +100,4 @@ private function getIsGrantedSubject(string|Expression $subjectRef, Request $req
97100

98101
return $arguments[$subjectRef];
99102
}
100-
101-
private function getIsGrantedString(IsGranted $isGranted): string
102-
{
103-
$processValue = fn ($value) => \sprintf($value instanceof Expression ? 'new Expression("%s")' : '"%s"', $value);
104-
105-
$argsString = $processValue($isGranted->attribute);
106-
107-
if (null !== $subject = $isGranted->subject) {
108-
$subject = !\is_array($subject) ? $processValue($subject) : array_map(function ($key, $value) use ($processValue) {
109-
$value = $processValue($value);
110-
111-
return \is_string($key) ? \sprintf('"%s" => %s', $key, $value) : $value;
112-
}, array_keys($subject), $subject);
113-
114-
$argsString .= ', '.(!\is_array($subject) ? $subject : '['.implode(', ', $subject).']');
115-
}
116-
117-
return $argsString;
118-
}
119103
}

‎Firewall/AccessListener.php

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Symfony\Component\HttpKernel\Event\RequestEvent;
1616
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
1717
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
18+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
1819
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
1920
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
2021
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
@@ -72,19 +73,16 @@ public function authenticate(RequestEvent $event): void
7273
}
7374

7475
$token = $this->tokenStorage->getToken() ?? new NullToken();
76+
$accessDecision = new AccessDecision();
7577

76-
if (!$this->accessDecisionManager->decide($token, $attributes, $request, true)) {
77-
throw $this->createAccessDeniedException($request, $attributes);
78-
}
79-
}
78+
if (!$accessDecision->isGranted = $this->accessDecisionManager->decide($token, $attributes, $request, $accessDecision, true)) {
79+
$e = new AccessDeniedException($accessDecision->getMessage());
80+
$e->setAttributes($attributes);
81+
$e->setSubject($request);
82+
$e->setAccessDecision($accessDecision);
8083

81-
private function createAccessDeniedException(Request $request, array $attributes): AccessDeniedException
82-
{
83-
$exception = new AccessDeniedException();
84-
$exception->setAttributes($attributes);
85-
$exception->setSubject($request);
86-
87-
return $exception;
84+
throw $e;
85+
}
8886
}
8987

9088
public static function getPriority(): int

‎Firewall/SwitchUserListener.php

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
2020
use Symfony\Component\Security\Core\Authentication\Token\SwitchUserToken;
2121
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
22+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
2223
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
2324
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
2425
use Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException;
@@ -153,12 +154,14 @@ private function attemptSwitchUser(Request $request, string $username): ?TokenIn
153154

154155
throw $e;
155156
}
157+
$accessDecision = new AccessDecision();
156158

157-
if (false === $this->accessDecisionManager->decide($token, [$this->role], $user)) {
158-
$exception = new AccessDeniedException();
159-
$exception->setAttributes($this->role);
159+
if (!$accessDecision->isGranted = $this->accessDecisionManager->decide($token, [$this->role], $user, $accessDecision)) {
160+
$e = new AccessDeniedException($accessDecision->getMessage());
161+
$e->setAttributes($this->role);
162+
$e->setAccessDecision($accessDecision);
160163

161-
throw $exception;
164+
throw $e;
162165
}
163166

164167
$this->logger?->info('Attempting to switch to user.', ['username' => $username]);

‎Tests/EventListener/IsGrantedAttributeListenerTest.php

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,20 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\ExpressionLanguage\Expression;
16-
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
1716
use Symfony\Component\HttpFoundation\Request;
1817
use Symfony\Component\HttpKernel\Event\ControllerArgumentsEvent;
1918
use Symfony\Component\HttpKernel\Exception\HttpException;
2019
use Symfony\Component\HttpKernel\HttpKernelInterface;
20+
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
21+
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
22+
use Symfony\Component\Security\Core\Authorization\AccessDecisionManager;
23+
use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
2124
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
25+
use Symfony\Component\Security\Core\Authorization\ExpressionLanguage;
26+
use Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter;
27+
use Symfony\Component\Security\Core\Authorization\Voter\RoleVoter;
28+
use Symfony\Component\Security\Core\Authorization\Voter\Vote;
29+
use Symfony\Component\Security\Core\Authorization\Voter\Voter;
2230
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
2331
use Symfony\Component\Security\Http\EventListener\IsGrantedAttributeListener;
2432
use Symfony\Component\Security\Http\Tests\Fixtures\IsGrantedAttributeController;
@@ -213,10 +221,23 @@ public function testExceptionWhenMissingSubjectAttribute()
213221
*/
214222
public function testAccessDeniedMessages(string|Expression $attribute, string|array|null $subject, string $method, int $numOfArguments, string $expectedMessage)
215223
{
216-
$authChecker = $this->createMock(AuthorizationCheckerInterface::class);
217-
$authChecker->expects($this->any())
218-
->method('isGranted')
219-
->willReturn(false);
224+
$authChecker = new AuthorizationChecker(new TokenStorage(), new AccessDecisionManager((function () use (&$authChecker) {
225+
yield new ExpressionVoter(new ExpressionLanguage(), null, $authChecker);
226+
yield new RoleVoter();
227+
yield new class() extends Voter {
228+
protected function supports(string $attribute, mixed $subject): bool
229+
{
230+
return 'POST_VIEW' === $attribute;
231+
}
232+
233+
protected function voteOnAttribute(string $attribute, mixed $subject, TokenInterface $token, ?Vote $vote = null): bool
234+
{
235+
$vote->reasons[] = 'Because I can 😈.';
236+
237+
return false;
238+
}
239+
};
240+
})()));
220241

221242
$expressionLanguage = $this->createMock(ExpressionLanguage::class);
222243
$expressionLanguage->expects($this->any())
@@ -252,12 +273,12 @@ public function testAccessDeniedMessages(string|Expression $attribute, string|ar
252273

253274
public static function getAccessDeniedMessageTests()
254275
{
255-
yield ['ROLE_ADMIN', null, 'admin', 0, 'Access Denied by #[IsGranted("ROLE_ADMIN")] on controller'];
256-
yield ['ROLE_ADMIN', 'bar', 'withSubject', 2, 'Access Denied by #[IsGranted("ROLE_ADMIN", "arg2Name")] on controller'];
257-
yield ['ROLE_ADMIN', ['arg1Name' => 'bar', 'arg2Name' => 'bar'], 'withSubjectArray', 2, 'Access Denied by #[IsGranted("ROLE_ADMIN", ["arg1Name", "arg2Name"])] on controller'];
258-
yield [new Expression('"ROLE_ADMIN" in role_names or is_granted("POST_VIEW", subject)'), 'bar', 'withExpressionInAttribute', 1, 'Access Denied by #[IsGranted(new Expression(""ROLE_ADMIN" in role_names or is_granted("POST_VIEW", subject)"), "post")] on controller'];
259-
yield [new Expression('user === subject'), 'bar', 'withExpressionInSubject', 1, 'Access Denied by #[IsGranted(new Expression("user === subject"), new Expression("args["post"].getAuthor()"))] on controller'];
260-
yield [new Expression('user === subject["author"]'), ['author' => 'bar', 'alias' => 'bar'], 'withNestedExpressionInSubject', 2, 'Access Denied by #[IsGranted(new Expression("user === subject["author"]"), ["author" => new Expression("args["post"].getAuthor()"), "alias" => "arg2Name"])] on controller'];
276+
yield ['ROLE_ADMIN', null, 'admin', 0, 'Access Denied. The user doesn\'t have ROLE_ADMIN.'];
277+
yield ['ROLE_ADMIN', 'bar', 'withSubject', 2, 'Access Denied. The user doesn\'t have ROLE_ADMIN.'];
278+
yield ['ROLE_ADMIN', ['arg1Name' => 'bar', 'arg2Name' => 'bar'], 'withSubjectArray', 2, 'Access Denied. The user doesn\'t have ROLE_ADMIN.'];
279+
yield [new Expression('"ROLE_ADMIN" in role_names or is_granted("POST_VIEW", subject)'), 'bar', 'withExpressionInAttribute', 1, 'Access Denied. Because I can 😈. Expression ("ROLE_ADMIN" in role_names or is_granted("POST_VIEW", subject)) is false.'];
280+
yield [new Expression('user === subject'), 'bar', 'withExpressionInSubject', 1, 'Access Denied. Expression (user === subject) is false.'];
281+
yield [new Expression('user === subject["author"]'), ['author' => 'bar', 'alias' => 'bar'], 'withNestedExpressionInSubject', 2, 'Access Denied. Expression (user === subject["author"]) is false.'];
261282
}
262283

263284
public function testNotFoundHttpException()

‎Tests/Firewall/AccessListenerTest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorage;
2121
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
2222
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
23+
use Symfony\Component\Security\Core\Authorization\AccessDecision;
2324
use Symfony\Component\Security\Core\Authorization\AccessDecisionManagerInterface;
2425
use Symfony\Component\Security\Core\Authorization\Voter\AuthenticatedVoter;
2526
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
@@ -235,7 +236,7 @@ public function testHandleMWithultipleAttributesShouldBeHandledAsAnd()
235236
$accessDecisionManager
236237
->expects($this->once())
237238
->method('decide')
238-
->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), true)
239+
->with($this->equalTo($authenticatedToken), $this->equalTo(['foo' => 'bar', 'bar' => 'baz']), $this->equalTo($request), new AccessDecision(), true)
239240
->willReturn(true)
240241
;
241242

0 commit comments

Comments
 (0)