Skip to content

Commit 19c5d06

Browse files
soyukasoyuka
committed
feat(symfony): isGranted before provider
1 parent 07d0ef8 commit 19c5d06

File tree

6 files changed

+219
-26
lines changed

6 files changed

+219
-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, 10)
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
};

‎src/Symfony/Security/ObjectVariableCheckerInterface.php‎

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: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use ApiPlatform\Metadata\ResourceAccessCheckerInterface;
2222
use ApiPlatform\State\ProviderInterface;
2323
use ApiPlatform\Symfony\Security\Exception\AccessDeniedException;
24+
use ApiPlatform\Symfony\Security\ObjectVariableCheckerInterface;
2425
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
2526

2627
/**
@@ -59,15 +60,15 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
5960
$message = $operation->getSecurityMessage();
6061
}
6162

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

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-
}
71+
$body = 'pre_read' === $this->event ? null : $this->decorated->provide($operation, $uriVariables, $context);
7172

7273
if ($operation instanceof HttpOperation) {
7374
$request = $context['request'] ?? null;
@@ -84,10 +85,14 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
8485
];
8586
}
8687

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

91-
return $body;
96+
return 'pre_read' === $this->event ? $this->decorated->provide($operation, $uriVariables, $context) : $body;
9297
}
9398
}

‎tests/Fixtures/TestBundle/ApiResource/IsGrantedTestResource.php‎

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)