Skip to content

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

Closed
@ThomasLandauer

Description

@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:
    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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions