Skip to content

Commit 7da1eef

Browse files
committed
feat(symfony): isGranted before provider
1 parent 07d0ef8 commit 7da1eef

File tree

7 files changed

+280
-26
lines changed

7 files changed

+280
-26
lines changed

src/Symfony/Bundle/Resources/config/state/security.php

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,39 @@
1313

1414
namespace Symfony\Component\DependencyInjection\Loader\Configurator;
1515

16+
use ApiPlatform\State\Provider\SecurityParameterProvider;
17+
use ApiPlatform\Symfony\Security\State\AccessCheckerProvider;
18+
1619
return function (ContainerConfigurator $container) {
1720
$services = $container->services();
1821

19-
$services->set('api_platform.state_provider.access_checker', 'ApiPlatform\Symfony\Security\State\AccessCheckerProvider')
22+
$services->set('api_platform.state_provider.access_checker', AccessCheckerProvider::class)
2023
->decorate('api_platform.state_provider.read', null, 0)
2124
->args([
2225
service('api_platform.state_provider.access_checker.inner'),
2326
service('api_platform.security.resource_access_checker'),
2427
]);
2528

26-
$services->set('api_platform.state_provider.access_checker.post_deserialize', 'ApiPlatform\Symfony\Security\State\AccessCheckerProvider')
29+
$services->set('api_platform.state_provider.access_checker.post_deserialize', AccessCheckerProvider::class)
2730
->decorate('api_platform.state_provider.deserialize', null, 0)
2831
->args([
2932
service('api_platform.state_provider.access_checker.post_deserialize.inner'),
3033
service('api_platform.security.resource_access_checker'),
3134
'post_denormalize',
3235
]);
3336

34-
$services->set('api_platform.state_provider.security_parameter', 'ApiPlatform\State\Provider\SecurityParameterProvider')
37+
$services->set('api_platform.state_provider.security_parameter', SecurityParameterProvider::class)
3538
->decorate('api_platform.state_provider.access_checker', null, 0)
3639
->args([
3740
service('api_platform.state_provider.security_parameter.inner'),
3841
service('api_platform.security.resource_access_checker'),
3942
]);
43+
44+
$services->set('api_platform.state_provider.access_checker.pre_read', AccessCheckerProvider::class)
45+
->decorate('api_platform.state_provider.read', null, 0)
46+
->args([
47+
service('api_platform.state_provider.access_checker.pre_read.inner'),
48+
service('api_platform.security.resource_access_checker'),
49+
'pre_read',
50+
]);
4051
};
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.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+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Symfony\Security;
15+
16+
interface ObjectVariableCheckerInterface
17+
{
18+
/**
19+
* @param string $expression a Expression Language string
20+
* @param array<string, mixed> $variables
21+
*/
22+
public function usesObjectVariable(string $expression, array $variables = []): bool;
23+
}

src/Symfony/Security/ResourceAccessChecker.php

Lines changed: 57 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
use ApiPlatform\Metadata\ResourceAccessCheckerInterface;
1717
use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
18+
use Symfony\Component\ExpressionLanguage\Node\NameNode;
19+
use Symfony\Component\ExpressionLanguage\Node\Node;
1820
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
1921
use Symfony\Component\Security\Core\Authentication\Token\NullToken;
2022
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
@@ -27,7 +29,7 @@
2729
*
2830
* @author Kévin Dunglas <dunglas@gmail.com>
2931
*/
30-
final class ResourceAccessChecker implements ResourceAccessCheckerInterface
32+
final class ResourceAccessChecker implements ResourceAccessCheckerInterface, ObjectVariableCheckerInterface
3133
{
3234
public function __construct(private readonly ?ExpressionLanguage $expressionLanguage = null, private readonly ?AuthenticationTrustResolverInterface $authenticationTrustResolver = null, private readonly ?RoleHierarchyInterface $roleHierarchy = null, private readonly ?TokenStorageInterface $tokenStorage = null, private readonly ?AuthorizationCheckerInterface $authorizationChecker = null)
3335
{
@@ -43,32 +45,32 @@ public function isGranted(string $resourceClass, string $expression, array $extr
4345
throw new \LogicException('The "symfony/expression-language" library must be installed to use the "security" attribute.');
4446
}
4547

46-
$variables = array_merge($extraVariables, [
47-
'trust_resolver' => $this->authenticationTrustResolver,
48-
'auth_checker' => $this->authorizationChecker, // needed for the is_granted expression function
49-
]);
50-
51-
if (null === $token = $this->tokenStorage->getToken()) {
52-
$token = new NullToken();
53-
}
54-
55-
$variables = array_merge($variables, $this->getVariables($token));
48+
return (bool) $this->expressionLanguage->evaluate($expression, $this->getVariables($extraVariables));
49+
}
5650

57-
return (bool) $this->expressionLanguage->evaluate($expression, $variables);
51+
public function usesObjectVariable(string $expression, array $variables = []): bool
52+
{
53+
return $this->hasObjectVariable($this->expressionLanguage->parse($expression, array_keys($this->getVariables($variables)))->getNodes()->toArray());
5854
}
5955

6056
/**
6157
* @copyright Fabien Potencier <fabien@symfony.com>
6258
*
6359
* @see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Core/Authorization/Voter/ExpressionVoter.php
6460
*/
65-
private function getVariables(TokenInterface $token): array
61+
private function getVariables(array $variables): array
6662
{
67-
return [
63+
if (null === $token = $this->tokenStorage->getToken()) {
64+
$token = new NullToken();
65+
}
66+
67+
return array_merge($variables, [
6868
'token' => $token,
6969
'user' => $token->getUser(),
7070
'roles' => $this->getEffectiveRoles($token),
71-
];
71+
'trust_resolver' => $this->authenticationTrustResolver,
72+
'auth_checker' => $this->authorizationChecker, // needed for the is_granted expression function
73+
]);
7274
}
7375

7476
/**
@@ -82,4 +84,44 @@ private function getEffectiveRoles(TokenInterface $token): array
8284

8385
return $this->roleHierarchy->getReachableRoleNames($token->getRoleNames());
8486
}
87+
88+
/**
89+
* Recursively checks if a variable named 'object' is present in the expression AST.
90+
*
91+
* @param Node|array<mixed>|null $nodeOrNodes the ExpressionLanguage Node instance or an array of nodes/values
92+
*/
93+
private function hasObjectVariable(Node|array|null $nodeOrNodes): bool
94+
{
95+
if ($nodeOrNodes instanceof NameNode) {
96+
if ('object' === $nodeOrNodes->attributes['name'] || 'previous_object' === $nodeOrNodes->attributes['name']) {
97+
return true;
98+
}
99+
100+
return false;
101+
}
102+
103+
if ($nodeOrNodes instanceof Node) {
104+
foreach ($nodeOrNodes->nodes as $childNode) {
105+
if ($this->hasObjectVariable($childNode)) {
106+
return true;
107+
}
108+
}
109+
110+
return false;
111+
}
112+
113+
if (\is_array($nodeOrNodes)) {
114+
foreach ($nodeOrNodes as $element) {
115+
if (\is_string($element)) {
116+
continue;
117+
}
118+
119+
if ($this->hasObjectVariable($element)) {
120+
return true;
121+
}
122+
}
123+
}
124+
125+
return false;
126+
}
85127
}

src/Symfony/Security/State/AccessCheckerProvider.php

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,15 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
5959
$message = $operation->getSecurityMessage();
6060
}
6161

62-
$body = $this->decorated->provide($operation, $uriVariables, $context);
63-
if (null === $isGranted) {
64-
return $body;
62+
if (
63+
null === $isGranted
64+
// On a GraphQl QueryCollection we want to perform security stage only on the top-level query
65+
|| ($operation instanceof QueryCollection && null !== ($context['source'] ?? null))
66+
) {
67+
return $this->decorated->provide($operation, $uriVariables, $context);
6568
}
6669

67-
// On a GraphQl QueryCollection we want to perform security stage only on the top-level query
68-
if ($operation instanceof QueryCollection && null !== ($context['source'] ?? null)) {
69-
return $body;
70-
}
70+
$body = 'pre_read' === $this->event ? null : $this->decorated->provide($operation, $uriVariables, $context);
7171

7272
if ($operation instanceof HttpOperation) {
7373
$request = $context['request'] ?? null;
@@ -84,10 +84,14 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
8484
];
8585
}
8686

87+
if ('pre_read' === $this->event && $this->resourceAccessChecker->usesObjectVariable($isGranted, $resourceAccessCheckerContext)) {
88+
return $this->decorated->provide($operation, $uriVariables, $context);
89+
}
90+
8791
if (!$this->resourceAccessChecker->isGranted($operation->getClass(), $isGranted, $resourceAccessCheckerContext)) {
8892
$operation instanceof GraphQlOperation ? throw new AccessDeniedHttpException($message ?? 'Access Denied.') : throw new AccessDeniedException($message ?? 'Access Denied.');
8993
}
9094

91-
return $body;
95+
return 'pre_read' === $this->event ? $this->decorated->provide($operation, $uriVariables, $context) : $body;
9296
}
9397
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.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+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Symfony\Security\State;
15+
16+
use ApiPlatform\Metadata\Operation;
17+
use ApiPlatform\State\ProviderInterface;
18+
use ApiPlatform\Symfony\Security\Exception\AccessDeniedException;
19+
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
20+
use Symfony\Component\Security\Http\Attribute\IsGranted;
21+
22+
/**
23+
* Denies access based on the ApiPlatform\Metadata\ResourceAccessCheckerInterface.
24+
* This implementation covers HTTP.
25+
*
26+
* @see ResourceAccessCheckerInterface
27+
*/
28+
final class IsGrantedAccessCheckerProvider implements ProviderInterface
29+
{
30+
public function __construct(private readonly ProviderInterface $decorated, private readonly ?AuthorizationCheckerInterface $authChecker = null)
31+
{
32+
}
33+
34+
public function provide(Operation $operation, array $uriVariables = [], array $context = []): object|array|null
35+
{
36+
if (!$this->authChecker) {
37+
return $this->decorated->provide($operation, $uriVariables, $context);
38+
}
39+
40+
$s = $operation->getSecurity();
41+
if (null === $s) {
42+
return $this->decorated->provide($operation, $uriVariables, $context);
43+
}
44+
45+
if (!\is_array($s)) {
46+
$s = [$s];
47+
}
48+
49+
foreach ($s as $isGranted) {
50+
if (!$isGranted instanceof IsGranted) {
51+
continue;
52+
}
53+
54+
if (!$this->authChecker->isGranted($isGranted->attribute, $isGranted->subject)) {
55+
$message = $isGranted->message ?: 'Access denied';
56+
throw new AccessDeniedException($message);
57+
}
58+
}
59+
60+
return $this->decorated->provide($operation, $uriVariables, $context);
61+
}
62+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.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+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource;
15+
16+
use ApiPlatform\Metadata\ApiResource;
17+
use ApiPlatform\Metadata\Get;
18+
use ApiPlatform\Metadata\Operation;
19+
20+
#[ApiResource(
21+
operations: [
22+
new Get(uriTemplate: 'is_granted_tests/{id}', security: 'is_granted("ROLE_ADMIN")', uriVariables: ['id'], provider: [self::class, 'provide']),
23+
new Get(uriTemplate: 'is_granted_test_call_provider/{id}', uriVariables: ['id'], security: 'is_granted("ROLE_ADMIN")', provider: [self::class, 'provideShouldNotBeCalled']),
24+
]
25+
)]
26+
class IsGrantedTestResource
27+
{
28+
private ?int $id = null;
29+
30+
public function getId(): ?int
31+
{
32+
return $this->id;
33+
}
34+
35+
public static function provide(Operation $operation, array $uriVariables = [], array $context = [])
36+
{
37+
return new self();
38+
}
39+
40+
public static function provideShouldNotBeCalled(Operation $operation, array $uriVariables = [], array $context = [])
41+
{
42+
throw new \RuntimeException('provider should not get called');
43+
}
44+
}

tests/Functional/IsGrantedTest.php

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the API Platform project.
5+
*
6+
* (c) Kévin Dunglas <dunglas@gmail.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+
declare(strict_types=1);
13+
14+
namespace ApiPlatform\Tests\Functional;
15+
16+
use ApiPlatform\Symfony\Bundle\Test\ApiTestCase;
17+
use ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\IsGrantedTestResource;
18+
use ApiPlatform\Tests\SetupClassResourcesTrait;
19+
use Symfony\Component\Security\Core\User\InMemoryUser;
20+
21+
final class IsGrantedTest extends ApiTestCase
22+
{
23+
use SetupClassResourcesTrait;
24+
25+
protected static ?bool $alwaysBootKernel = false;
26+
27+
/**
28+
* @return class-string[]
29+
*/
30+
public static function getResources(): array
31+
{
32+
return [IsGrantedTestResource::class];
33+
}
34+
35+
public function testGetIsGrantedAsAdmin(): void
36+
{
37+
$client = self::createClient();
38+
$client->loginUser(new InMemoryUser('admin', 'password', ['ROLE_ADMIN']));
39+
40+
$client->request('GET', '/is_granted_tests/1');
41+
$this->assertResponseIsSuccessful();
42+
}
43+
44+
public function testGetIsGrantedAsUser(): void
45+
{
46+
$client = self::createClient();
47+
$client->loginUser(new InMemoryUser('user', 'password', ['ROLE_USER']));
48+
49+
$client->request('GET', '/is_granted_tests/1');
50+
$this->assertResponseStatusCodeSame(403);
51+
}
52+
53+
public function testGetIsGrantedAsAnonymous(): void
54+
{
55+
$client = self::createClient();
56+
57+
$client->request('GET', '/is_granted_tests/1');
58+
$this->assertResponseStatusCodeSame(401);
59+
}
60+
61+
public function testGetIsGrantedShouldNotCallProvider(): void
62+
{
63+
$client = self::createClient();
64+
65+
$client->request('GET', '/is_granted_test_call_provider/1');
66+
$this->assertResponseStatusCodeSame(401);
67+
}
68+
}

0 commit comments

Comments
 (0)