Skip to content

Commit de5988d

Browse files
DavidBennettUKDavidBennettUK
authored
feat: add security_post_denormalize in ApiProperty (#4184)
1 parent 8671e3b commit de5988d

File tree

10 files changed

+560
-3
lines changed

10 files changed

+560
-3
lines changed

‎CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
## 2.7.0
44

5+
* Security: **BC** Fix `ApiProperty` `security` attribute expression being passed a class string for the `object` variable on updates/creates - null is now passed instead if the object is not available (#4184)
6+
* Security: `ApiProperty` now supports a `security_post_denormalize` attribute, which provides access to the `object` variable for the object being updated/created and `previous_object` for the object before it was updated (#4184)
57
* JSON Schema: Add support for generating property schema with numeric constraint restrictions (#4225)
68
* JSON Schema: Add support for generating property schema with Collection restriction (#4182)
79
* JSON Schema: Add support for generating property schema format for Url and Hostname (#4185)

‎features/authorization/deny.feature

+30
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,22 @@ Feature: Authorization checking
7878
And I send a "GET" request to "/secured_dummies/2"
7979
Then the response status code should be 200
8080

81+
Scenario: A user can see a secured owner-only property on an object they own
82+
When I add "Accept" header equal to "application/ld+json"
83+
And I add "Content-Type" header equal to "application/ld+json"
84+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
85+
And I send a "GET" request to "/secured_dummies/2"
86+
Then the response status code should be 200
87+
And the JSON node "ownerOnlyProperty" should exist
88+
And the JSON node "ownerOnlyProperty" should not be null
89+
90+
Scenario: An admin can't see a secured owner-only property on objects they don't own
91+
When I add "Accept" header equal to "application/ld+json"
92+
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
93+
And I send a "GET" request to "/secured_dummies"
94+
Then the response status code should be 200
95+
And the response should not contain "ownerOnlyProperty"
96+
8197
Scenario: A user can't assign to themself an item they doesn't own
8298
When I add "Accept" header equal to "application/ld+json"
8399
And I add "Content-Type" header equal to "application/ld+json"
@@ -151,3 +167,17 @@ Feature: Authorization checking
151167
Then the response status code should be 200
152168
And the response should contain "adminOnlyProperty"
153169
And the JSON node "hydra:member[2].adminOnlyProperty" should be equal to the string "Is it safe?"
170+
171+
Scenario: An user can update an owner-only secured property on an object they own
172+
When I add "Accept" header equal to "application/ld+json"
173+
And I add "Content-Type" header equal to "application/ld+json"
174+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
175+
And I send a "PUT" request to "/secured_dummies/3" with body:
176+
"""
177+
{
178+
"ownerOnlyProperty": "updated"
179+
}
180+
"""
181+
Then the response status code should be 200
182+
And the response should contain "ownerOnlyProperty"
183+
And the JSON node "ownerOnlyProperty" should be equal to the string "updated"

‎features/graphql/authorization.feature

+123
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,69 @@ Feature: Authorization checking
349349
And the header "Content-Type" should be equal to "application/json"
350350
And the JSON node "data.createSecuredDummy.securedDummy.owner" should be equal to "dunglas"
351351

352+
Scenario: An admin can create a secured resource with an owner-only property if they will be the owner
353+
When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
354+
And I send the following GraphQL request:
355+
"""
356+
mutation {
357+
createSecuredDummy(input: {owner: "admin", title: "Hi", description: "Desc", adminOnlyProperty: "secret", ownerOnlyProperty: "it works"}) {
358+
securedDummy {
359+
ownerOnlyProperty
360+
}
361+
}
362+
}
363+
"""
364+
Then the response status code should be 200
365+
And the response should be in JSON
366+
And the header "Content-Type" should be equal to "application/json"
367+
And the JSON node "data.createSecuredDummy.securedDummy.ownerOnlyProperty" should be equal to the string "it works"
368+
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
369+
And I send the following GraphQL request:
370+
"""
371+
{
372+
securedDummies {
373+
edges {
374+
node {
375+
ownerOnlyProperty
376+
}
377+
}
378+
}
379+
}
380+
"""
381+
Then the response status code should be 200
382+
And the response should be in JSON
383+
And the JSON node "data.securedDummies.edges[2].node.ownerOnlyProperty" should be equal to "it works"
384+
385+
Scenario: An admin can't create a secured resource with an owner-only property if they won't be the owner
386+
When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
387+
And I send the following GraphQL request:
388+
"""
389+
mutation {
390+
createSecuredDummy(input: {owner: "dunglas", title: "Hi", description: "Desc", adminOnlyProperty: "secret", ownerOnlyProperty: "should not be set"}) {
391+
securedDummy {
392+
ownerOnlyProperty
393+
}
394+
}
395+
}
396+
"""
397+
Then the response status code should be 200
398+
And the response should be in JSON
399+
And the header "Content-Type" should be equal to "application/json"
400+
And the JSON node "data.createSecuredDummy.securedDummy.ownerOnlyProperty" should exist
401+
And the JSON node "data.createSecuredDummy.securedDummy.ownerOnlyProperty" should be null
402+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
403+
And I send the following GraphQL request:
404+
"""
405+
{
406+
securedDummy(id: "/secured_dummies/4") {
407+
ownerOnlyProperty
408+
}
409+
}
410+
"""
411+
Then the response status code should be 200
412+
And the response should be in JSON
413+
And the JSON node "data.securedDummy.ownerOnlyProperty" should be equal to the string ""
414+
352415
Scenario: A user cannot retrieve an item they doesn't own
353416
When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
354417
And I send the following GraphQL request:
@@ -419,6 +482,66 @@ Feature: Authorization checking
419482
And the header "Content-Type" should be equal to "application/json"
420483
And the JSON node "data.securedDummy.adminOnlyProperty" should be null
421484

485+
Scenario: A user can see a secured owner-only property on an object they own
486+
When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
487+
And I send the following GraphQL request:
488+
"""
489+
{
490+
securedDummy(id: "/secured_dummies/2") {
491+
ownerOnlyProperty
492+
}
493+
}
494+
"""
495+
Then the response status code should be 200
496+
And the response should be in JSON
497+
And the header "Content-Type" should be equal to "application/json"
498+
And the JSON node "data.securedDummy.ownerOnlyProperty" should exist
499+
And the JSON node "data.securedDummy.ownerOnlyProperty" should not be null
500+
501+
Scenario: A user can update a secured owner-only property on an object they own
502+
When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
503+
And I send the following GraphQL request:
504+
"""
505+
mutation {
506+
updateSecuredDummy(input: {id: "/secured_dummies/2", ownerOnlyProperty: "updated"}) {
507+
securedDummy {
508+
ownerOnlyProperty
509+
}
510+
}
511+
}
512+
"""
513+
Then the response status code should be 200
514+
And the response should be in JSON
515+
And the header "Content-Type" should be equal to "application/json"
516+
And the JSON node "data.updateSecuredDummy.securedDummy.ownerOnlyProperty" should be equal to the string "updated"
517+
And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
518+
And I send the following GraphQL request:
519+
"""
520+
{
521+
securedDummy(id: "/secured_dummies/2") {
522+
ownerOnlyProperty
523+
}
524+
}
525+
"""
526+
Then the response status code should be 200
527+
And the response should be in JSON
528+
And the JSON node "data.securedDummy.ownerOnlyProperty" should be equal to the string "updated"
529+
530+
Scenario: An admin can't see a secured owner-only property on an object they don't own
531+
When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
532+
And I send the following GraphQL request:
533+
"""
534+
{
535+
securedDummy(id: "/secured_dummies/2") {
536+
ownerOnlyProperty
537+
}
538+
}
539+
"""
540+
Then the response status code should be 200
541+
And the response should be in JSON
542+
And the header "Content-Type" should be equal to "application/json"
543+
And the JSON node "data.securedDummy.ownerOnlyProperty" should be null
544+
422545
Scenario: A user can't assign to themself an item they doesn't own
423546
When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
424547
And I send the following GraphQL request:

‎src/Annotation/ApiProperty.php

+4-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
* @Attribute("jsonldContext", type="array"),
3131
* @Attribute("push", type="bool"),
3232
* @Attribute("security", type="string"),
33+
* @Attribute("securityPostDenormalize", type="string"),
3334
* @Attribute("swaggerContext", type="array")
3435
* )
3536
*/
@@ -112,6 +113,7 @@ final class ApiProperty
112113
* @param bool $push
113114
* @param string $security
114115
* @param array $swaggerContext
116+
* @param string $securityPostDenormalize
115117
*
116118
* @throws InvalidArgumentException
117119
*/
@@ -136,7 +138,8 @@ public function __construct(
136138
?array $openapiContext = null,
137139
?bool $push = null,
138140
?string $security = null,
139-
?array $swaggerContext = null
141+
?array $swaggerContext = null,
142+
?string $securityPostDenormalize = null
140143
) {
141144
if (!\is_array($description)) { // @phpstan-ignore-line Doctrine annotations support
142145
[$publicProperties, $configurableAttributes] = self::getConfigMetadata();

‎src/Serializer/AbstractItemNormalizer.php

+48-2
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,22 @@ public function denormalize($data, $class, $format = null, array $context = [])
254254
return $item;
255255
}
256256

257-
return parent::denormalize($data, $resourceClass, $format, $context);
257+
$previousObject = null !== $objectToPopulate ? clone $objectToPopulate : null;
258+
$object = parent::denormalize($data, $resourceClass, $format, $context);
259+
260+
// Revert attributes that aren't allowed to be changed after a post-denormalize check
261+
foreach (array_keys($data) as $attribute) {
262+
if (!$this->canAccessAttributePostDenormalize($object, $previousObject, $attribute, $context)) {
263+
if (null !== $previousObject) {
264+
$this->setValue($object, $attribute, $this->propertyAccessor->getValue($previousObject, $attribute));
265+
} else {
266+
$propertyMetaData = $this->propertyMetadataFactory->create($resourceClass, $attribute, $this->getFactoryOptions($context));
267+
$this->setValue($object, $attribute, $propertyMetaData->getDefault());
268+
}
269+
}
270+
}
271+
272+
return $object;
258273
}
259274

260275
/**
@@ -390,12 +405,43 @@ protected function isAllowedAttribute($classOrObject, $attribute, $format = null
390405
return false;
391406
}
392407

408+
return $this->canAccessAttribute(\is_object($classOrObject) ? $classOrObject : null, $attribute, $context);
409+
}
410+
411+
/**
412+
* Check if access to the attribute is granted.
413+
*
414+
* @param object $object
415+
*/
416+
protected function canAccessAttribute($object, string $attribute, array $context = []): bool
417+
{
393418
$options = $this->getFactoryOptions($context);
394419
$propertyMetadata = $this->propertyMetadataFactory->create($context['resource_class'], $attribute, $options);
395420
$security = $propertyMetadata->getAttribute('security');
396421
if ($this->resourceAccessChecker && $security) {
397422
return $this->resourceAccessChecker->isGranted($context['resource_class'], $security, [
398-
'object' => $classOrObject,
423+
'object' => $object,
424+
]);
425+
}
426+
427+
return true;
428+
}
429+
430+
/**
431+
* Check if access to the attribute is granted.
432+
*
433+
* @param object $object
434+
* @param object|null $previousObject
435+
*/
436+
protected function canAccessAttributePostDenormalize($object, $previousObject, string $attribute, array $context = []): bool
437+
{
438+
$options = $this->getFactoryOptions($context);
439+
$propertyMetadata = $this->propertyMetadataFactory->create($context['resource_class'], $attribute, $options);
440+
$security = $propertyMetadata->getAttribute('security_post_denormalize');
441+
if ($this->resourceAccessChecker && $security) {
442+
return $this->resourceAccessChecker->isGranted($context['resource_class'], $security, [
443+
'object' => $object,
444+
'previous_object' => $previousObject,
399445
]);
400446
}
401447

‎tests/Annotation/ApiPropertyTest.php

+4
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public function testConstruct()
5353
'fetchEager' => false,
5454
'jsonldContext' => ['foo' => 'bar'],
5555
'security' => 'is_granted(\'ROLE_ADMIN\')',
56+
'securityPostDenormalize' => 'is_granted(\'VIEW\', object)',
5657
'swaggerContext' => ['foo' => 'baz'],
5758
'openapiContext' => ['foo' => 'baz'],
5859
'push' => true,
@@ -64,6 +65,7 @@ public function testConstruct()
6465
'fetch_eager' => false,
6566
'jsonld_context' => ['foo' => 'bar'],
6667
'security' => 'is_granted(\'ROLE_ADMIN\')',
68+
'security_post_denormalize' => 'is_granted(\'VIEW\', object)',
6769
'swagger_context' => ['foo' => 'baz'],
6870
'openapi_context' => ['foo' => 'baz'],
6971
'push' => true,
@@ -83,6 +85,7 @@ public function testConstructAttribute()
8385
fetchEager: false,
8486
jsonldContext: ['foo' => 'bar'],
8587
security: 'is_granted(\'ROLE_ADMIN\')',
88+
securityPostDenormalize: 'is_granted(\'VIEW\', object)',
8689
swaggerContext: ['foo' => 'baz'],
8790
openapiContext: ['foo' => 'baz'],
8891
push: true,
@@ -97,6 +100,7 @@ public function testConstructAttribute()
97100
'fetch_eager' => false,
98101
'jsonld_context' => ['foo' => 'bar'],
99102
'security' => 'is_granted(\'ROLE_ADMIN\')',
103+
'security_post_denormalize' => 'is_granted(\'VIEW\', object)',
100104
'swagger_context' => ['foo' => 'baz'],
101105
'openapi_context' => ['foo' => 'baz'],
102106
'push' => true,

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

+21
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,17 @@ class SecuredDummy
8383
*/
8484
private $adminOnlyProperty = '';
8585

86+
/**
87+
* @var string Secret property, only readable/writable by owners
88+
*
89+
* @ODM\Field
90+
* @ApiProperty(
91+
* security="object == null or object.getOwner() == user",
92+
* securityPostDenormalize="object.getOwner() == user",
93+
* )
94+
*/
95+
private $ownerOnlyProperty = '';
96+
8697
/**
8798
* @var string The owner
8899
*
@@ -187,6 +198,16 @@ public function setAdminOnlyProperty(?string $adminOnlyProperty)
187198
$this->adminOnlyProperty = $adminOnlyProperty;
188199
}
189200

201+
public function getOwnerOnlyProperty(): ?string
202+
{
203+
return $this->ownerOnlyProperty;
204+
}
205+
206+
public function setOwnerOnlyProperty(?string $ownerOnlyProperty)
207+
{
208+
$this->ownerOnlyProperty = $ownerOnlyProperty;
209+
}
210+
190211
public function getOwner(): string
191212
{
192213
return $this->owner;

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

+21
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ class SecuredDummy
8484
*/
8585
private $adminOnlyProperty = '';
8686

87+
/**
88+
* @var string Secret property, only readable/writable by owners
89+
*
90+
* @ORM\Column
91+
* @ApiProperty(
92+
* security="object == null or object.getOwner() == user",
93+
* securityPostDenormalize="object.getOwner() == user",
94+
* )
95+
*/
96+
private $ownerOnlyProperty = '';
97+
8798
/**
8899
* @var string The owner
89100
*
@@ -198,6 +209,16 @@ public function setAdminOnlyProperty(?string $adminOnlyProperty)
198209
$this->adminOnlyProperty = $adminOnlyProperty;
199210
}
200211

212+
public function getOwnerOnlyProperty(): ?string
213+
{
214+
return $this->ownerOnlyProperty;
215+
}
216+
217+
public function setOwnerOnlyProperty(?string $ownerOnlyProperty)
218+
{
219+
$this->ownerOnlyProperty = $ownerOnlyProperty;
220+
}
221+
201222
public function getOwner(): string
202223
{
203224
return $this->owner;

0 commit comments

Comments
 (0)