feat: add security_post_denormalize in ApiProperty

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)

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"

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:

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();

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,
             ]);
         }
 

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,

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;

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;