Just checking: abstain vote from Symfony AbstractVoter isGranted

Question

I'm working on a Symfony2 application in which we want to introduce Security Voters. The DX initiative (jay!) brought us the simpler version of this mechanism so I'd like to use that. The only thing that's bothering me a bit about the use of \Symfony\Component\Security\Core\Authorization\Voter\AbstractVoter is the fact that I can no longer abstain a vote when just implementing the abstract isGranted method. I'd like to change that for our application but would really appreciate your opinion on the matter from a security point of view.

To be clear, in our application we will be using the unanimous access decision manager strategy - in short: you need at least one ACCESS_GRANTED and no ACCESS_DENIED - because we might later want to introduce voters for banning IP addresses or other shenanigans.

As shown in the relevant piece of code below, as soon as an implementation of the AbstractVoter supports an attribute, the vote will default to Denied.

public function vote(TokenInterface $token, $object, array $attributes)
{
    if (!$object || !$this->supportsClass(get_class($object))) {
        return self::ACCESS_ABSTAIN;
    }

    // abstain vote by default in case none of the attributes are supported
    $vote = self::ACCESS_ABSTAIN;

    foreach ($attributes as $attribute) {
        if (!$this->supportsAttribute($attribute)) {
            continue;
        }

        // as soon as at least one attribute is supported, default is to deny access
        $vote = self::ACCESS_DENIED;

        if ($this->isGranted($attribute, $object, $token->getUser())) {
            // grant access as soon as at least one voter returns a positive response
            return self::ACCESS_GRANTED;
        }
    }

    return $vote;
}

What I'd like to do is override that with the code below

public function vote(TokenInterface $token, $object, array $attributes)
{
    if (!$object || !$this->supportsClass(get_class($object))) {
        return self::ACCESS_ABSTAIN;
    }

    $vote = self::ACCESS_ABSTAIN;

    foreach ($attributes as $attribute) {
        if (!$this->supportsAttribute($attribute)) {
            continue;
        }

        // This is where we differ from SymfonyAbstractVoter. Only if there is an outspoken yes or no, a vote is cast
        // When returning null, it will still abstain
        $grant = $this->isGranted($attribute, $object, $token->getUser());
        if ($grant === true) {
            return self::ACCESS_GRANTED;
        }
        if($grant === false) {
            return self::ACCESS_DENIED;
        }

    }

    return $vote;
}

What do you think? Is it safe to return ACCESS_ABSTAIN when null is returned form isGranted and only cast a ACCESS_GRANTED or ACCESS_DENIED if a voter returns true or false?

---

Why would I even want to do this? To still make use of the AbstractVoter (because, yes I'm a lazy developer) but separate my concerns.

Let's say I make one voter for a post, PostVoter, that will allow me to edit my own posts. It will then cast a ACCESS_GRANTED. But as long as I don't own the post, this voter really doesn't care: it should just cast an ACCESS_ABSTAIN. Later on it might run into an AdminVoter that will still cast an ACCESS_GRANTED. When using the unanimous access decision manager strategy, however, if this PostVoter already casted an ACCESS_DENIED it will never get to any other voters.

I can't cast that ACCESS_ABSTAIN in the AbstractVoter now without overriding the vote method and the whole point of simpler voters in my mind was, well, to make it simpler :)

Answer 1

The AbstractVoter class is a simple bootstrap implementation for majority of use cases and it emulates how internal voters behave, so as soon as you feel your solution differs from the base case, you should feel free to override with your own methods.