4

I am using Voters to restrict access to entities in a REST API.

Step 1

Consider this voter that restricts users access to blog posts:

class BlogPostVoter extends Voter
{
    public function __construct(AccessDecisionManagerInterface $decisionManager)
    {
        $this->decisionManager = $decisionManager;
    }

    /**
     * Determines if the attribute and subject are supported by this voter.
     *
     * @param string $attribute An attribute
     * @param int $subject The subject to secure, e.g. an object the user wants to access or any other PHP type
     *
     * @return bool True if the attribute and subject are supported, false otherwise
     */
    protected function supports($attribute, $subject)
    {
        if (!in_array($attribute, $this->allowedAttributes)) {
            return false;
        }
        if (!$subject instanceof BlogPost) {
            return false;
        }

        return true;
    }

    /**
     * Perform a single access check operation on a given attribute, subject and token.
     *
     * @param string $attribute
     * @param mixed $subject
     * @param TokenInterface $token
     * @return bool
     * @throws \Exception
     */
    protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
    {
        return $this->canUserAccess($attribute, $subject, $token);
    }

    public function canUserAccess($attribute, $subject, TokenInterface $token) {
        if ($this->decisionManager->decide($token, array('ROLE_SUPPORT', 'ROLE_ADMIN'))) {
            return true;
        } 

        //other logic here omitted ...

        return false;
    }
}

You can see there is a public function canUserAccess to determine if the user is allowed to see the BlogPost. This all works just fine.

Step 2

Now I have another voter that checks something else, but also needs to check this same exact logic for BlogPosts. My thought was to:

  • add a new voter
  • perform some other checks
  • but then also perform this BlogPost check

So I thought I would inject the BlogPostVoter into my other voter like this:

class SomeOtherVoter extends Voter
{
    public function __construct(BlogPostVoter $blogPostVoter)
    {
        $this->decisionManager = $decisionManager;
    }

    ...

    protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
    {
        //other logic

        if ($this->blogPostVoter->canUserAccess($attribute, $subject, $token))    {
           return true;
        }

        return false;
    }
}

Problem

When I do this I get the following error, using both setter and constructor injection:

Circular reference detected for service "security.access.decision_manager", path: "security.access.decision_manager"

I don't see where the security.access.decision_manager should depend on the Voter implementations. So I'm not seeing where the circular reference is.

Is there another way I can call VoterA from VoterB?

MattW
  • 12,902
  • 5
  • 38
  • 65
  • You can look at the security compiler (or maybe just the resulting container in the cache) to see how the decision manager ends up being dependent on voters. Basically, all the tagged voters are injected as soon as the decision manager is instantiated. Same thing happens with the Doctrine entity manager and it's listeners. Off of the top of my head I might suggest moving the canUserAccess functionality into a trait and using the trait in both voters. Depends in on what the "additional functionality" does. – Cerad Oct 14 '16 at 16:07
  • Another trick is to use setter injection to inject the decision manager. Never really liked doing that but it seems to get around the circular reference issue. – Cerad Oct 14 '16 at 16:09
  • Not an answer, but is it possible to solve with class inheritance instead? – Cameron Hurd Oct 14 '16 at 16:13
  • Thanks @Cerad! I tried setter injection, same issue. Blah. For now I've moved the logic from the voter to a separete service that each voter can have injected safely. I kind of wanted to leave the security logic with the voter, but this seems to work fine and isn't very dirty. I'm not familar w/ traits however ... – MattW Oct 14 '16 at 18:09
  • @CameronHurd yeah I bet that'd be a good way to solve it also. But I went with a service instead. – MattW Oct 14 '16 at 18:10
  • Is it not enough to change strategy of decision manager? – malcolm Oct 14 '16 at 18:33
  • @malcolm I don't follow. I want the strategy to always be unanimous. And I don't want to have to call `denyAccessUnlessGranted` on multiple objects in the controller. But I could be missing something here .... – MattW Oct 14 '16 at 18:49

1 Answers1

2

To reference VoterOne from VoterTwo you can inject the AuthorizationCheckerInterface into VoterTwo and then call ->isGranted('ONE'). Where ONE is the supported attribute of VoterOne.

Like:

class VoterTwo extends Voter
{
    private $authorizationChecker;

    public function __construct(AuthorizationCheckerInterface $authorizationChecker)
    {
        $this->authorizationChecker = $authorizationChecker;
    }

    protected function supports($attribute, $subject)
    {
        return in_array($attribute, ['TWO']);
    }

    protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
    {
        return $this->authorizationChecker->isGranted('ONE', $subject);
    }
}

In this example VoterTwo does just redirect the request to VoterOne (or the voter that supports the attribute ONE). This can then be extended through additional conditions.

Kim
  • 1,757
  • 1
  • 17
  • 32