From 0e1de7a36129d77038704897a91eb31749112ff5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Boursin?= Date: Sat, 12 Nov 2022 20:05:47 +0100 Subject: [PATCH 1/2] fix: securityPostDenormalize not working because clone is made after denormalization --- src/Serializer/AbstractItemNormalizer.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 47f0cc9fe1f..073b586d730 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -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)) { From 0d6cac7861b70f8248463cf7f0e4f3585c07e499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Boursin?= Date: Mon, 14 Nov 2022 21:30:09 +0100 Subject: [PATCH 2/2] test: add non-regression tests --- features/authorization/deny.feature | 27 +++++++ ...mmyWithPropertiesDependingOnThemselves.php | 81 +++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 tests/Fixtures/TestBundle/Entity/SecuredDummyWithPropertiesDependingOnThemselves.php diff --git a/features/authorization/deny.feature b/features/authorization/deny.feature index d6c4e540a20..0f45e84b73b 100644 --- a/features/authorization/deny.feature +++ b/features/authorization/deny.feature @@ -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" diff --git a/tests/Fixtures/TestBundle/Entity/SecuredDummyWithPropertiesDependingOnThemselves.php b/tests/Fixtures/TestBundle/Entity/SecuredDummyWithPropertiesDependingOnThemselves.php new file mode 100644 index 00000000000..6301b711c3b --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/SecuredDummyWithPropertiesDependingOnThemselves.php @@ -0,0 +1,81 @@ + + * + * 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 + */ +#[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; + } +}