Fixed the sample security voter to take into account the role hierarchy #5024
10000Conversation
|
|
||
| public function __construct(RoleHierarchyInterface $roleHierarchy) | ||
| { | ||
| $this->roleHierarchy = $roleHirarchy; |
| * Checks if the user token has the given role taking into account the | ||
| * entire role hierarchy defined by the application. | ||
| */ | ||
| protected function hasRole($roleName, TokenInterface $userToken) |
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 |
| private function userHasRole(TokenInterface $userToken, $roleName) | ||
| { | ||
| if (null === $this->roleHierarchy) { | ||
| return false; |
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).
|
|
||
| /** | ||
| * Checks if the user token has the given role taking into account the | ||
| * entire role hierarchy defined by the application. |
There was a problem hiding this comment.
[...] hierarchy if defined [...] or [...] hierarchy as defined [...]?
| * Checks if the user token has the given role taking into account the | ||
| * entire role hierarchy if defined by the application. | ||
| */ | ||
| private function userHasRole(TokenInterface $userToken, $roleName) |
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.
| private function userHasRole(UserInterface $user, $roleName) | ||
| { | ||
| if (null === $this->roleHierarchy) { | ||
| return in_array($roleName, $user->getRoles(), true); |
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.