From e6b10e92f90ed81210149d5ca65688d10b8770be Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Thu, 19 Feb 2015 16:40:30 +0100 Subject: [PATCH 1/6] Fixed the sample security voter to take into account the role hierarchy --- best_practices/security.rst | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/best_practices/security.rst b/best_practices/security.rst index 06c9e3956eb..9f4eac31ca6 100644 --- a/best_practices/security.rst +++ b/best_practices/security.rst @@ -140,6 +140,8 @@ the same ``getAuthorEmail`` logic you used above: use Symfony\Component\Security\Core\Authorization\Voter\AbstractVoter; use Symfony\Component\Security\Core\User\UserInterface; + use Symfony\Component\Security\Core\Role\RoleHierarchyInterface; + use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; // AbstractVoter class requires Symfony 2.6 or higher version class PostVoter extends AbstractVoter @@ -147,6 +149,13 @@ the same ``getAuthorEmail`` logic you used above: const CREATE = 'create'; const EDIT = 'edit'; + protected $roleHierarchy; + + public function __construct(RoleHierarchyInterface $roleHierarchy) + { + $this->roleHierarchy = $roleHirarchy; + } + protected function getSupportedAttributes() { return array(self::CREATE, self::EDIT); @@ -163,7 +172,7 @@ the same ``getAuthorEmail`` logic you used above: return false; } - if ($attribute === self::CREATE && in_array('ROLE_ADMIN', $user->getRoles(), true)) { + if ($attribute === self::CREATE && $this->hasRole('ROLE_ADMIN', $user)) { return true; } @@ -173,6 +182,21 @@ the same ``getAuthorEmail`` logic you used above: return false; } + + /** + * 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) + { + foreach ($this->roleHierarchy->getReachableRoles($userToken->getRoles()) as $role) { + if ($roleName === $role->getRole()) { + return true; + } + } + + return false; + } } To enable the security voter in the application, define a new service: @@ -184,6 +208,7 @@ To enable the security voter in the application, define a new service: # ... post_voter: class: AppBundle\Security\PostVoter + arguments: ["@security.role_hierarchy"] public: false tags: - { name: security.voter } From f9be8688063406f4442a38abb8307517206d6ec6 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Thu, 19 Feb 2015 20:19:47 +0100 Subject: [PATCH 2/6] Minor fixes in the sample code --- best_practices/security.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/best_practices/security.rst b/best_practices/security.rst index 9f4eac31ca6..b177a554108 100644 --- a/best_practices/security.rst +++ b/best_practices/security.rst @@ -149,11 +149,11 @@ the same ``getAuthorEmail`` logic you used above: const CREATE = 'create'; const EDIT = 'edit'; - protected $roleHierarchy; + private $roleHierarchy; public function __construct(RoleHierarchyInterface $roleHierarchy) { - $this->roleHierarchy = $roleHirarchy; + $this->roleHierarchy = $roleHierarchy; } protected function getSupportedAttributes() @@ -172,7 +172,7 @@ the same ``getAuthorEmail`` logic you used above: return false; } - if ($attribute === self::CREATE && $this->hasRole('ROLE_ADMIN', $user)) { + if ($attribute === self::CREATE && $this->userHasRole($user, 'ROLE_ADMIN')) { return true; } @@ -187,7 +187,7 @@ the same ``getAuthorEmail`` logic you used above: * 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) + private function userHasRole(TokenInterface $userToken, $roleName) { foreach ($this->roleHierarchy->getReachableRoles($userToken->getRoles()) as $role) { if ($roleName === $role->getRole()) { From d68003c72c99965681aad615f0b4016d27d1576b Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Sat, 21 Feb 2015 23:05:02 +0100 Subject: [PATCH 3/6] 'security.role_hierarchy' service is only defined when you actually use a role hierarchy --- best_practices/security.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/best_practices/security.rst b/best_practices/security.rst index b177a554108..de7c2b84ac0 100644 --- a/best_practices/security.rst +++ b/best_practices/security.rst @@ -151,7 +151,7 @@ the same ``getAuthorEmail`` logic you used above: private $roleHierarchy; - public function __construct(RoleHierarchyInterface $roleHierarchy) + public function __construct(RoleHierarchyInterface $roleHierarchy = null) { $this->roleHierarchy = $roleHierarchy; } @@ -189,6 +189,10 @@ the same ``getAuthorEmail`` logic you used above: */ private function userHasRole(TokenInterface $userToken, $roleName) { + if (null === $this->roleHierarchy) { + return false; + } + foreach ($this->roleHierarchy->getReachableRoles($userToken->getRoles()) as $role) { if ($roleName === $role->getRole()) { return true; @@ -208,7 +212,9 @@ To enable the security voter in the application, define a new service: # ... post_voter: class: AppBundle\Security\PostVoter - arguments: ["@security.role_hierarchy"] + # beware that 'security.role_hierarchy' service is only defined + # when the application's security config uses role hierarchy + arguments: ["@?security.role_hierarchy"] public: false tags: - { name: security.voter } From 75b35e03971b1e8acfe71619984f20c9cf6f891b Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Mon, 23 Feb 2015 12:48:10 +0100 Subject: [PATCH 4/6] Fixed the code sample when the application doesn't use the RoleHierarchy --- best_practices/security.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/best_practices/security.rst b/best_practices/security.rst index de7c2b84ac0..8844776bb7d 100644 --- a/best_practices/security.rst +++ b/best_practices/security.rst @@ -190,7 +190,7 @@ the same ``getAuthorEmail`` logic you used above: private function userHasRole(TokenInterface $userToken, $roleName) { if (null === $this->roleHierarchy) { - return false; + return in_array($roleName, $userToken->getRoles(), true); } foreach ($this->roleHierarchy->getReachableRoles($userToken->getRoles()) as $role) { From 3dd747c0d0338fefba097d5b5dd153571e7eacf8 Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Tue, 24 Feb 2015 08:00:43 +0100 Subject: [PATCH 5/6] Fixed some grammar issues --- best_practices/security.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/best_practices/security.rst b/best_practices/security.rst index 8844776bb7d..9785a58945a 100644 --- a/best_practices/security.rst +++ b/best_practices/security.rst @@ -185,7 +185,7 @@ the same ``getAuthorEmail`` logic you used above: /** * Checks if the user token has the given role taking into account the - * entire role hierarchy defined by the application. + * entire role hierarchy if defined by the application. */ private function userHasRole(TokenInterface $userToken, $roleName) { @@ -212,8 +212,8 @@ To enable the security voter in the application, define a new service: # ... post_voter: class: AppBundle\Security\PostVoter - # beware that 'security.role_hierarchy' service is only defined - # when the application's security config uses role hierarchy + # beware that the 'security.role_hierarchy' service is only defined + # when the application's security config uses the role hierarchy arguments: ["@?security.role_hierarchy"] public: false tags: From 471ecda7a602d1cc2720040362820bf61d8bd00d Mon Sep 17 00:00:00 2001 From: Javier Eguiluz Date: Sat, 28 Feb 2015 11:17:21 +0100 Subject: [PATCH 6/6] Replaced TokenInterface by UserInterface --- best_practices/security.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/best_practices/security.rst b/best_practices/security.rst index 9785a58945a..8ffcb4955b4 100644 --- a/best_practices/security.rst +++ b/best_practices/security.rst @@ -141,7 +141,7 @@ the same ``getAuthorEmail`` logic you used above: use Symfony\Component\Security\Core\Authorization\Voter\AbstractVoter; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\Role\RoleHierarchyInterface; - use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; + use Symfony\Component\Security\Core\User\UserInterface; // AbstractVoter class requires Symfony 2.6 or higher version class PostVoter extends AbstractVoter @@ -187,13 +187,13 @@ the same ``getAuthorEmail`` logic you used above: * 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) + private function userHasRole(UserInterface $user, $roleName) { if (null === $this->roleHierarchy) { - return in_array($roleName, $userToken->getRoles(), true); + return in_array($roleName, $user->getRoles(), true); } - foreach ($this->roleHierarchy->getReachableRoles($userToken->getRoles()) as $role) { + foreach ($this->roleHierarchy->getReachableRoles($user->getRoles()) as $role) { if ($roleName === $role->getRole()) { return true; }