diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a4588498e..68b1bcf08 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -16,7 +16,7 @@ before_script: - php -d memory_limit=2G composer.phar install - php tests/app/bin/console doctrine:migrations:migrate -n - php -d memory_limit=2G tests/app/bin/console cache:clear --env=dev - - php -d memory_limit=2G tests/app/bin/console doctrine:fixtures:load -n + - php -d memory_limit=3G tests/app/bin/console doctrine:fixtures:load -n - php -d memory_limit=2G tests/app/bin/console cache:clear --env=test - echo "before_script finished" diff --git a/CHANGELOG.md b/CHANGELOG.md index 507d4dc40..2a4d8aa69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,11 +11,13 @@ and this project adheres to ## Unreleased + * [tasks] improve tasks with filter order * [tasks] refactor singleControllerTasks: limit the number of conditions from the context * [validations] validation of accompanying period added: no duplicate participations or resources (https://gitlab.com/champs-libres/departement-de-la-vendee/accent-suivi-developpement/-/issues/60). * [renderbox] If gender of person is not defined, no icon is displayed instead of neuter-icon (https://gitlab.com/champs-libres/departement-de-la-vendee/accent-suivi-developpement/-/issues/129). * [confidential information] module added to blur confidential information (https://gitlab.com/champs-libres/departement-de-la-vendee/chill/-/issues/248). +* refactor `AuthorizationHelper` and `UserACLAwareRepository` to fix constructor, and separate logic for parent role helper into `ParentRoleHelper` ## Test releases diff --git a/src/Bundle/ChillMainBundle/Repository/UserACLAwareRepository.php b/src/Bundle/ChillMainBundle/Repository/UserACLAwareRepository.php index 44399000b..de0ded2c2 100644 --- a/src/Bundle/ChillMainBundle/Repository/UserACLAwareRepository.php +++ b/src/Bundle/ChillMainBundle/Repository/UserACLAwareRepository.php @@ -6,15 +6,23 @@ use Chill\MainBundle\Entity\Center; use Chill\MainBundle\Entity\Scope; use Chill\MainBundle\Entity\User; use Chill\MainBundle\Security\Authorization\AuthorizationHelper; +use Chill\MainBundle\Security\ParentRoleHelper; +use Doctrine\ORM\EntityManagerInterface; class UserACLAwareRepository implements UserACLAwareRepositoryInterface { + private ParentRoleHelper $parentRoleHelper; + private EntityManagerInterface $em; - private AuthorizationHelper $authorizationHelper; + public function __construct(ParentRoleHelper $parentRoleHelper, EntityManagerInterface $em) + { + $this->parentRoleHelper = $parentRoleHelper; + $this->em = $em; + } public function findUsersByReachedACL(string $role, $center, $scope = null, bool $onlyEnabled = true): array { - $parents = $this->authorizationHelper->getParentRoles($role); + $parents = $this->parentRoleHelper->getParentRoles($role); $parents[] = $role; $qb = $this->em->createQueryBuilder(); diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php index 8713d6ce3..03adbdbc9 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php +++ b/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,40 +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 + ScopeResolverDispatcher $scopeResolverDispatcher, + 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; } /** @@ -91,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) { @@ -332,45 +321,32 @@ class AuthorizationHelper implements AuthorizationHelperInterface */ public function findUsersReaching(string $role, $center, $scope = null, bool $onlyEnabled = true): array { - return $this->userACLAwareRepository->findUsersByReachedACL($role, $center, $scope, $onlyEnabled); + return $this->userACLAwareRepository + ->findUsersByReachedACL($role, $center, $scope, $onlyEnabled); } /** * 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); } } diff --git a/src/Bundle/ChillMainBundle/Security/ParentRoleHelper.php b/src/Bundle/ChillMainBundle/Security/ParentRoleHelper.php new file mode 100644 index 000000000..07366dafa --- /dev/null +++ b/src/Bundle/ChillMainBundle/Security/ParentRoleHelper.php @@ -0,0 +1,71 @@ +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); + } + +} diff --git a/src/Bundle/ChillMainBundle/Tests/Authorization/ParentRoleHelperTest.php b/src/Bundle/ChillMainBundle/Tests/Authorization/ParentRoleHelperTest.php new file mode 100644 index 000000000..0d718ec09 --- /dev/null +++ b/src/Bundle/ChillMainBundle/Tests/Authorization/ParentRoleHelperTest.php @@ -0,0 +1,36 @@ +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')); + } +} diff --git a/src/Bundle/ChillMainBundle/config/services/security.yaml b/src/Bundle/ChillMainBundle/config/services/security.yaml index 15d6f7da5..dc7949556 100644 --- a/src/Bundle/ChillMainBundle/config/services/security.yaml +++ b/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'