Skip to content

Commit ec43f9e

Browse files
vincentchalamonvincentchalamon
committed
Fix #1646 
1 parent f1802ea commit ec43f9e

File tree

18 files changed

+496
-65
lines changed

18 files changed

+496
-65
lines changed

‎features/authorization/legacy_deny.feature

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
Feature: Authorization checking
2+
In order to use the API
3+
As a client software user
4+
I need to be authorized to access a given resource using legacy access_control attribute.
5+
6+
@createSchema
7+
Scenario: An anonymous user retrieves a secured resource
8+
When I add "Accept" header equal to "application/ld+json"
9+
And I send a "GET" request to "/legacy_secured_dummies"
10+
Then the response status code should be 401
11+
12+
Scenario: An authenticated user retrieve a secured resource
13+
When I add "Accept" header equal to "application/ld+json"
14+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
15+
And I send a "GET" request to "/legacy_secured_dummies"
16+
Then the response status code should be 200
17+
And the response should be in JSON
18+
19+
Scenario: A standard user cannot create a secured resource
20+
When I add "Accept" header equal to "application/ld+json"
21+
And I add "Content-Type" header equal to "application/ld+json"
22+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
23+
And I send a "POST" request to "/legacy_secured_dummies" with body:
24+
"""
25+
{
26+
"title": "Title",
27+
"description": "Description",
28+
"owner": "foo"
29+
}
30+
"""
31+
Then the response status code should be 403
32+
33+
Scenario: An admin can create a secured resource
34+
When I add "Accept" header equal to "application/ld+json"
35+
And I add "Content-Type" header equal to "application/ld+json"
36+
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
37+
And I send a "POST" request to "/legacy_secured_dummies" with body:
38+
"""
39+
{
40+
"title": "Title",
41+
"description": "Description",
42+
"owner": "someone"
43+
}
44+
"""
45+
Then the response status code should be 201
46+
47+
Scenario: An admin can create another secured resource
48+
When I add "Accept" header equal to "application/ld+json"
49+
And I add "Content-Type" header equal to "application/ld+json"
50+
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
51+
And I send a "POST" request to "/legacy_secured_dummies" with body:
52+
"""
53+
{
54+
"title": "Special Title",
55+
"description": "Description",
56+
"owner": "dunglas"
57+
}
58+
"""
59+
Then the response status code should be 201
60+
61+
Scenario: A user cannot retrieve an item they doesn't own
62+
When I add "Accept" header equal to "application/ld+json"
63+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
64+
And I send a "GET" request to "/legacy_secured_dummies/1"
65+
Then the response status code should be 403
66+
And the response should be in JSON
67+
68+
Scenario: A user can retrieve an item they owns
69+
When I add "Accept" header equal to "application/ld+json"
70+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
71+
And I send a "GET" request to "/legacy_secured_dummies/2"
72+
Then the response status code should be 200
73+
74+
Scenario: A user can't assign to themself an item they doesn't own
75+
When I add "Accept" header equal to "application/ld+json"
76+
And I add "Content-Type" header equal to "application/ld+json"
77+
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
78+
And I send a "PUT" request to "/legacy_secured_dummies/2" with body:
79+
"""
80+
{
81+
"owner": "kitten"
82+
}
83+
"""
84+
Then the response status code should be 403
85+
86+
Scenario: A user can update an item they owns and transfer it
87+
When I add "Accept" header equal to "application/ld+json"
88+
And I add "Content-Type" header equal to "application/ld+json"
89+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
90+
And I send a "PUT" request to "/legacy_secured_dummies/2" with body:
91+
"""
92+
{
93+
"owner": "vincent"
94+
}
95+
"""
96+
Then the response status code should be 200

‎src/Annotation/ApiResource.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
* @Attributes(
2626
* @Attribute("accessControl", type="string"),
2727
* @Attribute("accessControlMessage", type="string"),
28+
* @Attribute("security", type="string"),
29+
* @Attribute("securityMessage", type="string"),
30+
* @Attribute("securityPostDenormalize", type="string"),
31+
* @Attribute("securityPostDenormalizeMessage", type="string"),
2832
* @Attribute("attributes", type="array"),
2933
* @Attribute("cacheHeaders", type="array"),
3034
* @Attribute("collectionOperations", type="array"),
@@ -116,6 +120,34 @@ final class ApiResource
116120
*/
117121
private $accessControlMessage;
118122

123+
/**
124+
* @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
125+
*
126+
* @var string
127+
*/
128+
private $security;
129+
130+
/**
131+
* @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
132+
*
133+
* @var string
134+
*/
135+
private $securityMessage;
136+
137+
/**
138+
* @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
139+
*
140+
* @var string
141+
*/
142+
private $securityPostDenormalize;
143+
144+
/**
145+
* @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
146+
*
147+
* @var string
148+
*/
149+
private $securityPostDenormalizeMessage;
150+
119151
/**
120152
* @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
121153
*

‎src/Bridge/Symfony/Bundle/Resources/config/security.xml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818
<argument type="service" id="api_platform.metadata.resource.metadata_factory" />
1919
<argument type="service" id="api_platform.security.resource_access_checker" />
2020

21-
<!-- This listener must be executed only when the current object is available -->
22-
<tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="1" />
21+
<!-- This method must be executed only when the current object is available, before deserialization -->
22+
<tag name="kernel.event_listener" event="kernel.request" method="onSecurity" priority="3" />
23+
<!-- This method must be executed only when the current object is available, after deserialization -->
24+
<tag name="kernel.event_listener" event="kernel.request" method="onSecurityPostDenormalize" priority="1" />
2325
</service>
2426
</services>
2527

‎src/EventListener/ReadListener.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,6 @@ public function onKernelRequest(GetResponseEvent $event): void
115115
}
116116

117117
$request->attributes->set('data', $data);
118-
$request->attributes->set('previous_data', \is_object($data) ? clone $data : $data);
118+
$request->attributes->set('previous_data', \is_object($data) && (new \ReflectionClass(\get_class($data)))->isCloneable() ? clone $data : $data);
119119
}
120120
}

‎src/GraphQl/Resolver/Factory/CollectionResolverFactory.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,12 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
9595
$collection = $this->collectionDataProvider->getCollection($resourceClass, null, $dataProviderContext);
9696
}
9797

98-
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
98+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, 'security', [
9999
'object' => $collection,
100-
'previous_object' => \is_object($collection) ? clone $collection : $collection,
100+
], $operationName ?? 'query');
101+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, 'security_post_denormalize', [
102+
'object' => $collection,
103+
'previous_object' => \is_object($collection) && (new \ReflectionClass(\get_class($collection)))->isCloneable() ? clone $collection : $collection,
101104
], $operationName ?? 'query');
102105

103106
if (!$this->paginationEnabled) {

‎src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,17 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
9696
throw Error::createLocatedError(sprintf('Item "%s" did not match expected type "%s".', $args['input']['id'], $resourceMetadata->getShortName()), $info->fieldNodes, $info->path);
9797
}
9898
}
99-
$previousItem = \is_object($item) ? clone $item : $item;
99+
$previousItem = \is_object($item) && (new \ReflectionClass(\get_class($item)))->isCloneable() ? clone $item : $item;
100+
101+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, 'security', [
102+
'object' => $item,
103+
], $operationName);
100104

101105
$inputMetadata = $resourceMetadata->getGraphqlAttribute($operationName, 'input', null, true);
102106
$inputClass = null;
103107
if (\is_array($inputMetadata) && \array_key_exists('class', $inputMetadata)) {
104108
if (null === $inputMetadata['class']) {
105-
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
109+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, 'security', [
106110
'object' => $item,
107111
'previous_object' => $previousItem,
108112
], $operationName);
@@ -121,7 +125,7 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
121125

122126
$context += $resourceMetadata->getGraphqlAttribute($operationName, 'denormalization_context', [], true);
123127
$item = $this->normalizer->denormalize($args['input'], $inputClass ?: $resourceClass, ItemNormalizer::FORMAT, $context);
124-
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
128+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, 'security_post_denormalize', [
125129
'object' => $item,
126130
'previous_object' => $previousItem,
127131
], $operationName);
@@ -135,7 +139,7 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
135139
return [$wrapFieldName => $this->normalizer->normalize($persistResult ?? $item, ItemNormalizer::FORMAT, $normalizationContext)] + $data;
136140
}
137141

138-
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
142+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, 'security_post_denormalize', [
139143
'object' => $item,
140144
'previous_object' => $previousItem,
141145
], $operationName);

‎src/GraphQl/Resolver/ItemResolver.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,12 @@ public function __invoke($source, $args, $context, ResolveInfo $info)
6969

7070
$resourceClass = $this->getObjectClass($item);
7171
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
72-
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
72+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, 'security', [
7373
'object' => $item,
74-
'previous_object' => clone $item,
74+
], 'query');
75+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, 'security_post_denormalize', [
76+
'object' => $item,
77+
'previous_object' => \is_object($item) && (new \ReflectionClass(\get_class($item)))->isCloneable() ? clone $item : $item,
7578
], 'query');
7679

7780
$normalizationContext = $resourceMetadata->getGraphqlAttribute('query', 'normalization_context', [], true);

‎src/GraphQl/Resolver/ResourceAccessCheckerTrait.php

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,29 @@ trait ResourceAccessCheckerTrait
3030
/**
3131
* @throws Error
3232
*/
33-
public function canAccess(?ResourceAccessCheckerInterface $resourceAccessChecker, ResourceMetadata $resourceMetadata, string $resourceClass, ResolveInfo $info, $extraVariables = [], string $operationName = null): void
33+
public function canAccess(?ResourceAccessCheckerInterface $resourceAccessChecker, ResourceMetadata $resourceMetadata, string $resourceClass, ResolveInfo $info, string $attribute, $extraVariables = [], string $operationName = null): void
3434
{
35+
if ('access_control' === $attribute) {
36+
@trigger_error('Sending "access_control" attribute is deprecated since API Platform 2.4 and will not be possible anymore in API Platform 3. Use "security" or "security_post_denormalize" attributes instead.', E_USER_DEPRECATED);
37+
}
38+
3539
if (null === $resourceAccessChecker) {
3640
return;
3741
}
3842

39-
$isGranted = $resourceMetadata->getGraphqlAttribute($operationName ?? '', 'access_control', null, true);
43+
$isGranted = $resourceMetadata->getGraphqlAttribute($operationName ?? '', $attribute, null, true);
44+
if (null === $isGranted) {
45+
// Backward compatibility
46+
$isGranted = $resourceMetadata->getGraphqlAttribute($operationName ?? '', 'access_control', null, true);
47+
if (null !== $isGranted) {
48+
@trigger_error('Using "access_control" attribute is deprecated since API Platform 2.4 and will not be possible anymore in API Platform 3. Use "security" attribute instead.', E_USER_DEPRECATED);
49+
}
50+
}
51+
4052
if (null === $isGranted || $resourceAccessChecker->isGranted($resourceClass, $isGranted, $extraVariables)) {
4153
return;
4254
}
4355

44-
throw Error::createLocatedError($resourceMetadata->getGraphqlAttribute($operationName ?? '', 'access_control_message', 'Access Denied.'), $info->fieldNodes, $info->path);
56+
throw Error::createLocatedError($resourceMetadata->getGraphqlAttribute($operationName ?? '', 'security_message', 'Access Denied.'), $info->fieldNodes, $info->path);
4557
}
4658
}

‎src/Security/EventListener/DenyAccessListener.php

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use ApiPlatform\Core\Security\ResourceAccessChecker;
1919
use ApiPlatform\Core\Security\ResourceAccessCheckerInterface;
2020
use ApiPlatform\Core\Util\RequestAttributesExtractor;
21+
use Symfony\Component\HttpFoundation\Request;
2122
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
2223
use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface;
2324
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
@@ -52,31 +53,55 @@ public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFa
5253
@trigger_error(sprintf('Passing an instance of "%s" or null as second argument of "%s" is deprecated since API Platform 2.2 and will not be possible anymore in API Platform 3. Pass an instance of "%s" and no extra argument instead.', ExpressionLanguage::class, self::class, ResourceAccessCheckerInterface::class), E_USER_DEPRECATED);
5354
}
5455

56+
public function onKernelRequest(GetResponseEvent $event): void
57+
{
58+
@trigger_error(sprintf('Method "%1$s::onKernelRequest" is deprecated since API Platform 2.4 and will not be available anymore in API Platform 3. Prefer calling "%1$s::onSecurity" instead.', self::class), E_USER_DEPRECATED);
59+
$this->onSecurityPostDenormalize($event);
60+
}
61+
62+
public function onSecurity(GetResponseEvent $event): void
63+
{
64+
$this->checkSecurity($event->getRequest(), 'security', false);
65+
}
66+
67+
public function onSecurityPostDenormalize(GetResponseEvent $event): void
68+
{
69+
$request = $event->getRequest();
70+
$this->checkSecurity($request, 'security_post_denormalize', true, [
71+
'previous_object' => $request->attributes->get('previous_data'),
72+
]);
73+
}
74+
5575
/**
5676
* @throws AccessDeniedException
5777
*/
58-
public function onKernelRequest(GetResponseEvent $event): void
78+
private function checkSecurity(Request $request, string $attribute, bool $backwardCompatibility, array $extraVariables = []): void
5979
{
60-
$request = $event->getRequest();
6180
if (!$attributes = RequestAttributesExtractor::extractAttributes($request)) {
6281
return;
6382
}
6483

6584
$resourceMetadata = $this->resourceMetadataFactory->create($attributes['resource_class']);
6685

67-
$isGranted = $resourceMetadata->getOperationAttribute($attributes, 'access_control', null, true);
86+
$isGranted = $resourceMetadata->getOperationAttribute($attributes, $attribute, null, true);
87+
if ($backwardCompatibility && null === $isGranted) {
88+
// Backward compatibility
89+
$isGranted = $resourceMetadata->getOperationAttribute($attributes, 'access_control', null, true);
90+
if (null !== $isGranted) {
91+
@trigger_error('Using "access_control" attribute is deprecated since API Platform 2.4 and will not be possible anymore in API Platform 3. Use "security" attribute instead.', E_USER_DEPRECATED);
92+
}
93+
}
6894

6995
if (null === $isGranted) {
7096
return;
7197
}
7298

73-
$extraVariables = $request->attributes->all();
99+
$extraVariables += $request->attributes->all();
74100
$extraVariables['object'] = $request->attributes->get('data');
75-
$extraVariables['previous_object'] = $request->attributes->get('previous_data');
76101
$extraVariables['request'] = $request;
77102

78103
if (!$this->resourceAccessChecker->isGranted($attributes['resource_class'], $isGranted, $extraVariables)) {
79-
throw new AccessDeniedException($resourceMetadata->getOperationAttribute($attributes, 'access_control_message', 'Access Denied.', true));
104+
throw new AccessDeniedException($resourceMetadata->getOperationAttribute($attributes, $attribute.'_message', 'Access Denied.', true));
80105
}
81106
}
82107
}

‎src/Security/ResourceAccessChecker.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,13 @@ public function __construct(ExpressionLanguage $expressionLanguage = null, Authe
4545
public function isGranted(string $resourceClass, string $expression, array $extraVariables = []): bool
4646
{
4747
if (null === $this->tokenStorage || null === $this->authenticationTrustResolver) {
48-
throw new \LogicException('The "symfony/security" library must be installed to use the "access_control" attribute.');
48+
throw new \LogicException('The "symfony/security" library must be installed to use the "security" attribute.');
4949
}
5050
if (null === $token = $this->tokenStorage->getToken()) {
51-
throw new \LogicException('The current token must be set to use the "access_control" attribute (is the URL behind a firewall?).');
51+
throw new \LogicException('The current token must be set to use the "security" attribute (is the URL behind a firewall?).');
5252
}
5353
if (null === $this->expressionLanguage) {
54-
throw new \LogicException('The "symfony/expression-language" library must be installed to use the "access_control".');
54+
throw new \LogicException('The "symfony/expression-language" library must be installed to use the "security".');
5555
}
5656

5757
return (bool) $this->expressionLanguage->evaluate($expression, array_merge($extraVariables, $this->getVariables($token)));

0 commit comments

Comments
 (0)