Skip to content

Commit

Permalink
fix: securityPostDenormalize not working because clone is made after …
Browse files Browse the repository at this point in the history
…denormalization (#5182)

* fix: securityPostDenormalize not working because clone is made after denormalization

* test: add non-regression tests
  • Loading branch information
LoicBoursin authored Nov 15, 2022
1 parent 62af874 commit d188135
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 1 deletion.
27 changes: 27 additions & 0 deletions features/authorization/deny.feature
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,33 @@ Feature: Authorization checking
And the JSON node "ownerOnlyProperty" should exist
And the JSON node "ownerOnlyProperty" should not be null

Scenario: An admin can create a secured resource with properties depending on themselves
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
And I send a "POST" request to "/secured_dummy_with_properties_depending_on_themselves" with body:
"""
{
"canUpdateProperty": false,
"property": false
}
"""
Then the response status code should be 201

Scenario: A user cannot patch a secured property if not granted
When I add "Content-Type" header equal to "application/merge-patch+json"
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
And I send a "PATCH" request to "/secured_dummy_with_properties_depending_on_themselves/1" with body:
"""
{
"canUpdateProperty": true,
"property": true
}
"""
Then the response status code should be 200
And the JSON node "canUpdateProperty" should be true
And the JSON node "property" should be false

Scenario: An admin can't see a secured owner-only property on objects they don't own
When I add "Accept" header equal to "application/ld+json"
And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu"
Expand Down
3 changes: 2 additions & 1 deletion src/Serializer/AbstractItemNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,14 @@ public function denormalize(mixed $data, string $class, string $format = null, a
throw new UnexpectedValueException(sprintf('Expected IRI or document for resource "%s", "%s" given.', $resourceClass, \gettype($data)));
}

$previousObject = isset($objectToPopulate) ? clone $objectToPopulate : null;

$object = parent::denormalize($data, $class, $format, $context);

if (!$this->resourceClassResolver->isResourceClass($class)) {
return $object;
}

$previousObject = isset($objectToPopulate) ? clone $objectToPopulate : null;
// Revert attributes that aren't allowed to be changed after a post-denormalize check
foreach (array_keys($data) as $attribute) {
if (!$this->canAccessAttributePostDenormalize($object, $previousObject, $attribute, $context)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php

/*
* This file is part of the API Platform project.
*
* (c) Kévin Dunglas <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace ApiPlatform\Tests\Fixtures\TestBundle\Entity;

use ApiPlatform\Metadata\ApiProperty;
use ApiPlatform\Metadata\ApiResource;
use ApiPlatform\Metadata\Get;
use ApiPlatform\Metadata\Patch;
use ApiPlatform\Metadata\Post;
use Doctrine\ORM\Mapping as ORM;

/**
* Secured resource with properties depending on themselves.
*
* @author Loïc Boursin <[email protected]>
*/
#[ApiResource(
operations: [
new Get(),
new Patch(inputFormats: ['json' => ['application/merge-patch+json'], 'jsonapi']),
new Post(security: 'is_granted("ROLE_ADMIN")'),
]
)]
#[ORM\Entity]
class SecuredDummyWithPropertiesDependingOnThemselves
{
#[ORM\Column(type: 'integer')]
#[ORM\Id]
#[ORM\GeneratedValue(strategy: 'AUTO')]
private ?int $id = null;

#[ORM\Column]
private bool $canUpdateProperty = false;

/**
* @var bool Special property, only writable when granted rights by another property
*/
#[ApiProperty(securityPostDenormalize: 'previous_object and previous_object.getCanUpdateProperty()')]
#[ORM\Column]
private bool $property = false;

public function __construct()
{
}

public function getId(): ?int
{
return $this->id;
}

public function getCanUpdateProperty(): bool
{
return $this->canUpdateProperty;
}

public function setCanUpdateProperty(bool $canUpdateProperty): void
{
$this->canUpdateProperty = $canUpdateProperty;
}

public function getProperty(): bool
{
return $this->property;
}

public function setProperty(bool $property): void
{
$this->property = $property;
}
}

0 comments on commit d188135

Please sign in to comment.