[Security] Voters: What's the advantage over doing it in the controller?

[ThomasLandauer]

The entire example used throughout https://symfony.com/doc/4.4/security/voters.html (namely checking if a certain user is allowed to edit this specific post) can be achieved with 2 lines in the "edit"-Controller:

if ($post->getOwner() !== $this->getUser()) {
    throw new AccessDeniedException();
}

So what's the advantage of setting up this complicated voting system?? This should be explained on the page.

[xabbuh]

Looking at the lines of code used in the voter I would have said that it should be obvious to spot while you rather not want to place such a check in each individual controller. Do you think it would makes it more clear if there also was a sentence explaining that voter centralise the logic so that there is only one place to update when requirements change?

[ThomasLandauer]

In some cases (like in the "Post" example), there is only one controller/route to protect. So you will have to change it in just one place anyway. So the question is not about "centralizing" it, but about "outsourcing" it.

So the question is: In which situations is one approach smarter than the other? What are the relevant questions you have to ask yourself?

[xabbuh]

From my experience I would never inline such a check in controllers. Using voters always has the advantage of having a few places where you control your access control rules which allows you to quite easily spot how permissions are distributed. Furthermore, you have a central place where you can adjust checks without risking to forget a place.

[dbrumann]

How does this relate to the docs? Are you proposing a discussion section on when to use/not use features? I can't think of any other part in the docs where we discuss when/how components are "justified".

[ThomasLandauer]

Well, take look at e.g. at https://symfony.com/doc/current/forms.html#creating-forms-in-controllers

"Creating Forms in Controllers" is explained first, "Creating Form Classes" as alternative. Plus:

Symfony recommends to put as little logic as possible in controllers. That's why it's better to move complex forms to dedicated classes instead of defining them in controller actions. Besides, forms defined in classes can be reused in multiple actions and services.

This is a clear recommendation, along with a reason (=reuseability). So why not having some pros and cons for alternative approaches in the Security docs too?

[ThomasLandauer]

I created a (somehow) related gist: https://gist.github.com/ThomasLandauer/668d7353dc5794da62be4cec9e8091ab

[ThomasLandauer]

Just for the records: What I had in mind, is not explained in today's PR ;-)

[wouterj]

I think it's best to always do these things in a voter. The big difference between the form and security examples is that forms are usable in controllers, the Security Authorization system isn't and would involve writing your own logic (your example doesn't use anything from Symfony Security).

In #13522 I made Voters the more clear go-to abstraction for non-role based permissions. I think that's good enough.

[wouterj]

For the record: There are always 101 ways to solve a problem in software. What way is best depends completely on the type of application, the experience/size of the dev team, and many more parameters. So if the if-statement works and is the best in your case, feel free to go with it! 😃

[ThomasLandauer]

Well, I appreciate there's at least some progress here :-) And your PR is certainly a step in the right direction! See also #13553

My example above uses $this->getUser() from Security. And all I'm asking for is a short note (1-2 sentences) saying: If you have just 1-2 controllers, it's ok to do it like this: ... But if you want to do it more professional, see here: ...

If you agree in principle, I'll look for an appropriate place and come up with a PR. Just don't want to waste my time, if you won't merge it ;-)