diff --git a/CHANGELOG.md b/CHANGELOG.md index b81907cea9b..ef9fddde27b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## 2.7.0 +* 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) +* 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) * JSON Schema: Add support for generating property schema with numeric constraint restrictions (#4225) * JSON Schema: Add support for generating property schema with Collection restriction (#4182) * JSON Schema: Add support for generating property schema format for Url and Hostname (#4185) diff --git a/features/authorization/deny.feature b/features/authorization/deny.feature index 54d63181f1f..d6c4e540a20 100644 --- a/features/authorization/deny.feature +++ b/features/authorization/deny.feature @@ -78,6 +78,22 @@ Feature: Authorization checking And I send a "GET" request to "/secured_dummies/2" Then the response status code should be 200 + Scenario: A user can see a secured owner-only property on an object they own + 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 ZHVuZ2xhczprZXZpbg==" + And I send a "GET" request to "/secured_dummies/2" + Then the response status code should be 200 + And the JSON node "ownerOnlyProperty" should exist + And the JSON node "ownerOnlyProperty" should not be null + + 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" + And I send a "GET" request to "/secured_dummies" + Then the response status code should be 200 + And the response should not contain "ownerOnlyProperty" + Scenario: A user can't assign to themself an item they doesn't own When I add "Accept" header equal to "application/ld+json" And I add "Content-Type" header equal to "application/ld+json" @@ -151,3 +167,17 @@ Feature: Authorization checking Then the response status code should be 200 And the response should contain "adminOnlyProperty" And the JSON node "hydra:member[2].adminOnlyProperty" should be equal to the string "Is it safe?" + + Scenario: An user can update an owner-only secured property on an object they own + 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 ZHVuZ2xhczprZXZpbg==" + And I send a "PUT" request to "/secured_dummies/3" with body: + """ + { + "ownerOnlyProperty": "updated" + } + """ + Then the response status code should be 200 + And the response should contain "ownerOnlyProperty" + And the JSON node "ownerOnlyProperty" should be equal to the string "updated" diff --git a/features/graphql/authorization.feature b/features/graphql/authorization.feature index 3844387d884..a9c16a7131b 100644 --- a/features/graphql/authorization.feature +++ b/features/graphql/authorization.feature @@ -349,6 +349,69 @@ Feature: Authorization checking And the header "Content-Type" should be equal to "application/json" And the JSON node "data.createSecuredDummy.securedDummy.owner" should be equal to "dunglas" + Scenario: An admin can create a secured resource with an owner-only property if they will be the owner + When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu" + And I send the following GraphQL request: + """ + mutation { + createSecuredDummy(input: {owner: "admin", title: "Hi", description: "Desc", adminOnlyProperty: "secret", ownerOnlyProperty: "it works"}) { + securedDummy { + ownerOnlyProperty + } + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/json" + And the JSON node "data.createSecuredDummy.securedDummy.ownerOnlyProperty" should be equal to the string "it works" + And I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu" + And I send the following GraphQL request: + """ + { + securedDummies { + edges { + node { + ownerOnlyProperty + } + } + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the JSON node "data.securedDummies.edges[2].node.ownerOnlyProperty" should be equal to "it works" + + Scenario: An admin can't create a secured resource with an owner-only property if they won't be the owner + When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu" + And I send the following GraphQL request: + """ + mutation { + createSecuredDummy(input: {owner: "dunglas", title: "Hi", description: "Desc", adminOnlyProperty: "secret", ownerOnlyProperty: "should not be set"}) { + securedDummy { + ownerOnlyProperty + } + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/json" + And the JSON node "data.createSecuredDummy.securedDummy.ownerOnlyProperty" should exist + And the JSON node "data.createSecuredDummy.securedDummy.ownerOnlyProperty" should be null + And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" + And I send the following GraphQL request: + """ + { + securedDummy(id: "/secured_dummies/4") { + ownerOnlyProperty + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the JSON node "data.securedDummy.ownerOnlyProperty" should be equal to the string "" + Scenario: A user cannot retrieve an item they doesn't own When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" And I send the following GraphQL request: @@ -419,6 +482,66 @@ Feature: Authorization checking And the header "Content-Type" should be equal to "application/json" And the JSON node "data.securedDummy.adminOnlyProperty" should be null + Scenario: A user can see a secured owner-only property on an object they own + When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" + And I send the following GraphQL request: + """ + { + securedDummy(id: "/secured_dummies/2") { + ownerOnlyProperty + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/json" + And the JSON node "data.securedDummy.ownerOnlyProperty" should exist + And the JSON node "data.securedDummy.ownerOnlyProperty" should not be null + + Scenario: A user can update a secured owner-only property on an object they own + When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" + And I send the following GraphQL request: + """ + mutation { + updateSecuredDummy(input: {id: "/secured_dummies/2", ownerOnlyProperty: "updated"}) { + securedDummy { + ownerOnlyProperty + } + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/json" + And the JSON node "data.updateSecuredDummy.securedDummy.ownerOnlyProperty" should be equal to the string "updated" + And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg==" + And I send the following GraphQL request: + """ + { + securedDummy(id: "/secured_dummies/2") { + ownerOnlyProperty + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the JSON node "data.securedDummy.ownerOnlyProperty" should be equal to the string "updated" + + Scenario: An admin can't see a secured owner-only property on an object they don't own + When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu" + And I send the following GraphQL request: + """ + { + securedDummy(id: "/secured_dummies/2") { + ownerOnlyProperty + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/json" + And the JSON node "data.securedDummy.ownerOnlyProperty" should be null + Scenario: A user can't assign to themself an item they doesn't own When I add "Authorization" header equal to "Basic YWRtaW46a2l0dGVu" And I send the following GraphQL request: diff --git a/src/Annotation/ApiProperty.php b/src/Annotation/ApiProperty.php index 3140eae218f..a61629624f4 100644 --- a/src/Annotation/ApiProperty.php +++ b/src/Annotation/ApiProperty.php @@ -30,6 +30,7 @@ * @Attribute("jsonldContext", type="array"), * @Attribute("push", type="bool"), * @Attribute("security", type="string"), + * @Attribute("securityPostDenormalize", type="string"), * @Attribute("swaggerContext", type="array") * ) */ @@ -112,6 +113,7 @@ final class ApiProperty * @param bool $push * @param string $security * @param array $swaggerContext + * @param string $securityPostDenormalize * * @throws InvalidArgumentException */ @@ -136,7 +138,8 @@ public function __construct( ?array $openapiContext = null, ?bool $push = null, ?string $security = null, - ?array $swaggerContext = null + ?array $swaggerContext = null, + ?string $securityPostDenormalize = null ) { if (!\is_array($description)) { // @phpstan-ignore-line Doctrine annotations support [$publicProperties, $configurableAttributes] = self::getConfigMetadata(); diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 8ab7b0e116a..be441f9ca53 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -254,7 +254,22 @@ public function denormalize($data, $class, $format = null, array $context = []) return $item; } - return parent::denormalize($data, $resourceClass, $format, $context); + $previousObject = null !== $objectToPopulate ? clone $objectToPopulate : null; + $object = parent::denormalize($data, $resourceClass, $format, $context); + + // 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)) { + if (null !== $previousObject) { + $this->setValue($object, $attribute, $this->propertyAccessor->getValue($previousObject, $attribute)); + } else { + $propertyMetaData = $this->propertyMetadataFactory->create($resourceClass, $attribute, $this->getFactoryOptions($context)); + $this->setValue($object, $attribute, $propertyMetaData->getDefault()); + } + } + } + + return $object; } /** @@ -390,12 +405,43 @@ protected function isAllowedAttribute($classOrObject, $attribute, $format = null return false; } + return $this->canAccessAttribute(\is_object($classOrObject) ? $classOrObject : null, $attribute, $context); + } + + /** + * Check if access to the attribute is granted. + * + * @param object $object + */ + protected function canAccessAttribute($object, string $attribute, array $context = []): bool + { $options = $this->getFactoryOptions($context); $propertyMetadata = $this->propertyMetadataFactory->create($context['resource_class'], $attribute, $options); $security = $propertyMetadata->getAttribute('security'); if ($this->resourceAccessChecker && $security) { return $this->resourceAccessChecker->isGranted($context['resource_class'], $security, [ - 'object' => $classOrObject, + 'object' => $object, + ]); + } + + return true; + } + + /** + * Check if access to the attribute is granted. + * + * @param object $object + * @param object|null $previousObject + */ + protected function canAccessAttributePostDenormalize($object, $previousObject, string $attribute, array $context = []): bool + { + $options = $this->getFactoryOptions($context); + $propertyMetadata = $this->propertyMetadataFactory->create($context['resource_class'], $attribute, $options); + $security = $propertyMetadata->getAttribute('security_post_denormalize'); + if ($this->resourceAccessChecker && $security) { + return $this->resourceAccessChecker->isGranted($context['resource_class'], $security, [ + 'object' => $object, + 'previous_object' => $previousObject, ]); } diff --git a/tests/Annotation/ApiPropertyTest.php b/tests/Annotation/ApiPropertyTest.php index f4c1ec790c0..9946e49ed73 100644 --- a/tests/Annotation/ApiPropertyTest.php +++ b/tests/Annotation/ApiPropertyTest.php @@ -53,6 +53,7 @@ public function testConstruct() 'fetchEager' => false, 'jsonldContext' => ['foo' => 'bar'], 'security' => 'is_granted(\'ROLE_ADMIN\')', + 'securityPostDenormalize' => 'is_granted(\'VIEW\', object)', 'swaggerContext' => ['foo' => 'baz'], 'openapiContext' => ['foo' => 'baz'], 'push' => true, @@ -64,6 +65,7 @@ public function testConstruct() 'fetch_eager' => false, 'jsonld_context' => ['foo' => 'bar'], 'security' => 'is_granted(\'ROLE_ADMIN\')', + 'security_post_denormalize' => 'is_granted(\'VIEW\', object)', 'swagger_context' => ['foo' => 'baz'], 'openapi_context' => ['foo' => 'baz'], 'push' => true, @@ -83,6 +85,7 @@ public function testConstructAttribute() fetchEager: false, jsonldContext: ['foo' => 'bar'], security: 'is_granted(\'ROLE_ADMIN\')', + securityPostDenormalize: 'is_granted(\'VIEW\', object)', swaggerContext: ['foo' => 'baz'], openapiContext: ['foo' => 'baz'], push: true, @@ -97,6 +100,7 @@ public function testConstructAttribute() 'fetch_eager' => false, 'jsonld_context' => ['foo' => 'bar'], 'security' => 'is_granted(\'ROLE_ADMIN\')', + 'security_post_denormalize' => 'is_granted(\'VIEW\', object)', 'swagger_context' => ['foo' => 'baz'], 'openapi_context' => ['foo' => 'baz'], 'push' => true, diff --git a/tests/Fixtures/TestBundle/Document/SecuredDummy.php b/tests/Fixtures/TestBundle/Document/SecuredDummy.php index 277b1220066..1356529c0ec 100644 --- a/tests/Fixtures/TestBundle/Document/SecuredDummy.php +++ b/tests/Fixtures/TestBundle/Document/SecuredDummy.php @@ -83,6 +83,17 @@ class SecuredDummy */ private $adminOnlyProperty = ''; + /** + * @var string Secret property, only readable/writable by owners + * + * @ODM\Field + * @ApiProperty( + * security="object == null or object.getOwner() == user", + * securityPostDenormalize="object.getOwner() == user", + * ) + */ + private $ownerOnlyProperty = ''; + /** * @var string The owner * @@ -187,6 +198,16 @@ public function setAdminOnlyProperty(?string $adminOnlyProperty) $this->adminOnlyProperty = $adminOnlyProperty; } + public function getOwnerOnlyProperty(): ?string + { + return $this->ownerOnlyProperty; + } + + public function setOwnerOnlyProperty(?string $ownerOnlyProperty) + { + $this->ownerOnlyProperty = $ownerOnlyProperty; + } + public function getOwner(): string { return $this->owner; diff --git a/tests/Fixtures/TestBundle/Entity/SecuredDummy.php b/tests/Fixtures/TestBundle/Entity/SecuredDummy.php index 3a35ccf6ada..3af569999dc 100644 --- a/tests/Fixtures/TestBundle/Entity/SecuredDummy.php +++ b/tests/Fixtures/TestBundle/Entity/SecuredDummy.php @@ -84,6 +84,17 @@ class SecuredDummy */ private $adminOnlyProperty = ''; + /** + * @var string Secret property, only readable/writable by owners + * + * @ORM\Column + * @ApiProperty( + * security="object == null or object.getOwner() == user", + * securityPostDenormalize="object.getOwner() == user", + * ) + */ + private $ownerOnlyProperty = ''; + /** * @var string The owner * @@ -198,6 +209,16 @@ public function setAdminOnlyProperty(?string $adminOnlyProperty) $this->adminOnlyProperty = $adminOnlyProperty; } + public function getOwnerOnlyProperty(): ?string + { + return $this->ownerOnlyProperty; + } + + public function setOwnerOnlyProperty(?string $ownerOnlyProperty) + { + $this->ownerOnlyProperty = $ownerOnlyProperty; + } + public function getOwner(): string { return $this->owner; diff --git a/tests/Serializer/AbstractItemNormalizerTest.php b/tests/Serializer/AbstractItemNormalizerTest.php index 09592c80316..9d259dd4808 100644 --- a/tests/Serializer/AbstractItemNormalizerTest.php +++ b/tests/Serializer/AbstractItemNormalizerTest.php @@ -266,6 +266,310 @@ public function testNormalizeWithSecuredProperty() ])); } + public function testDenormalizeWithSecuredProperty() + { + $data = [ + 'title' => 'foo', + 'adminOnlyProperty' => 'secret', + ]; + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(SecuredDummy::class, [])->willReturn(new PropertyNameCollection(['title', 'adminOnlyProperty'])); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'title', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', false, true)); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'adminOnlyProperty', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', true, true, null, null, null, null, null, null, ['security' => 'is_granted(\'ROLE_ADMIN\')'])); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + + $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass(null, SecuredDummy::class)->willReturn(SecuredDummy::class); + + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->willImplement(NormalizerInterface::class); + + $resourceAccessChecker = $this->prophesize(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->isGranted( + SecuredDummy::class, + 'is_granted(\'ROLE_ADMIN\')', + ['object' => null] + )->willReturn(false); + + $normalizer = $this->getMockForAbstractClass(AbstractItemNormalizer::class, [ + $propertyNameCollectionFactoryProphecy->reveal(), + $propertyMetadataFactoryProphecy->reveal(), + $iriConverterProphecy->reveal(), + $resourceClassResolverProphecy->reveal(), + $propertyAccessorProphecy->reveal(), + null, + null, + null, + false, + [], + [], + null, + $resourceAccessChecker->reveal(), + ]); + $normalizer->setSerializer($serializerProphecy->reveal()); + + $actual = $normalizer->denormalize($data, SecuredDummy::class); + + $this->assertInstanceOf(SecuredDummy::class, $actual); + + $propertyAccessorProphecy->setValue($actual, 'title', 'foo')->shouldHaveBeenCalled(); + $propertyAccessorProphecy->setValue($actual, 'adminOnlyProperty', 'secret')->shouldNotHaveBeenCalled(); + } + + public function testDenormalizeCreateWithDeniedPostDenormalizeSecuredProperty() + { + $data = [ + 'title' => 'foo', + 'ownerOnlyProperty' => 'should not be set', + ]; + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(SecuredDummy::class, [])->willReturn(new PropertyNameCollection(['title', 'ownerOnlyProperty'])); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'title', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', false, true)); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'ownerOnlyProperty', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', true, true, null, null, null, null, null, null, ['security_post_denormalize' => 'false'], null, null, '')); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + + $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass(null, SecuredDummy::class)->willReturn(SecuredDummy::class); + + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->willImplement(NormalizerInterface::class); + + $resourceAccessChecker = $this->prophesize(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->isGranted( + SecuredDummy::class, + 'false', + Argument::any() + )->willReturn(false); + + $normalizer = $this->getMockForAbstractClass(AbstractItemNormalizer::class, [ + $propertyNameCollectionFactoryProphecy->reveal(), + $propertyMetadataFactoryProphecy->reveal(), + $iriConverterProphecy->reveal(), + $resourceClassResolverProphecy->reveal(), + $propertyAccessorProphecy->reveal(), + null, + null, + null, + false, + [], + [], + null, + $resourceAccessChecker->reveal(), + ]); + $normalizer->setSerializer($serializerProphecy->reveal()); + + $actual = $normalizer->denormalize($data, SecuredDummy::class); + + $this->assertInstanceOf(SecuredDummy::class, $actual); + + $propertyAccessorProphecy->setValue($actual, 'title', 'foo')->shouldHaveBeenCalled(); + $propertyAccessorProphecy->setValue($actual, 'ownerOnlyProperty', 'should not be set')->shouldHaveBeenCalled(); + $propertyAccessorProphecy->setValue($actual, 'ownerOnlyProperty', '')->shouldHaveBeenCalled(); + } + + public function testDenormalizeUpdateWithSecuredProperty() + { + $dummy = new SecuredDummy(); + + $data = [ + 'title' => 'foo', + 'ownerOnlyProperty' => 'secret', + ]; + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(SecuredDummy::class, [])->willReturn(new PropertyNameCollection(['title', 'ownerOnlyProperty'])); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'title', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', false, true)); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'ownerOnlyProperty', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', true, true, null, null, null, null, null, null, ['security' => 'true'])); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + + $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass($dummy, SecuredDummy::class)->willReturn(SecuredDummy::class); + + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->willImplement(NormalizerInterface::class); + + $resourceAccessChecker = $this->prophesize(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->isGranted( + SecuredDummy::class, + 'true', + ['object' => null] + )->willReturn(true); + $resourceAccessChecker->isGranted( + SecuredDummy::class, + 'true', + ['object' => $dummy] + )->willReturn(true); + + $normalizer = $this->getMockForAbstractClass(AbstractItemNormalizer::class, [ + $propertyNameCollectionFactoryProphecy->reveal(), + $propertyMetadataFactoryProphecy->reveal(), + $iriConverterProphecy->reveal(), + $resourceClassResolverProphecy->reveal(), + $propertyAccessorProphecy->reveal(), + null, + null, + null, + false, + [], + [], + null, + $resourceAccessChecker->reveal(), + ]); + $normalizer->setSerializer($serializerProphecy->reveal()); + + $context = [AbstractItemNormalizer::OBJECT_TO_POPULATE => $dummy]; + $actual = $normalizer->denormalize($data, SecuredDummy::class, null, $context); + + $this->assertInstanceOf(SecuredDummy::class, $actual); + + $propertyAccessorProphecy->setValue($actual, 'title', 'foo')->shouldHaveBeenCalled(); + $propertyAccessorProphecy->setValue($actual, 'ownerOnlyProperty', 'secret')->shouldHaveBeenCalled(); + } + + public function testDenormalizeUpdateWithDeniedSecuredProperty() + { + $dummy = new SecuredDummy(); + $dummy->setOwnerOnlyProperty('secret'); + + $data = [ + 'title' => 'foo', + 'ownerOnlyProperty' => 'should not be set', + ]; + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(SecuredDummy::class, [])->willReturn(new PropertyNameCollection(['title', 'ownerOnlyProperty'])); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'title', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', false, true)); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'ownerOnlyProperty', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', true, true, null, null, null, null, null, null, ['security' => 'false'])); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + + $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass($dummy, SecuredDummy::class)->willReturn(SecuredDummy::class); + + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->willImplement(NormalizerInterface::class); + + $resourceAccessChecker = $this->prophesize(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->isGranted( + SecuredDummy::class, + 'false', + ['object' => null] + )->willReturn(false); + $resourceAccessChecker->isGranted( + SecuredDummy::class, + 'false', + ['object' => $dummy] + )->willReturn(false); + + $normalizer = $this->getMockForAbstractClass(AbstractItemNormalizer::class, [ + $propertyNameCollectionFactoryProphecy->reveal(), + $propertyMetadataFactoryProphecy->reveal(), + $iriConverterProphecy->reveal(), + $resourceClassResolverProphecy->reveal(), + $propertyAccessorProphecy->reveal(), + null, + null, + null, + false, + [], + [], + null, + $resourceAccessChecker->reveal(), + ]); + $normalizer->setSerializer($serializerProphecy->reveal()); + + $context = [AbstractItemNormalizer::OBJECT_TO_POPULATE => $dummy]; + $actual = $normalizer->denormalize($data, SecuredDummy::class, null, $context); + + $this->assertInstanceOf(SecuredDummy::class, $actual); + + $propertyAccessorProphecy->setValue($actual, 'title', 'foo')->shouldHaveBeenCalled(); + $propertyAccessorProphecy->setValue($actual, 'ownerOnlyProperty', 'should not be set')->shouldNotHaveBeenCalled(); + } + + public function testDenormalizeUpdateWithDeniedPostDenormalizeSecuredProperty() + { + $dummy = new SecuredDummy(); + $dummy->setOwnerOnlyProperty('secret'); + + $data = [ + 'title' => 'foo', + 'ownerOnlyProperty' => 'should not be set', + ]; + + $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); + $propertyNameCollectionFactoryProphecy->create(SecuredDummy::class, [])->willReturn(new PropertyNameCollection(['title', 'ownerOnlyProperty'])); + + $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'title', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', false, true)); + $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'ownerOnlyProperty', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', true, true, null, null, null, null, null, null, ['security_post_denormalize' => 'false'])); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + + $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); + $propertyAccessorProphecy->getValue($dummy, 'ownerOnlyProperty')->willReturn('secret'); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass($dummy, SecuredDummy::class)->willReturn(SecuredDummy::class); + + $serializerProphecy = $this->prophesize(SerializerInterface::class); + $serializerProphecy->willImplement(NormalizerInterface::class); + + $resourceAccessChecker = $this->prophesize(ResourceAccessCheckerInterface::class); + $resourceAccessChecker->isGranted( + SecuredDummy::class, + 'false', + ['object' => $dummy, 'previous_object' => $dummy] + )->willReturn(false); + + $normalizer = $this->getMockForAbstractClass(AbstractItemNormalizer::class, [ + $propertyNameCollectionFactoryProphecy->reveal(), + $propertyMetadataFactoryProphecy->reveal(), + $iriConverterProphecy->reveal(), + $resourceClassResolverProphecy->reveal(), + $propertyAccessorProphecy->reveal(), + null, + null, + null, + false, + [], + [], + null, + $resourceAccessChecker->reveal(), + ]); + $normalizer->setSerializer($serializerProphecy->reveal()); + + $context = [AbstractItemNormalizer::OBJECT_TO_POPULATE => $dummy]; + $actual = $normalizer->denormalize($data, SecuredDummy::class, null, $context); + + $this->assertInstanceOf(SecuredDummy::class, $actual); + + $propertyAccessorProphecy->setValue($actual, 'title', 'foo')->shouldHaveBeenCalled(); + $propertyAccessorProphecy->setValue($actual, 'ownerOnlyProperty', 'should not be set')->shouldHaveBeenCalled(); + $propertyAccessorProphecy->setValue($actual, 'ownerOnlyProperty', 'secret')->shouldHaveBeenCalled(); + } + public function testNormalizeReadableLinks() { $relatedDummy = new RelatedDummy(); @@ -787,6 +1091,8 @@ public function testDenormalizeBadKeyType() $propertyNameCollectionFactoryProphecy->create(Dummy::class, [])->willReturn(new PropertyNameCollection(['relatedDummies'])); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING), '', false, true)); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummy', [])->willReturn(new PropertyMetadata(new Type(Type::BUILTIN_TYPE_OBJECT, false, RelatedDummy::class), '', false, true, false, false)); $propertyMetadataFactoryProphecy->create(Dummy::class, 'relatedDummies', [])->willReturn( new PropertyMetadata( new Type( diff --git a/tests/Serializer/ItemNormalizerTest.php b/tests/Serializer/ItemNormalizerTest.php index 7a097aae786..9a63baea547 100644 --- a/tests/Serializer/ItemNormalizerTest.php +++ b/tests/Serializer/ItemNormalizerTest.php @@ -164,6 +164,7 @@ public function testDenormalizeWithIri() $propertyMetadata = new PropertyMetadata(null, null, true, true); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); + $propertyMetadataFactoryProphecy->create(Dummy::class, 'id', [])->willReturn($propertyMetadata)->shouldBeCalled(); $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', [])->willReturn($propertyMetadata)->shouldBeCalled(); $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);