-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[Security] Voters: What's the advantage over doing it in the controller? #13406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
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? |
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? |
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. |
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". |
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:
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? |
I created a (somehow) related gist: https://gist.github.com/ThomasLandauer/668d7353dc5794da62be4cec9e8091ab |
Just for the records: What I had in mind, is not explained in today's PR ;-) |
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. |
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! 😃 |
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 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 ;-) |
Ah, then I misunderstood. I thought you wanted to first discuss doing things without using |
See #13554 |
…auer, wouterj) This PR was merged into the 3.4 branch. Discussion ---------- Explaining controllers as viable alternative See #13406 (comment) Please wrap it in a "caution" box, or whatever you think is appropriate. I'm unsure about how much context of the controller should be shown in the code, since the entire controller is only shown later. If you think it's too confusing now, I'd suggest to show the controller as "skeleton" above this note. Commits ------- 15c4adc Update voters.rst bb0872a Update voters.rst 577dfa2 Update voters.rst fdfd1ad Update security/voters.rst 221bbb1 Update voters.rst 164aec4 [#13554] Slightly reworded the tip 2bc5c54 Update voters.rst 40bbd81 Explaining controllers as viable alternative
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:
So what's the advantage of setting up this complicated voting system?? This should be explained on the page.
The text was updated successfully, but these errors were encountered: