Skip to content

Commit 076e008

Browse files
dunglasdunglas
authored
GraphQL: add support for previous_object in access_control (#2814)
* GraphQL: add support for previous_object in access_control * Try to fix MongoDB tests * Use gender neutral pronouns * Typo
1 parent d0b3ae6 commit 076e008

File tree

8 files changed

+167
-41
lines changed

8 files changed

+167
-41
lines changed

‎features/authorization/deny.feature

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,20 @@ Feature: Authorization checking
5858
"""
5959
Then the response status code should be 201
6060

61-
Scenario: A user cannot retrieve an item he doesn't own
61+
Scenario: A user cannot retrieve an item they doesn't own
6262
When I add "Accept" header equal to "application/ld+json"
6363
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
6464
And I send a "GET" request to "/secured_dummies/1"
6565
Then the response status code should be 403
6666
And the response should be in JSON
6767

68-
Scenario: A user can retrieve an item he owns
68+
Scenario: A user can retrieve an item they owns
6969
When I add "Accept" header equal to "application/ld+json"
7070
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
7171
And I send a "GET" request to "/secured_dummies/2"
7272
Then the response status code should be 200
7373

74-
Scenario: A user can't assign him an item he doesn't own
74+
Scenario: A user can't assign to themself an item they doesn't own
7575
When I add "Accept" header equal to "application/ld+json"
7676
And I add "Content-Type" header equal to "application/ld+json"
7777
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
@@ -83,7 +83,7 @@ Feature: Authorization checking
8383
"""
8484
Then the response status code should be 403
8585

86-
Scenario: A user can update an item he owns and transfer it
86+
Scenario: A user can update an item they owns and transfer it
8787
When I add "Accept" header equal to "application/ld+json"
8888
And I add "Content-Type" header equal to "application/ld+json"
8989
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="

‎features/graphql/authorization.feature

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ Feature: Authorization checking
4040
And the header "Content-Type" should be equal to "application/json"
4141
And the JSON node "errors[0].message" should be equal to "Access Denied."
4242

43-
Scenario: An anonymous user tries to create a resource he is not allowed to
43+
Scenario: An anonymous user tries to create a resource they are not allowed to
4444
When I send the following GraphQL request:
4545
"""
4646
mutation {
@@ -56,3 +56,112 @@ Feature: Authorization checking
5656
And the response should be in JSON
5757
And the header "Content-Type" should be equal to "application/json"
5858
And the JSON node "errors[0].message" should be equal to "Only admins can create a secured dummy."
59+
60+
@createSchema
61+
Scenario: An admin can create a secured resource
62+
When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
63+
And I send the following GraphQL request:
64+
"""
65+
mutation {
66+
createSecuredDummy(input: {owner: "someone", title: "Hi", description: "Desc"}) {
67+
securedDummy {
68+
id
69+
title
70+
owner
71+
}
72+
}
73+
}
74+
"""
75+
Then the response status code should be 200
76+
And the response should be in JSON
77+
And the header "Content-Type" should be equal to "application/json"
78+
And the JSON node "data.createSecuredDummy.securedDummy.owner" should be equal to "someone"
79+
80+
Scenario: An admin can create another secured resource
81+
When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
82+
And I send the following GraphQL request:
83+
"""
84+
mutation {
85+
createSecuredDummy(input: {owner: "dunglas", title: "Hi", description: "Desc"}) {
86+
securedDummy {
87+
id
88+
title
89+
owner
90+
}
91+
}
92+
}
93+
"""
94+
Then the response status code should be 200
95+
And the response should be in JSON
96+
And the header "Content-Type" should be equal to "application/json"
97+
And the JSON node "data.createSecuredDummy.securedDummy.owner" should be equal to "dunglas"
98+
99+
Scenario: A user cannot retrieve an item they doesn't own
100+
When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
101+
And I send the following GraphQL request:
102+
"""
103+
{
104+
securedDummy(id: "/secured_dummies/1") {
105+
owner
106+
title
107+
}
108+
}
109+
"""
110+
Then the response status code should be 200
111+
And the response should be in JSON
112+
And the header "Content-Type" should be equal to "application/json"
113+
And the JSON node "errors[0].message" should be equal to "Access Denied."
114+
115+
Scenario: A user can retrieve an item they owns
116+
When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
117+
And I send the following GraphQL request:
118+
"""
119+
{
120+
securedDummy(id: "/secured_dummies/2") {
121+
owner
122+
title
123+
}
124+
}
125+
"""
126+
Then the response status code should be 200
127+
And the response should be in JSON
128+
And the header "Content-Type" should be equal to "application/json"
129+
And the JSON node "data.securedDummy.owner" should be equal to the string "dunglas"
130+
131+
Scenario: A user can't assign to themself an item they doesn't own
132+
When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
133+
And I send the following GraphQL request:
134+
"""
135+
mutation {
136+
updateSecuredDummy(input: {id: "/secured_dummies/1", owner: "kitten"}) {
137+
securedDummy {
138+
id
139+
title
140+
owner
141+
}
142+
}
143+
}
144+
"""
145+
Then the response status code should be 200
146+
And the response should be in JSON
147+
And the header "Content-Type" should be equal to "application/json"
148+
And the JSON node "errors[0].message" should be equal to "Access Denied."
149+
150+
Scenario: A user can update an item they owns and transfer it
151+
When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
152+
And I send the following GraphQL request:
153+
"""
154+
mutation {
155+
updateSecuredDummy(input: {id: "/secured_dummies/2", owner: "vincent"}) {
156+
securedDummy {
157+
id
158+
title
159+
owner
160+
}
161+
}
162+
}
163+
"""
164+
Then the response status code should be 200
165+
And the response should be in JSON
166+
And the header "Content-Type" should be equal to "application/json"
167+
And the JSON node "data.updateSecuredDummy.securedDummy.owner" should be equal to the string "vincent"

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
9292
$collection = $this->collectionDataProvider->getCollection($resourceClass, null, $dataProviderContext);
9393
}
9494

95-
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, $collection, $operationName ?? 'query');
95+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
96+
'object' => $collection,
97+
'previous_object' => \is_object($collection) ? clone $collection : $collection,
98+
], $operationName ?? 'query');
9699

97100
if (!$this->paginationEnabled) {
98101
$data = [];

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

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -93,43 +93,56 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
9393
throw Error::createLocatedError(sprintf('Item "%s" did not match expected type "%s".', $args['input']['id'], $resourceMetadata->getShortName()), $info->fieldNodes, $info->path);
9494
}
9595
}
96-
97-
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, $item, $operationName);
96+
$previousItem = \is_object($item) ? clone $item : $item;
9897

9998
$inputMetadata = $resourceMetadata->getGraphqlAttribute($operationName, 'input', null, true);
10099
$inputClass = null;
101100
if (\is_array($inputMetadata) && \array_key_exists('class', $inputMetadata)) {
102101
if (null === $inputMetadata['class']) {
102+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
103+
'object' => $item,
104+
'previous_object' => $previousItem,
105+
], $operationName);
106+
103107
return $data;
104108
}
105109

106110
$inputClass = $inputMetadata['class'];
107111
}
108112

109-
switch ($operationName) {
110-
case 'create':
111-
case 'update':
112-
$context = ['resource_class' => $resourceClass, 'graphql_operation_name' => $operationName];
113-
if (null !== $item) {
114-
$context['object_to_populate'] = $item;
115-
}
116-
$context += $resourceMetadata->getGraphqlAttribute($operationName, 'denormalization_context', [], true);
117-
$item = $this->normalizer->denormalize($args['input'], $inputClass ?: $resourceClass, ItemNormalizer::FORMAT, $context);
118-
$this->validate($item, $info, $resourceMetadata, $operationName);
119-
$persistResult = $this->dataPersister->persist($item, $context);
120-
121-
if (null === $persistResult) {
122-
@trigger_error(sprintf('Returning void from %s::persist() is deprecated since API Platform 2.3 and will not be supported in API Platform 3, an object should always be returned.', DataPersisterInterface::class), E_USER_DEPRECATED);
123-
}
124-
125-
return [$wrapFieldName => $this->normalizer->normalize($persistResult ?? $item, ItemNormalizer::FORMAT, $normalizationContext)] + $data;
126-
case 'delete':
127-
if ($item) {
128-
$this->dataPersister->remove($item);
129-
$data[$wrapFieldName]['id'] = $args['input']['id'];
130-
} else {
131-
$data[$wrapFieldName]['id'] = null;
132-
}
113+
if ('create' === $operationName || 'update' === $operationName) {
114+
$context = ['resource_class' => $resourceClass, 'graphql_operation_name' => $operationName];
115+
if (null !== $item) {
116+
$context['object_to_populate'] = $item;
117+
}
118+
119+
$context += $resourceMetadata->getGraphqlAttribute($operationName, 'denormalization_context', [], true);
120+
$item = $this->normalizer->denormalize($args['input'], $inputClass ?: $resourceClass, ItemNormalizer::FORMAT, $context);
121+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
122+
'object' => $item,
123+
'previous_object' => $previousItem,
124+
], $operationName);
125+
$this->validate($item, $info, $resourceMetadata, $operationName);
126+
$persistResult = $this->dataPersister->persist($item, $context);
127+
128+
if (null === $persistResult) {
129+
@trigger_error(sprintf('Returning void from %s::persist() is deprecated since API Platform 2.3 and will not be supported in API Platform 3, an object should always be returned.', DataPersisterInterface::class), E_USER_DEPRECATED);
130+
}
131+
132+
return [$wrapFieldName => $this->normalizer->normalize($persistResult ?? $item, ItemNormalizer::FORMAT, $normalizationContext)] + $data;
133+
}
134+
135+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
136+
'object' => $item,
137+
'previous_object' => $previousItem,
138+
], $operationName);
139+
140+
if ('delete' === $operationName) {
141+
$data[$wrapFieldName]['id'] = null;
142+
if ($item) {
143+
$this->dataPersister->remove($item);
144+
$data[$wrapFieldName]['id'] = $args['input']['id'];
145+
}
133146
}
134147

135148
return $data;

‎src/GraphQl/Resolver/ItemResolver.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,10 @@ 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, $item, 'query');
72+
$this->canAccess($this->resourceAccessChecker, $resourceMetadata, $resourceClass, $info, [
73+
'object' => $item,
74+
'previous_object' => \is_object($item) ? clone $item : $item,
75+
], 'query');
7376

7477
$normalizationContext = $resourceMetadata->getGraphqlAttribute('query', 'normalization_context', [], true);
7578
$normalizationContext['resource_class'] = $resourceClass;

‎src/GraphQl/Resolver/ResourceAccessCheckerTrait.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,16 @@
2828
trait ResourceAccessCheckerTrait
2929
{
3030
/**
31-
* @param mixed|null $object
32-
*
3331
* @throws Error
3432
*/
35-
public function canAccess(?ResourceAccessCheckerInterface $resourceAccessChecker, ResourceMetadata $resourceMetadata, string $resourceClass, ResolveInfo $info, $object = null, string $operationName = null): void
33+
public function canAccess(?ResourceAccessCheckerInterface $resourceAccessChecker, ResourceMetadata $resourceMetadata, string $resourceClass, ResolveInfo $info, $extraVariables = [], string $operationName = null): void
3634
{
3735
if (null === $resourceAccessChecker) {
3836
return;
3937
}
4038

4139
$isGranted = $resourceMetadata->getGraphqlAttribute($operationName ?? '', 'access_control', null, true);
42-
if (null === $isGranted || $resourceAccessChecker->isGranted($resourceClass, $isGranted, ['object' => $object])) {
40+
if (null === $isGranted || $resourceAccessChecker->isGranted($resourceClass, $isGranted, $extraVariables)) {
4341
return;
4442
}
4543

‎tests/Fixtures/TestBundle/Document/SecuredDummy.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@
3434
* "put"={"access_control"="has_role('ROLE_USER') and previous_object.getOwner() == user"},
3535
* },
3636
* graphql={
37-
* "query"={},
37+
* "query"={"access_control"="has_role('ROLE_USER') and object.getOwner() == user"},
3838
* "delete"={},
39-
* "update"={},
39+
* "update"={"access_control"="has_role('ROLE_USER') and previous_object.getOwner() == user"},
4040
* "create"={"access_control"="has_role('ROLE_ADMIN')", "access_control_message"="Only admins can create a secured dummy."}
4141
* }
4242
* )

‎tests/Fixtures/TestBundle/Entity/SecuredDummy.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@
3333
* "put"={"access_control"="has_role('ROLE_USER') and previous_object.getOwner() == user"},
3434
* },
3535
* graphql={
36-
* "query"={},
36+
* "query"={"access_control"="has_role('ROLE_USER') and object.getOwner() == user"},
3737
* "delete"={},
38-
* "update"={},
38+
* "update"={"access_control"="has_role('ROLE_USER') and previous_object.getOwner() == user"},
3939
* "create"={"access_control"="has_role('ROLE_ADMIN')", "access_control_message"="Only admins can create a secured dummy."}
4040
* }
4141
* )

0 commit comments

Comments
 (0)