Security Voter not aware of role hierarchy in Symfony Best Practises

[richard-keller]

The section on Security suggests using Security Voters to implement complex security logic. The provided code example makes use of the getRoles() function to determine whether the user has the given role. However, it should be noted that this won't work if role hierarchies are used, since the User entity is not aware of the full role hierarchy.

Code snippet in question:

protected function isGranted($attribute, $post, $user = null)
{
    if ($attribute == self::CREATE && in_array(ROLE_ADMIN, $user->getRoles())) {
      return true;
    }

    return false;
}

I imagine that the best way to handle this would be to use something like $securityContext->isGranted('ROLE_ADMIN') but then we should be injecting security.context, and not the User.

Edit: It would appear that using security.context isn't a great idea, since it works using the token for the currently logged-in user. Regardless, I think the documentation should note that the solution provided will not work for hierarchical role structures.

[richard-keller]

So it turns out that injecting security.context so that we can use isGranted() will not work, since it results in a circular dependency.

Update: the following does not work. $container->getParameter('security.role_hierarchy.roles') returns all the roles, not just the ones assigned to the user.

The following is a working solution

Inject the Service Container into the voter via the constructor:

services:
    call_voter:
        class:  Acme\FooBundle\Security\CustomVoter
        public: false
        arguments: ['@service_container']
        tags:
            - { name: security.voter }

The the voter class would look something like this:

class CustomVoter extends AbstractVoter
{
    const DELETE = 'delete'; 
    private $roles;

    function __construct(ContainerInterface $container)
    {
        $this->roles = array_keys($container->getParameter('security.role_hierarchy.roles'));
    }

    protected function isGranted($attribute, $object, $user = null)
    {
        if (!$user instanceof UserInterface)
        {
            return false;
        }

        if ($attribute == self::DELETE)
        {
            if (in_array('ROLE_USER', $this->roles))
                return true;
        }

        return false;
    }
}

[xabbuh]

@richard-keller Your code always returns true when ROLE_USER is in the role hierarchy.

[jdachtera]

First inject @security.role_hierarchy into the voter. Then use this code:

protected function hasRole($roleName, TokenInterface $token)
{
    foreach ($this->roleHierarchy->getReachableRoles($token->getRoles()) as $role) {
        if ($roleName === $role->getRole()) {
            return TRUE;
        }
    }
    return FALSE;
}

[wouterj]

If I understand things correctly, this can't be fixed at the moment. @javiereguiluz can you please confirm?

[javiereguiluz]

@wouterj sadly this problem cannot be solved yet. There is a pending PR symfony/symfony#14048 which unfortunately didn't get much attention.

[weaverryan]

See symfony/symfony#14048 (comment) - this is now actionable in 2.8 and we really should have a cookbook article or section about this.

[weaverryan]

this is now fixable in Symfony 2.8: see #5908 for the example. Someone needs to update best_practices/security.rst to close this issue and #5728

[weaverryan]

Fixed!