Fixed the sample security voter to take into account the role hierarchy#5024
Fixed the sample security voter to take into account the role hierarchy#5024javiereguiluz wants to merge 6 commits intosymfony:2.3from
Conversation
best_practices/security.rst
Outdated
best_practices/security.rst
Outdated
There was a problem hiding this comment.
wouldn't userHasRole be a better name. It now looks like a hasser/isser of the class itself
|
The RoleHierarchy service will not exist if you don't use the hierarchy in your security config |
…se a role hierarchy
|
@stof thanks for the tip about the |
best_practices/security.rst
Outdated
There was a problem hiding this comment.
this is wrong. You should still use $userToken->getRoles() (which are the only reachable roles in this case).
best_practices/security.rst
Outdated
There was a problem hiding this comment.
[...] hierarchy if defined [...] or [...] hierarchy as defined [...]?
best_practices/security.rst
Outdated
8000
There was a problem hiding this comment.
I think you should pass UserInterface here.
private function userHasRole(UserInterface $user, $roleName)
There was a problem hiding this comment.
Much better. Fixed now. Thanks.
There was a problem hiding this comment.
this is wrong. You should be using the roles from the token to be consistent with the way the RoleVoter is working
There was a problem hiding this comment.
there is no way to retrieve token https://github.com/symfony/symfony/blob/2.7/src/Symfony/Component/Security/Core/Authorization/Voter/AbstractVoter.php#L76
|
Closing it because we can't fix what we want to fix (because of the comments of @stof and @aitboudad) |
I applied the solution suggested by @jdachtera because it seems to be the best one.