Skip to content

Problem with new ApiProperty security #1795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
VincentDugard opened this issue Feb 5, 2021 · 5 comments
Open

Problem with new ApiProperty security #1795

VincentDugard opened this issue Feb 5, 2021 · 5 comments

Comments

@VincentDugard
Copy link

VincentDugard commented Feb 5, 2021

API Platform version(s) affected: 2.6.1

Description

I have this property in my User entity

image

The entity User have a manager that can access this property.
This code works for GET request, however for PUT or PATCH request I get this error
Unable to get a property on a non-object.

I don't see why this wouldn't work so I think it's a bug

edit : I did more research and it looks like "object" value is "App\Entity\User" and not the User object

How to reproduce
Access a property of "object" in @ApiProperty security feature (see code above).

@mbrodala
Copy link

I can confirm this issue. So technically the example given in the docs can actually not work:

/**
 * Secured resource.
 *
 * @ApiResource(
 *     attributes={"security"="is_granted('ROLE_USER')"},
 *     collectionOperations={
 *         "get",
 *         "post"={"security"="is_granted('ROLE_ADMIN')"}
 *     },
 *     itemOperations={
 *         "get",
 *         "put"={"security"="is_granted('ROLE_ADMIN') or object.owner == user"},
 *     }
 * )
 * @ORM\Entity
 */

Here object can be an instance of Book or just the FQCN of Book.

We noticed this issue in our API tests where the processing of the request seems rather different and thus causes denormalization compared to normalization.

@yobud
Copy link

yobud commented May 4, 2021

Also, shouldn't we be able to specify different security for reading and writing?

I mean, when you make a PUT, returned result might contain others fields (readonly for your security context) than the ones you sent.

@dennobaby
Copy link

Also, on a collection, in my case, object is a doctrine-paginator instance. Thats very unexpected, i think.

@mbrodala
Copy link

mbrodala commented Feb 21, 2022

A possible hotfix would be this:

--- src/Serializer/AbstractItemNormalizer.php	2022-02-21 09:58:43.996978817 +0100
+++ src/Serializer/AbstractItemNormalizer.php	2022-02-21 09:57:44.872673911 +0100
@@ -372,6 +372,7 @@
     {
         $options = $this->getFactoryOptions($context);
         $propertyNames = $this->propertyNameCollectionFactory->create($context['resource_class'], $options);
+        $classOrObject = $context[self::OBJECT_TO_POPULATE] ?? $classOrObject;
 
         $allowedAttributes = [];
         foreach ($propertyNames as $propertyName) {

See https://github.com/api-platform/core/blob/119b05b631aae94a922838c6c3ba2f64618210a5/src/Serializer/AbstractItemNormalizer.php#L416

The object is actually there but not forwarded here. Generally it's rather obscure why Symfony Serializer itself already supports an object or FQCN here.

@mbrodala
Copy link

Also see api-platform/docs#1351, api-platform/docs#1349 and api-platform/core#4184 which are related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants