@ExclusionPolicy("all") is not respected by the parent classes

[ruimarinho]

I'm using the agnostic storage pattern for a bundle and I have three User classes:

Vendor\Bundle\AuthenticationBundle\Model\User
|- Vendor\Bundle\AuthenticationBundle\Entity\User
  |- Project\AuthenticationBundle\Entity\User

When using the @ExclusionPolicy("all") on the Project\AuthenticationBundle\Entity\User one would expect all properties of the parents to be also excluded. However, this is not the case and requires me to add the same exclusion policy on each parent, otherwise all the parent properties are serialized. This is a problem because I don't want to create a dependency like this for the bundles.

The problem seems to lie in the way AnnotationDriver lists the class properties, since it doesn't take into account the parent class annotations, if set.

Is there a workaround for this?

[schmittjoh]

The exclusion policy is only applied to the class in which it is defined. This is the currently expected behavior, and I think it is also the easiest to understand. I might reconsider this if there are compelling reasons for not doing this though.

If you don't want to create a dependency on this bundle, you could instead use for example YAML as a metadata source.

[ruimarinho]

In my opinion, said dependency is compelling enough. We've recently started migrating YAML metadata sources to Annotations to allow us to use Traits that come with mapped properties too. This is useful for cases such as "drop in" support for Facebook profile data. I think this is a really nice addition and I expect it to be a more generic practice in the future.

Can't this behavior be an explicitly option set on the Annotation?

[schmittjoh]

How would you expose properties of the inherited class again?

I haven't worked with traits yet, could you elaborate how annotations are superior to YAML there?

[ruimarinho]

You'd have to override those properties, as you have to do now (and this works). Here's an example with the username property overridden with Traits:

// Vendor\Bundle\AuthenticationBundle\Entity\User.php
namespace Vendor\Bundle\AuthenticationBundle\Entity;

use Vendor\Bundle\AuthenticationBundle\Model\User as BaseUser;

/**
 * @ORM\MappedSuperclass()
 *
 * @ExclusionPolicy("all") --> this should be removed
 */
abstract class User extends BaseUser
{
     /**
     * @ORM\Column(type="string", length=255, unique=true, nullable=true)
     */
    protected $username;

    /**
     * @ORM\Column(type="string", length=255, nullable=true)
     */
    protected $password;
}

// Project\AuthenticationBundle\Entity\User;
namespace Project\AuthenticationBundle\Entity\User;

use Vendor\Bundle\AuthenticationBundle\Entity\User as BaseUser;
use Vendor\Bundle\SocialBundle\Facebook\Traits as Facebook;

/**
 * @ExclusionPolicy("all")
 */
class User extends BaseUser
{
   use Facebook\User

    /**
     * @ORM\Column(type="string", length=255, unique=false, nullable=true)
     *
     * @Expose()
     */
    protected $username;
}

The Facebook\User Trait includes the mapping for FB related data:

// Vendor\Bundle\SocialBundle\Facebook\Traits
trait User
{
    /**
     * @ORM\Column(name="fb_uid", type="string", length=255, unique=true, nullable=true)
     */
    protected $facebookUid;

While I understand it might be the easiest way to understand the @ExclusionPolicy, it can also be very dangerous, since without a similar declaration on parent class, all kind of sensitive user data would be serialized.

Regarding the Trait, this is a clear advantage as the ORM doesn't support loading YAML metadata files for traits. This drop in support for mapping will allow users to easily mix and organize different functionality driven classes for other interesting areas, such as persistence behaviors (timestampable, geolocatable, etc).

Also, right now the serializer can't expose properties from these traits. Is this related to this issue?

At a glance, I don't see why this feature should not be supported for Annotations, since they're so nice to work with.

[schmittjoh]

The way how you expose properties does not work for private properties. There would be no way to expose any of them. However, you can achieve what you want by using YAML metadata (only for the serializer, Doctrine is not affected by this) for the parent class (for the child class, you can still use annotations).

Regarding properties on traits, the serializer at the moment does not scan them, that's why they are not included there.

[ruimarinho]

I managed to do just that. I guess this is more towards the agnostic storage model pattern for bundles, specially due to the properties visibility problem you mentioned.

It took me a while to get the YAML metadata FileLocator to find the file I wanted, because the filename has a different convention than I expected. FileLocator.php looks for Entity.User.yml and not the class name only. Is this intentional?

Example:

ClassName: Vendor\Bundle\AuthenticationBundle\Entity\User
Prefix: Vendor\Bundle\AuthenticationBundle\
FileLocate looks for: (...)/Resources/config/serializer/Entity.User.yml

As a side effect, this does also not allow overriding the metadata from Vendor\Bundle\AuthenticationBundle under Project\AuthenticationBundle\Resources\config\serializer using the metadata.directories configuration option because there would be a filename collision. I think Doctrine handles does by writing the FQCN with dots, so:

// Project\AuthenticationBundle\Resources\config\serializer
Project.AuthenticationBundle.Entity.User.yml
Vendor.Bundle.AuthenticationBundle.Entity.User.yml

Would this work? What do you suggest?

Finally, regarding traits, should I open an issue for them?

[schmittjoh]

Yes, you can open an issue for traits. I'm not using PHP 5.4, so don't expect anything all too soon on this, but keeping track of this is always a good idea.

The FileLocator basically looks for the relative namespace (Doctrine does this as well btw, but supports both ways for BC reasons). You can overwrite the metadata location though (we have a recipe for this in the docs).

[ruimarinho]

Using the metadata override works for me. I would probably throw a Configuration error if the namespace_prefix is empty, since right now it generates a strpos(): Empty delimiter error. Better yet, by allowing null values, we could have the FQCN as the file name, if wanted.

By the way, KnpLabs just published a nice post about Traits with concrete examples and code (see issue #102 for Traits support tracking).

[timwhite]

I find this confusing as well. I override properties from the parent, and have a default policy of exclude all. I even tried an explicit exclusion on the overriden property, and it's still exposed.

For example, I'm using the FOSUserBundle, and my extended user object is exposing groups, even though I have an annotation telling it to exclude it. It also means that password hashes are exposed because the ExclusionPolicy only applies to the child class.

Ideally a modification to ExclusionPolicy that allows excluding including parent would be ideal

[johnpancoast]

@schmittjoh it seems people have brought up compelling reasons to add this feature.

1.) We don't always have control over extended classes.
2.) Even when we do have control over extended classes, it's time consuming to add an exclusion policy on N number of extended classes.
3.) It's a potential security issue! Let's say I'm serializing A and A extends N number of parents and I add the exclusion policy for all parents. If someone later comes and refactors the parents, there's easily a potential to miss the exclusions thus exposing potentially secure data when children are serialized.

Is this library still being maintained? Would you accept a PR?

[goetas]

@johnpancoast

Is this library still being maintained?

yes https://github.com/schmittjoh/serializer/graphs/contributors

Would you accept a PR?

yes, only if it solves properly the problem (how to achieve this is not yet clear), it is backward compatible and has proper tests.

Regarding the "how", I do not have any brilliant idea to do it. Some ideas:

• Applying @ExclusionPolicy("all") on the parent is a BC break.
• another solution might be @ExclusionPolicy("all", exludeFromParents={prop1, prop2,prop3}, includeFromParents={prop1, prop2,prop3})
• allow @Exclude and @Expose at class level with a parameter, as example: @Exclude("useranme")
• just use YAML or XML metadata definition
• other?

[mvrhov]

There was a talk about validation inheritance in symfony a few weeks ago. Amongs my suggestions the one below had most sense.
However this one won't be BC.
Applying it to serialization the behavior would be. If at least one is specified at the child level all needed have to be specified if none is specified than what parent defined is used.

[goetas]

@mvrhov can you make an example? did not get it

[mvrhov]

If at least one serialization setting is written for the child this would mean that you need to repeat all from a parent that you need. If you don't define any, then the ones from the parent are used.

[goetas]

repeat all from a parent that you need

Do you mean that you have to re-declare properties from parent classes?

[mvrhov]

Yes.

[goetas]

not the best option, with my proposal is not necessary

[johnpancoast]

Hi @goetas. Thanks for quick reply.

Applying @ExclusionPolicy("all") on the parent is a BC break.

Not sure what mean. Did you mean to make the exclusion apply to all inherited classes like people in this issue brought up? That would definitely be a BC break like you said, I just don't know if that's what you meant but is a major version possible or in the works?

another solution might be @ExclusionPolicy("all", exludeFromParents={prop1, prop2,prop3}, includeFromParents={prop1, prop2,prop3})

allow @exclude and @expose at class level with a parameter, as example: @exclude("useranme")

I like the include idea if it's possible to exclude all local and inherited properties.

The issue is that blacklisting values only goes as far as the values that you know at the time you made the policy. Someone can add a property later that the child isn't aware of so whitelisting is ideal IMO.

just use YAML or XML metadata definition

This works for the cases where you know what's being inherited and you know that won't change. That's not always the case.

Regarding the "how", I do not have any brilliant idea to do it...

Me neither! I'll try to dig in when I have some time. I just didn't see activity on this but I obviously wasn't looking in the right places. Thanks!

[jcornide]

Hi @goetas , I have exactly the same problem with the FOS User bundle as described by @timwhite , I'm not sure what you mean by

just use YAML or XML metadata definition

I use YAML instead of annotations but I still can not exclude parent properties, here's my yaml for
user:

  properties:
    username:
      exclude: true
    usernameCannonical:
      exclude: true
    password:
      exclude: true
    emailCanonical:
      exclude: true
    skills:
      exclude: true
    languages:
      exclude: true
    subordinates:
      exclude: true

skills, subordinates and languages get properly excluded but not password of emailCanonical.

I added those properties to my User entity, but even that way they're still exposed

[Preclowski]

I can just confirm its really painful issue. Even if I add exclusion_policy: ALL and exclude: true in yml mapping for inherited property, serializer don't respect those.

[goetas]

im not familiar with fos user bundle. if somebody is able to provide a failing test case, will do my best to fix it.

[Preclowski]

In my case its just simple inherited class (InvoiceAddress extends Address)

[goetas]

All the cases explained here are more or less simple... but all of them are different and failing maybe for small differences... if you can provide a minimal failing example, possibly as PR, would be great

[Preclowski]

It would be much easier for me to give you example here.

<?php

class Address
{
    private $firstName;
    private $lastName;
}

<?php

class InvoiceAddress extends Address
{
    private $taxNumber;
}

yml mapping

InvoiceAddress:
  exclusion_policy: ALL
  properties:
    firstName:
      exclude: true
    lastName:
      exclude: true
    taxNumber:
      expose: true

Result of serialization would be like

{
  first_name: John,
  last_name: Smith,
  tax_number: 123456
}

but expected behavior was to only show taxNumber.

[mvrhov]

Wrong expectation. The mapping is inherited. AFAIK this is documented. The other thing that's also documented is that you can only map the properties that are in a specific class. You cannot map the properties of a parent in a child.
You should use groups for what you want to achieve.

[Preclowski]

IMO it should inherit default exclusions. Using serialization groups is a great feature, but inheritance should be optional.

And actually mapping isnt inherited (or I miss something). If I create mapping for inherited class (Address in this case) it wont be respected.

[mvrhov]

Everything you write in a xxx.yml or xxx.xml is per class and it applies only on properties of that specific class.
When you extend the class and do a serialization the serializer will look at the configuration of each aclass and apply only to that specific class.

[goetas]

@Preclowski you need Address.yml

Address:
  properties:
    firstName:
      exclude: true
    lastName:
      exclude: true

As @mvrhov said, metadata are per class.

[goetas]

I went a bit deeper in the issue.
This can be defined as unexpected behaviour and probably should be documented a bit better, but as @mvrhov said:

Everything you write in a xxx.yml or xxx.xml is per class and it applies only on properties of that specific class.

This meas that if you want to influence the mapping for class, you have to define mapping information explicitly for that class (in YAML or XML).

[yapro]

        $serializer = SerializerBuilder::create()
            ->setSerializationContextFactory(function () {
                return \JMS\Serializer\SerializationContext::create()
                    ->setSerializeNull(true)
                    ;
            })
            ->setDeserializationContextFactory(function () {
                return \JMS\Serializer\DeserializationContext::create()
                    ->setSerializeNull(true)
                    ;
            })
            ->setAnnotationReader($annotationReader)
            ->setPropertyNamingStrategy(new SerializedNameImportantThanPropertyNameStrategy())
            ->build();

Ignoring some parent classes: CComponent, CModel, CActiveRecord

        $class = new \ReflectionClass($serializer);
        $property = $class->getProperty('navigator');
        $property->setAccessible(true);
        $navigator = $property->getValue($serializer);

        $class = new \ReflectionClass($navigator);
        $property = $class->getProperty('metadataFactory');
        $property->setAccessible(true);
        $metadataFactory = $property->getValue($navigator);

        $class = new \ReflectionClass($metadataFactory);
        $property = $class->getProperty('loadedClassMetadata');
        $property->setAccessible(true);
        $property->setValue($metadataFactory, [
            'CComponent' => new NullMetadata(new \stdClass()),
            'CModel' => new NullMetadata(new \stdClass()),
            'CActiveRecord' => new NullMetadata(new \stdClass()),
        ]);

[goetas]

Better to use YAML or XML metdata for the CComponent CModel CActiveRecord classes intead of doing this with the builder

[yapro]

@goetas can you provide a link to the documentation page or to an example page?

[goetas]

https://stackoverflow.com/questions/12960141/jmsserializerbundle-no-control-over-third-party-meta-data