Add example for using a voter to restrict switch_user #9353
Conversation
jwmickey
changed the title
security/impersonating_user.rst
Outdated
|
|
||
| First, create the voter class:: | ||
|
|
||
| namespace AppBundle\Security\Voter; |
There was a problem hiding this comment.
namespace App\Security\Voter;to be coherent with SF4 structure
security/impersonating_user.rst
Outdated
|
|
||
| if (in_array('ROLE_CUSTOMER', $subject->getRoles()) | ||
| && $this->hasSwitchToCustomerRole($token)) { | ||
| return self::ACCESS_GRANTED; |
There was a problem hiding this comment.
It should be return true; no?
security/impersonating_user.rst
Outdated
|
|
||
| // Explicitly configure the service | ||
| $container->getDefinition(SwitchToCustomerVoter::class) | ||
| ->setArgument('$roleHierarchy', '@security.role_hierarchy'); |
There was a problem hiding this comment.
This should be new Reference('security.role_hierarchy')
|
Thank you for reviewing this PR @atailouloute, I have added your recommendations. |
security/impersonating_user.rst
Outdated
|
|
||
| private function hasSwitchToCustomerRole(TokenInterface $token) | ||
| { | ||
| $roles = $this->roleHierarchy->getReachableRoles($token->getRoles()); |
There was a problem hiding this comment.
I would leave out the role hierarchy here. It IMO adds more complexity than needed to understand the example.
|
I think we should add a headline before the new section to make it stand out more. And we should then also add a |
security/impersonating_user.rst
Outdated
| @@ -187,6 +187,179 @@ setting: | |||
| ), | |||
| )); | |||
|
|
|||
| If you need more control over user switching, but don't require the complexity | |||
| of a full ACL implementation, you can use a security voter. For example, you | |||
There was a problem hiding this comment.
2 spaces should be one voter. For example
security/impersonating_user.rst
Outdated
|
|
||
| Notice that when checking for the ``ROLE_CUSTOMER`` role on the target user, only the roles | ||
| explicitly assigned to the user are checked rather than checking all reachable roles from | ||
| the role hierarchy. The reason for this is to avoid accidentally granting access to an |
There was a problem hiding this comment.
double spaces: hierarchy. The reason
security/impersonating_user.rst
Outdated
| ), | ||
| )); | ||
|
|
||
| Thanks to autowiring, we only need to configure the role hierarchy argument when registering |
There was a problem hiding this comment.
we only need to configure. Correct me if I'm wrong, but I believe the Symfony docs point towards "you", rather than "we".
carsonbot
added
Status: Needs Work
and removed
Status: Needs Review
Status: Needs Work
labels
- removed role_heirarchy from switch user voter to simplify - removed double-spaces between sentences. - added versionadded - added header
…wmickey, javiereguiluz) This PR was merged into the 4.1 branch. Discussion ---------- Add example for using a voter to restrict switch_user This recreates #9353, which has too many conflicts when merging it. Commits ------- 7853ec0 Minor tweaks 876257b implemented community suggestions e6c569b Fix recommendations from community review cf3bd00 [WIP] add example for using a voter to restrict switch_user
|
@jwmickey thanks a lot for contributing this nice example (and thanks to all reviewers!). I apologize for having taken so long to merge it. I tried to merge but there were lots of conflicts ... so I created #10842 to replace this and that is now merged. Of course I kept your original commits so you'll get full credit of your contribution. So, let's close this now that #10842 is merged. Thanks! |
|
Thanks @javiereguiluz, glad this was able to be included! |
No description provided.