Skip to content

Caution that roles should start with ROLE_ #4218

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

Merged
merged 4 commits into from
Sep 16, 2014
Merged

Caution that roles should start with ROLE_ #4218

merged 4 commits into from
Sep 16, 2014

Conversation

jrjohnson
Copy link
Contributor

In order to save someone else a few hours of frustration make it clear that the ROLE_ preface is not just for show.

Q A
Doc fix? yes
New docs? no
Applies to all
Fixed tickets

In order to save someone else a few hours of frustration make it clear that the ROLE_ preface is not just for show.
.. caution::

The default role voter requires that all of the roles returned should be prefaced by ROLE_.
Example ROLE_ADMIN, ROLE_USER; not just ADMIN or USER.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would reword this a bit and also link to the "Roles" section in the security chapter of the book:

.. caution::

    The default role voter requires to prefix all roles with ``ROLE_`` (see
    the :ref:`section about roles <book-security-roles>` in the book). For
    example, your roles will be ``ROLE_ADMIN`` or ``ROLE_USER`` instead of
    ``ADMIN`` or ``USER``.

In /book/security.rst you would then have to change it to something like this (add the label before the headline):

.. _book-security-roles:

Roles
-----

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not say The default role voter. the main confusion actually comes from a wrong usage of the term role in the documentation. See #4158

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof Well, I thought that at least http://symfony.com/doc/current/book/security.html#roles and its note are correct.

@jrjohnson
Copy link
Contributor Author

How about this version? I'm not sure 'security examples on this page' is clear, but it describes the problem I was having.

@jrjohnson
Copy link
Contributor Author

Let me know if you would rather I rebased this once the language is correct. PS thanks Travis for catching each of my bonehead mistakes.

@xabbuh
Copy link
Member

xabbuh commented Sep 13, 2014

👍 I think it's ready.

@weaverryan
Copy link
Member

Thanks for the addition and all of the fast tweaks. Cheers Jonathan!

@weaverryan weaverryan merged commit 749996b into symfony:2.3 Sep 16, 2014
weaverryan added a commit that referenced this pull request Sep 16, 2014
This PR was merged into the 2.3 branch.

Discussion
----------

Caution that roles should start with ROLE_

In order to save someone else a few hours of frustration make it clear that the ROLE_ preface is not just for show.

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets |

Commits
-------

749996b Fix reference label
f856641 Add label book-security-roles
9d13930 Add formatting, links, and clarity
1522de7 Caution that roles should start with ROLE_
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants