Symfony-docs: [Security] Severe: When following the docs, each user can access each other user's data

[ThomasLandauer]

I'm upset today, since I followed https://symfony.com/doc/current/security.html and ended up with a ridiculous "security" system in which every user can access everything (i.e. every other users' data).

The relevant information on how to do it right is hidden in the chapter https://symfony.com/doc/current/security.html#access-control-lists-acls-securing-individual-database-objects
This entire chapter is problematic for many reasons:

• Under the heading "Access Control Lists" it is recommended to avoid Access Control Lists.
• The second part of the heading ("Securing individual Database Objects") is outright wrong, since this has nothing to do with database objects at all.
• "Imagine you are designing a blog..." sounds more like the beginning of a fairy tale than the beginning of one of the most important parts on the entire page.
• The real important information is just not there. It's hidden under a tiny link to "Voters" (which is a term that sounds only relevant for organizers of a public election).

Usually I don't expand so much on the shortcomings of any part of the docs. I try to improve it instead. But I've been asked the question "Why do you want to change it?" too often. So I'm trying it the other way round today.

So what needs to be done on this page IMHO:

• Get rid of this "role" stuff! For the majority of users, it would be better if they've never heard about it, cause (a) they won't need it, and (b) it's highly misleading. Why? The term ROLE_USER suggests that we are talking about individual users. But we aren't. Anything "restricted" to ROLE_USER is open to any user. Unanswered (unanswerable?) question: Why do those "pseudo-individual" users then need separate passwords at all, when (in the end) it just doesn't make any difference??
• The voters stuff should become a top-level heading and go up higher on the page, at least up to "Denying Access, Roles and other Authorization", maybe even up to 'The "User Provider"'.
• In that voters chapter the easiest way to do it should be shown right away:
  php if ($user->getId() !== $this->getUser()->getId()) { throw new AccessDeniedException(); }
  ... and only refered to https://symfony.com/doc/current/security/voters.html for more complex situations.

I'd be willing to re-write it in principle, but I already have 5 PR's sitting around without much progress: https://github.com/symfony/symfony-docs/pulls/ThomasLandauer

[javiereguiluz]

Thomas, I don't care if you are upset or had a bad day. The tone and wording of your issue is not acceptable. Please, reword it to avoid all the aggressive and insulting parts and then we'll consider it. Symfony Doc maintainers and contributor's deserve some respect and you should know better because you are also a contributor. Thanks.

[javiereguiluz]

Thank you for rewording your original issue description. Now we'll consider the issue and we'll come with a reply to your questions/comments.

[xabbuh]

I do not agree on dropping the part about roles. We can of course discuss whether voters are as important as roles, but from my experience there are many real applications where you do never need to protect objects individually, but there are just certain groups of people that share the same level of permissions and for which roles are the solution that fits best. But we should of course proofread the chapter to see if it is clear here that roles are not the solution to protect individual objects.

I would still keep the voters in their own article, but we should maybe make it more clear in the section about roles that there is another tool to protect individual objects to make its discovery a bit easier.

[ThomasLandauer]

Loosely related: https://github.com/symfony/symfony-docs/issues/13406

[ThomasLandauer]

Zero progress after almost a year.

@xabbuh: I didn't mean to drop roles completely. It's enough if you <sarcasm> hide it under a tiny link with a misleading title </sarcasm> - just like "Voters" ;-)

OK, now seriously: If roles are "advertised" so much, this question should be answered: What's the advantage of having different users at all, when ultimately they all have the same permissions?

[wouterj]

Roles and permissions, as discussed in this issue, are 2 very different topics. See also https://wouterj.nl/2020/01/grant-on-permissions-not-roles

I do agree that voters should get a more prominent place in the main Security guide than it has now.
I however don't agree that this results in severely insecure applications. We don't claim anywhere that doing $this->denyAccessUnlessGranted("ROLE_USER") means the user can access only their own stuff. And the voters guide does imho show a good example to make sure users can only view their own stuff.