Refactor authorization helper to separate some methods

Methods regarding to role hierarchi are now delegated to a parent role helper.

src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php

@@ -25,6 +25,7 @@ use Chill\MainBundle\Entity\HasCenterInterface;
 use Chill\MainBundle\Entity\HasScopeInterface;
 use Chill\MainBundle\Repository\UserACLAwareRepository;
 use Chill\MainBundle\Repository\UserACLAwareRepositoryInterface;
+use Chill\MainBundle\Security\ParentRoleHelper;
 use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher;
 use Chill\MainBundle\Security\Resolver\ScopeResolverDispatcher;
 use Chill\MainBundle\Security\Resolver\ScopeResolverInterface;
@@ -46,42 +47,28 @@ use Chill\MainBundle\Entity\RoleScope;
  */
 class AuthorizationHelper implements AuthorizationHelperInterface
 {
-    protected RoleHierarchyInterface $roleHierarchy;
+    private CenterResolverDispatcher $centerResolverDispatcher;
 
-    /**
-     * The role in a hierarchy, given by the parameter
-     * `security.role_hierarchy.roles` from the container.
-     *
-     * @var string[]
-     */
-    protected array $hierarchy;
+    private ScopeResolverDispatcher $scopeResolverDispatcher;
 
-    protected EntityManagerInterface $em;
-
-    protected CenterResolverDispatcher $centerResolverDispatcher;
-
-    protected ScopeResolverDispatcher $scopeResolverDispatcher;
-
-    protected LoggerInterface $logger;
+    private LoggerInterface $logger;
 
     private UserACLAwareRepositoryInterface $userACLAwareRepository;
 
+    private ParentRoleHelper $parentRoleHelper;
+
     public function __construct(
-        RoleHierarchyInterface $roleHierarchy,
-        ParameterBagInterface $parameterBag,
-        EntityManagerInterface $em,
         CenterResolverDispatcher $centerResolverDispatcher,
         LoggerInterface $logger,
         ScopeResolverDispatcher $scopeResolverDispatcher,
-        UserACLAwareRepositoryInterface $userACLAwareRepository
+        UserACLAwareRepositoryInterface $userACLAwareRepository,
+        ParentRoleHelper $parentRoleHelper
     ) {
-        $this->roleHierarchy = $roleHierarchy;
-        $this->hierarchy     = $parameterBag->get('security.role_hierarchy.roles');
-        $this->em = $em;
         $this->centerResolverDispatcher = $centerResolverDispatcher;
         $this->logger = $logger;
         $this->scopeResolverDispatcher = $scopeResolverDispatcher;
         $this->userACLAwareRepository = $userACLAwareRepository;
+        $this->parentRoleHelper = $parentRoleHelper;
     }
 
     /**
@@ -93,7 +80,7 @@ class AuthorizationHelper implements AuthorizationHelperInterface
      * @param Center|Center[] $center May be an array of center
      * @return bool
      */
-    public function userCanReachCenter(User $user, $center)
+    public function userCanReachCenter(User $user, $center): bool
     {
         if ($center instanceof \Traversable) {
             foreach ($center as $c) {
@@ -340,39 +327,25 @@ class AuthorizationHelper implements AuthorizationHelperInterface
     /**
      * Test if a parent role may give access to a given child role
      *
-     * @param Role $childRole The role we want to test if he is reachable
-     * @param Role $parentRole The role which should give access to $childRole
+     * @param string $childRole The role we want to test if he is reachable
+     * @param string $parentRole The role which should give access to $childRole
      * @return boolean true if the child role is granted by parent role
      */
-    protected function isRoleReached($childRole, $parentRole)
+    private function isRoleReached(string $childRole, string $parentRole)
     {
-        $reachableRoles = $this->roleHierarchy
-                ->getReachableRoleNames([$parentRole]);
-
-        return in_array($childRole, $reachableRoles);
+        return $this->parentRoleHelper->isRoleReached($childRole, $parentRole);
     }
 
     /**
      * Return all the role which give access to the given role. Only the role
      * which are registered into Chill are taken into account.
      *
-     * @param Role $role
+     * @param string $role
      * @return string[] the role which give access to the given $role
      */
-    public function getParentRoles($role): array
+    public function getParentRoles(string $role): array
     {
-        $parentRoles = [];
-        // transform the roles from role hierarchy from string to Role
-        $roles = \array_keys($this->hierarchy);
-
-        foreach ($roles as $r) {
-            $childRoles = $this->roleHierarchy->getReachableRoleNames([$r]);
-
-            if (\in_array($role, $childRoles)) {
-                $parentRoles[] = $r;
-            }
-        }
-
-        return $parentRoles;
+        trigger_deprecation('Chill\MainBundle', '2.0', 'use ParentRoleHelper::getParentRoles instead');
+        return $this->parentRoleHelper->getParentRoles($role);
     }
 }

src/Bundle/ChillMainBundle/Security/ParentRoleHelper.php

@@ -0,0 +1,71 @@
+<?php
+
+namespace Chill\MainBundle\Security;
+
+use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
+use Symfony\Component\Security\Core\Role\Role;
+use Symfony\Component\Security\Core\Role\RoleHierarchyInterface;
+
+/**
+ * Helper which traverse all role to find parents
+ */
+class ParentRoleHelper
+{
+    protected RoleHierarchyInterface $roleHierarchy;
+
+    /**
+     * The role in a hierarchy, given by the parameter
+     * `security.role_hierarchy.roles` from the container.
+     *
+     * @var string[]
+     */
+    protected array $hierarchy;
+
+    public function __construct(
+        RoleHierarchyInterface $roleHierarchy,
+        ParameterBagInterface $parameterBag
+    ) {
+        $this->roleHierarchy = $roleHierarchy;
+        $this->hierarchy     = $parameterBag->get('security.role_hierarchy.roles');
+    }
+
+    /**
+     * Return all the role which give access to the given role. Only the role
+     * which are registered into Chill are taken into account.
+     *
+     * @param string $role
+     * @return string[] the role which give access to the given $role
+     */
+    public function getParentRoles(string $role): array
+    {
+        $parentRoles = [];
+        // transform the roles from role hierarchy from string to Role
+        $roles = \array_keys($this->hierarchy);
+
+        foreach ($roles as $r) {
+            $childRoles = $this->roleHierarchy->getReachableRoleNames([$r]);
+
+            if (\in_array($role, $childRoles)) {
+                $parentRoles[] = $r;
+            }
+        }
+
+        return $parentRoles;
+    }
+
+    /**
+     * Test if a parent role may give access to a given child role
+     *
+     * @param string $childRole The role we want to test if he is reachable
+     * @param string $parentRole The role which should give access to $childRole
+     * @return bool true if the child role is granted by parent role
+     */
+    public function isRoleReached($childRole, $parentRole): bool
+    {
+        $reachableRoles = $this->roleHierarchy
+            ->getReachableRoleNames([$parentRole]);
+
+        return in_array($childRole, $reachableRoles);
+    }
+
+}

src/Bundle/ChillMainBundle/Tests/Authorization/ParentRoleHelperTest.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace Chill\MainBundle\Tests\Security\Authorization;
+
+use Chill\MainBundle\Security\ParentRoleHelper;
+use Chill\PersonBundle\Security\Authorization\PersonVoter;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+
+class ParentRoleHelperTest extends KernelTestCase
+{
+    private ParentRoleHelper $parentRoleHelper;
+
+    public function setUp()
+    {
+        self::bootKernel();
+        $this->parentRoleHelper = self::$container->get(ParentRoleHelper::class);
+    }
+
+    public function testGetReachableRoles()
+    {
+        // this test will be valid until the role hierarchy for person is changed.
+        // this is not perfect but spare us a mock
+
+        $parentRoles = $this->parentRoleHelper->getParentRoles(PersonVoter::SEE);
+
+        $this->assertCount(2, $parentRoles);
+        $this->assertContains(PersonVoter::CREATE, $parentRoles);
+        $this->assertContains(PersonVoter::UPDATE, $parentRoles);
+    }
+
+    public function testIsRoleReached()
+    {
+        $this->assertTrue($this->parentRoleHelper->isRoleReached(PersonVoter::SEE, PersonVoter::CREATE));
+        $this->assertFalse($this->parentRoleHelper->isRoleReached(PersonVoter::SEE, 'foo'));
+    }
+}

src/Bundle/ChillMainBundle/config/services/security.yaml

@@ -40,6 +40,10 @@ services:
     Chill\MainBundle\Security\Authorization\AuthorizationHelper: '@chill.main.security.authorization.helper'
     Chill\MainBundle\Security\Authorization\AuthorizationHelperInterface: '@chill.main.security.authorization.helper'
 
+    Chill\MainBundle\Security\ParentRoleHelper:
+        autowire: true
+        autoconfigure: true
+
     chill.main.role_provider:
         class: Chill\MainBundle\Security\RoleProvider
     Chill\MainBundle\Security\RoleProvider: '@chill.main.role_provider'