From 74598ee926bd7cf653693b46c8bc74c8fd37016f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Fri, 17 Sep 2021 13:57:45 +0200 Subject: [PATCH] apply new role on accompanying period --- .../Form/Type/ScopePickerType.php | 9 +- .../Authorization/AuthorizationHelper.php | 23 +-- .../Authorization/DefaultVoterHelper.php | 2 +- .../DefaultVoterHelperGenerator.php | 2 +- .../Authorization/VoterGeneratorInterface.php | 2 +- .../Authorization/AuthorizationHelperTest.php | 157 +++++++++--------- .../config/services/security.yaml | 4 +- .../AccompanyingCourseController.php | 8 +- .../AccompanyingPeriodController.php | 31 ++-- .../ChillPersonExtension.php | 29 +++- .../Menu/PersonMenuBuilder.php | 27 ++- .../AccompanyingPeriodACLAwareRepository.php | 58 +++++++ .../AccompanyingPeriodRepository.php | 5 + .../Repository/PersonACLAwareRepository.php | 4 +- .../Authorization/AccompanyingPeriodVoter.php | 88 ++++++---- .../Security/Authorization/PersonVoter.php | 8 - .../config/services/controller.yaml | 5 +- .../config/services/menu.yaml | 11 +- .../config/services/security.yaml | 3 +- 19 files changed, 281 insertions(+), 195 deletions(-) create mode 100644 src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php diff --git a/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php b/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php index 832db628f..ebf474657 100644 --- a/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php +++ b/src/Bundle/ChillMainBundle/Form/Type/ScopePickerType.php @@ -146,14 +146,7 @@ class ScopePickerType extends AbstractType ->setParameter('center', $center->getId()) // role constraints ->andWhere($qb->expr()->in('rs.role', ':roles')) - ->setParameter( - 'roles', \array_map( - function (Role $role) { - return $role->getRole(); - }, - $roles - ) - ) + ->setParameter('roles', $roles) // user contraint ->andWhere(':user MEMBER OF gc.users') ->setParameter('user', $this->tokenStorage->getToken()->getUser()); diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php index ab1d1aed1..d7507f7a0 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php +++ b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php @@ -269,16 +269,12 @@ class AuthorizationHelper /** * - * @param Role $role - * @param Center $center - * @param Scope $circle - * @return Users + * @return User[] */ - public function findUsersReaching(Role $role, Center $center, Scope $circle = null) + public function findUsersReaching(string $role, Center $center, Scope $circle = null): array { $parents = $this->getParentRoles($role); $parents[] = $role; - $parentRolesString = \array_map(function(Role $r) { return $r->getRole(); }, $parents); $qb = $this->em->createQueryBuilder(); $qb @@ -288,7 +284,7 @@ class AuthorizationHelper ->join('gc.permissionsGroup', 'pg') ->join('pg.roleScopes', 'rs') ->where('gc.center = :center') - ->andWhere($qb->expr()->in('rs.role', $parentRolesString)) + ->andWhere($qb->expr()->in('rs.role', $parents)) ; $qb->setParameter('center', $center); @@ -322,21 +318,16 @@ class AuthorizationHelper * which are registered into Chill are taken into account. * * @param Role $role - * @return Role[] the role which give access to the given $role + * @return string[] the role which give access to the given $role */ - public function getParentRoles(Role $role) + public function getParentRoles($role): array { $parentRoles = []; // transform the roles from role hierarchy from string to Role - $roles = \array_map( - function($string) { - return new Role($string); - }, - \array_keys($this->hierarchy) - ); + $roles = \array_keys($this->hierarchy); foreach ($roles as $r) { - $childRoles = $this->roleHierarchy->getReachableRoleNames([$r->getRole()]); + $childRoles = $this->roleHierarchy->getReachableRoleNames([$r]); if (\in_array($role, $childRoles)) { $parentRoles[] = $r; diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterHelper.php b/src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterHelper.php index bd1e0e1cb..ef1d319ac 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterHelper.php +++ b/src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterHelper.php @@ -5,7 +5,7 @@ namespace Chill\MainBundle\Security\Authorization; use Chill\MainBundle\Entity\User; use Chill\MainBundle\Security\Resolver\CenterResolverDispatcher; -class DefaultVoterHelper implements VoterHelperInterface +final class DefaultVoterHelper implements VoterHelperInterface { protected AuthorizationHelper $authorizationHelper; diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterHelperGenerator.php b/src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterHelperGenerator.php index 753586270..c8300e45a 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterHelperGenerator.php +++ b/src/Bundle/ChillMainBundle/Security/Authorization/DefaultVoterHelperGenerator.php @@ -18,7 +18,7 @@ final class DefaultVoterHelperGenerator implements VoterGeneratorInterface $this->centerResolverDispatcher = $centerResolverDispatcher; } - public function addCheckFor($subject, $attributes): self + public function addCheckFor(?string $subject, array $attributes): self { $this->configuration[] = [$attributes, $subject]; diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/VoterGeneratorInterface.php b/src/Bundle/ChillMainBundle/Security/Authorization/VoterGeneratorInterface.php index 306427238..70e517536 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/VoterGeneratorInterface.php +++ b/src/Bundle/ChillMainBundle/Security/Authorization/VoterGeneratorInterface.php @@ -9,7 +9,7 @@ interface VoterGeneratorInterface * @param array $attributes an array of attributes * @return $this */ - public function addCheckFor(string $class, array $attributes): self; + public function addCheckFor(?string $class, array $attributes): self; public function build(): VoterHelperInterface; } diff --git a/src/Bundle/ChillMainBundle/Tests/Security/Authorization/AuthorizationHelperTest.php b/src/Bundle/ChillMainBundle/Tests/Security/Authorization/AuthorizationHelperTest.php index fbd3cd4d1..ae167adb5 100644 --- a/src/Bundle/ChillMainBundle/Tests/Security/Authorization/AuthorizationHelperTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Security/Authorization/AuthorizationHelperTest.php @@ -30,22 +30,22 @@ use Chill\MainBundle\Entity\Scope; use Chill\MainBundle\Entity\Center; /** - * + * * * @author Julien Fastré */ class AuthorizationHelperTest extends KernelTestCase { - + use PrepareUserTrait, PrepareCenterTrait, PrepareScopeTrait, ProphecyTrait; - - public function setUp() + + public function setUp() { static::bootKernel(); } - + /** - * + * * @return \Chill\MainBundle\Security\Authorization\AuthorizationHelper */ private function getAuthorizationHelper() @@ -54,13 +54,13 @@ class AuthorizationHelperTest extends KernelTestCase ->get('chill.main.security.authorization.helper') ; } - + /** * Test function userCanReach of helper. - * + * * A user can reach center => the function should return true. */ - public function testUserCanReachCenter_UserShouldReach() + public function testUserCanReachCenter_UserShouldReach() { $center = $this->prepareCenter(1, 'center'); $scope = $this->prepareScope(1, 'default'); @@ -72,16 +72,16 @@ class AuthorizationHelperTest extends KernelTestCase ) )); $helper = $this->getAuthorizationHelper(); - + $this->assertTrue($helper->userCanReachCenter($user, $center)); } - + /** * Test function userCanReach of helper - * + * * A user can not reachcenter =>W the function should return false */ - public function testUserCanReachCenter_UserShouldNotReach() + public function testUserCanReachCenter_UserShouldNotReach() { $centerA = $this->prepareCenter(1, 'center'); $centerB = $this->prepareCenter(2, 'centerB'); @@ -94,11 +94,11 @@ class AuthorizationHelperTest extends KernelTestCase ) )); $helper = $this->getAuthorizationHelper(); - + $this->assertFalse($helper->userCanReachCenter($user, $centerB)); - + } - + public function testUserHasAccess_shouldHaveAccess_EntityWithoutScope() { $center = $this->prepareCenter(1, 'center'); @@ -114,11 +114,11 @@ class AuthorizationHelperTest extends KernelTestCase $entity = $this->getProphet()->prophesize(); $entity->willImplement('\Chill\MainBundle\Entity\HasCenterInterface'); $entity->getCenter()->willReturn($center); - - $this->assertTrue($helper->userHasAccess($user, $entity->reveal(), + + $this->assertTrue($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); } - + public function testUserHasAccess_ShouldHaveAccessWithInheritance_EntityWithoutScope() { $center = $this->prepareCenter(1, 'center'); @@ -130,17 +130,17 @@ class AuthorizationHelperTest extends KernelTestCase ) ) )); - + $helper = $this->getAuthorizationHelper(); $entity = $this->getProphet()->prophesize(); $entity->willImplement('\Chill\MainBundle\Entity\HasCenterInterface'); $entity->getCenter()->willReturn($center); - - $this->assertTrue($helper->userHasAccess($user, $entity->reveal(), + + $this->assertTrue($helper->userHasAccess($user, $entity->reveal(), 'CHILL_INHERITED_ROLE_1')); } - - + + public function testuserHasAccess_UserHasNoRole_EntityWithoutScope() { $center = $this->prepareCenter(1, 'center'); @@ -156,10 +156,10 @@ class AuthorizationHelperTest extends KernelTestCase $entity = $this->getProphet()->prophesize(); $entity->willImplement('\Chill\MainBundle\Entity\HasCenterInterface'); $entity->getCenter()->willReturn($center); - + $this->assertFalse($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); } - + /** * test that a user has no access on a entity, but is granted on the same role * on another center @@ -186,10 +186,10 @@ class AuthorizationHelperTest extends KernelTestCase $entity = $this->getProphet()->prophesize(); $entity->willImplement('\Chill\MainBundle\Entity\HasCenterInterface'); $entity->getCenter()->willReturn($centerA); - + $this->assertFalse($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); } - + public function testtestUserHasAccess_UserShouldHaveAccess_EntityWithScope() { $center = $this->prepareCenter(1, 'center'); @@ -207,10 +207,10 @@ class AuthorizationHelperTest extends KernelTestCase $entity->willImplement('\Chill\MainBundle\Entity\HasScopeInterface'); $entity->getCenter()->willReturn($center); $entity->getScope()->willReturn($scope); - + $this->assertTrue($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); } - + public function testUserHasAccess_UserHasNoRole_EntityWithScope() { $center = $this->prepareCenter(1, 'center'); @@ -228,10 +228,10 @@ class AuthorizationHelperTest extends KernelTestCase $entity->willImplement('\Chill\MainBundle\Entity\HasScopeInterface'); $entity->getCenter()->willReturn($center); $entity->getScope()->willReturn($scope); - + $this->assertFalse($helper->userHasAccess($user, $entity->reveal(), 'ANOTHER_ROLE')); } - + public function testUserHasAccess_UserHasNoCenter_EntityWithScope() { $centerA = $this->prepareCenter(1, 'center'); //the user will have this center @@ -250,10 +250,10 @@ class AuthorizationHelperTest extends KernelTestCase $entity->willImplement('\Chill\MainBundle\Entity\HasScopeInterface'); $entity->getCenter()->willReturn($centerB); $entity->getScope()->willReturn($scope); - + $this->assertFalse($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); } - + public function testUserHasAccess_UserHasNoScope_EntityWithScope() { $center = $this->prepareCenter(1, 'center'); @@ -272,12 +272,12 @@ class AuthorizationHelperTest extends KernelTestCase $entity->willImplement('\Chill\MainBundle\Entity\HasScopeInterface'); $entity->getCenter()->willReturn($center); $entity->getScope()->willReturn($scopeA); - + $this->assertFalse($helper->userHasAccess($user, $entity->reveal(), 'CHILL_ROLE')); } - + /** - * + * * @dataProvider dataProvider_getReachableCenters * @param Center $shouldHaveCenter * @param User $user @@ -288,7 +288,7 @@ class AuthorizationHelperTest extends KernelTestCase { $this->assertEquals($test, $result, $msg); } - + public function dataProvider_getReachableCenters() { $this->setUp(); @@ -297,10 +297,10 @@ class AuthorizationHelperTest extends KernelTestCase $scopeA = $this->prepareScope(1, 'scope default'); $scopeB = $this->prepareScope(2, 'scope B'); $scopeC = $this->prepareScope(3, 'scope C'); - + $userA = $this->prepareUser(array( array( - 'center' => $centerA, + 'center' => $centerA, 'permissionsGroup' => array( ['scope' => $scopeB, 'role' => 'CHILL_ROLE_1'], ['scope' => $scopeA, 'role' => 'CHILL_ROLE_2'] @@ -313,62 +313,62 @@ class AuthorizationHelperTest extends KernelTestCase ['scope' => $scopeC, 'role' => 'CHILL_ROLE_2'] ) ) - + )); - + $ah = $this->getAuthorizationHelper(); - + return array( // without scopes array( - true, - in_array($centerA, $ah->getReachableCenters($userA, + true, + in_array($centerA, $ah->getReachableCenters($userA, new Role('CHILL_ROLE_1'), null)), 'center A should be available for userA, with role 1 ' ), array( - true, - in_array($centerA, $ah->getReachableCenters($userA, + true, + in_array($centerA, $ah->getReachableCenters($userA, new Role('CHILL_ROLE_2'), null)), 'center A should be available for userA, with role 2 ' ), array( - true, - in_array($centerB, $ah->getReachableCenters($userA, + true, + in_array($centerB, $ah->getReachableCenters($userA, new Role('CHILL_ROLE_2'), null)), 'center A should be available for userA, with role 2 ' ), array( - false, + false, in_array($centerB, $ah->getReachableCenters($userA, new Role('CHILL_ROLE_1'), null)), 'center B should NOT be available for userA, with role 1 ' ), // with scope array( - true, - in_array($centerA, $ah->getReachableCenters($userA, + true, + in_array($centerA, $ah->getReachableCenters($userA, new Role('CHILL_ROLE_1'), $scopeB)), 'center A should be available for userA, with role 1, scopeC ' ), array( - false, - in_array($centerA, $ah->getReachableCenters($userA, + false, + in_array($centerA, $ah->getReachableCenters($userA, new Role('CHILL_ROLE_2'), $scopeC)), 'center A should NOT be available for userA, with role 2, scopeA ' ), array( - true, - in_array($centerB, $ah->getReachableCenters($userA, + true, + in_array($centerB, $ah->getReachableCenters($userA, new Role('CHILL_ROLE_2'), $scopeA)), 'center B should be available for userA, with role 2, scopeA ' ), ); - + } - + /** - * + * * @dataProvider dataProvider_getReachableScopes * @param boolean $expectedResult * @param Scope $testedScope @@ -382,11 +382,11 @@ class AuthorizationHelperTest extends KernelTestCase { $reachableScopes = $this->getAuthorizationHelper() ->getReachableScopes($user, $role, $center); - + $this->assertEquals($expectedResult, in_array($testedScope, $reachableScopes), $message); } - + public function dataProvider_getReachableScopes() { $centerA = $this->prepareCenter(1, 'center A'); @@ -394,10 +394,10 @@ class AuthorizationHelperTest extends KernelTestCase $scopeA = $this->prepareScope(1, 'scope default'); $scopeB = $this->prepareScope(2, 'scope B'); $scopeC = $this->prepareScope(3, 'scope C'); - + $userA = $this->prepareUser(array( array( - 'center' => $centerA, + 'center' => $centerA, 'permissionsGroup' => array( ['scope' => $scopeB, 'role' => 'CHILL_ROLE_1'], ['scope' => $scopeA, 'role' => 'CHILL_ROLE_2'] @@ -411,9 +411,9 @@ class AuthorizationHelperTest extends KernelTestCase ['scope' => $scopeB, 'role' => 'CHILL_ROLE_2'] ) ) - + )); - + return array( array( true, @@ -442,37 +442,30 @@ class AuthorizationHelperTest extends KernelTestCase ) ); } - + public function testGetParentRoles() { $parentRoles = $this->getAuthorizationHelper() - ->getParentRoles(new Role('CHILL_INHERITED_ROLE_1')); - - $this->assertContains( - 'CHILL_MASTER_ROLE', - \array_map( - function(Role $role) { - return $role->getRole(); - }, - $parentRoles - ), + ->getParentRoles('CHILL_INHERITED_ROLE_1'); + + $this->assertContains('CHILL_MASTER_ROLE', $parentRoles, "Assert that `CHILL_MASTER_ROLE` is a parent of `CHILL_INHERITED_ROLE_1`"); } - + public function testFindUsersReaching() { $centerA = static::$kernel->getContainer() ->get('doctrine.orm.entity_manager') ->getRepository(Center::class) ->findOneByName('Center A'); - + $users = $this->getAuthorizationHelper() - ->findUsersReaching(new Role('CHILL_PERSON_SEE'), + ->findUsersReaching(new Role('CHILL_PERSON_SEE'), $centerA); - + $usernames = \array_map(function(User $u) { return $u->getUsername(); }, $users); - + $this->assertContains('center a_social', $usernames); } - + } diff --git a/src/Bundle/ChillMainBundle/config/services/security.yaml b/src/Bundle/ChillMainBundle/config/services/security.yaml index 7590acb5f..c9703dbfe 100644 --- a/src/Bundle/ChillMainBundle/config/services/security.yaml +++ b/src/Bundle/ChillMainBundle/config/services/security.yaml @@ -19,11 +19,11 @@ services: autowire: true # do not autowire the directory Security/Resolver - Chill\MainBundle\Security\Authorization\DefaultVoterFactory: + Chill\MainBundle\Security\Authorization\DefaultVoterHelperFactory: autowire: true # do not autowire the directory Security/Resolver - Chill\MainBundle\Security\Authorization\VoterFactoryInterface: '@Chill\MainBundle\Security\Authorization\DefaultVoterFactory' + Chill\MainBundle\Security\Authorization\VoterHelperFactoryInterface: '@Chill\MainBundle\Security\Authorization\DefaultVoterHelperFactory' chill.main.security.authorization.helper: class: Chill\MainBundle\Security\Authorization\AuthorizationHelper diff --git a/src/Bundle/ChillPersonBundle/Controller/AccompanyingCourseController.php b/src/Bundle/ChillPersonBundle/Controller/AccompanyingCourseController.php index 2f086c9f5..1bf40d146 100644 --- a/src/Bundle/ChillPersonBundle/Controller/AccompanyingCourseController.php +++ b/src/Bundle/ChillPersonBundle/Controller/AccompanyingCourseController.php @@ -73,7 +73,7 @@ class AccompanyingCourseController extends Controller } } - $this->denyAccessUnlessGranted(AccompanyingPeriodVoter::SEE, $period); + $this->denyAccessUnlessGranted(AccompanyingPeriodVoter::CREATE, $period); $em->persist($period); $em->flush(); @@ -92,6 +92,8 @@ class AccompanyingCourseController extends Controller */ public function indexAction(AccompanyingPeriod $accompanyingCourse): Response { + $this->denyAccessUnlessGranted(AccompanyingPeriodVoter::SEE, $accompanyingCourse); + // compute some warnings // get persons without household $withoutHousehold = []; @@ -131,6 +133,8 @@ class AccompanyingCourseController extends Controller */ public function editAction(AccompanyingPeriod $accompanyingCourse): Response { + $this->denyAccessUnlessGranted(AccompanyingPeriodVoter::SEE, $accompanyingCourse); + return $this->render('@ChillPerson/AccompanyingCourse/edit.html.twig', [ 'accompanyingCourse' => $accompanyingCourse ]); @@ -146,6 +150,8 @@ class AccompanyingCourseController extends Controller */ public function historyAction(AccompanyingPeriod $accompanyingCourse): Response { + $this->denyAccessUnlessGranted(AccompanyingPeriodVoter::SEE, $accompanyingCourse); + return $this->render('@ChillPerson/AccompanyingCourse/history.html.twig', [ 'accompanyingCourse' => $accompanyingCourse ]); diff --git a/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php b/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php index f31dfa585..9cba2ec1d 100644 --- a/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php +++ b/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php @@ -23,7 +23,10 @@ namespace Chill\PersonBundle\Controller; use Chill\PersonBundle\Privacy\PrivacyEvent; +use Chill\PersonBundle\Repository\AccompanyingPeriodACLAwareRepository; +use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; use Doctrine\DBAL\Exception; +use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Chill\PersonBundle\Entity\Person; use Chill\PersonBundle\Form\AccompanyingPeriodType; @@ -53,21 +56,24 @@ class AccompanyingPeriodController extends AbstractController */ protected $validator; - /** - * AccompanyingPeriodController constructor. - * - * @param EventDispatcherInterface $eventDispatcher - * @param ValidatorInterface $validator - */ - public function __construct(EventDispatcherInterface $eventDispatcher, ValidatorInterface $validator) - { + protected AccompanyingPeriodACLAwareRepository $accompanyingPeriodACLAwareRepository; + + public function __construct( + AccompanyingPeriodACLAwareRepository $accompanyingPeriodACLAwareRepository, + EventDispatcherInterface $eventDispatcher, + ValidatorInterface $validator + ) { + $this->accompanyingPeriodACLAwareRepository = $accompanyingPeriodACLAwareRepository; $this->eventDispatcher = $eventDispatcher; $this->validator = $validator; } - public function listAction(int $person_id): Response + /** + * @ParamConverter("person", options={"id"="person_id"}) + */ + public function listAction(Person $person): Response { - $person = $this->_getPerson($person_id); + $this->denyAccessUnlessGranted(AccompanyingPeriodVoter::SEE, $person); $event = new PrivacyEvent($person, [ 'element_class' => AccompanyingPeriod::class, @@ -75,9 +81,10 @@ class AccompanyingPeriodController extends AbstractController ]); $this->eventDispatcher->dispatch(PrivacyEvent::PERSON_PRIVACY_EVENT, $event); - $accompanyingPeriods = $person->getAccompanyingPeriodsOrdered(); + $accompanyingPeriods = $this->accompanyingPeriodACLAwareRepository + ->findByPerson($person, AccompanyingPeriodVoter::SEE); - return $this->render('ChillPersonBundle:AccompanyingPeriod:list.html.twig', [ + return $this->render('@ChillPerson/AccompanyingPeriod/list.html.twig', [ 'accompanying_periods' => $accompanyingPeriods, 'person' => $person ]); diff --git a/src/Bundle/ChillPersonBundle/DependencyInjection/ChillPersonExtension.php b/src/Bundle/ChillPersonBundle/DependencyInjection/ChillPersonExtension.php index 48ac993c3..485185ce7 100644 --- a/src/Bundle/ChillPersonBundle/DependencyInjection/ChillPersonExtension.php +++ b/src/Bundle/ChillPersonBundle/DependencyInjection/ChillPersonExtension.php @@ -18,6 +18,7 @@ namespace Chill\PersonBundle\DependencyInjection; +use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\Config\FileLocator; use Symfony\Component\HttpKernel\DependencyInjection\Extension; @@ -258,14 +259,26 @@ class ChillPersonExtension extends Extension implements PrependExtensionInterfac */ protected function prependRoleHierarchy(ContainerBuilder $container) { - $container->prependExtensionConfig('security', array( - 'role_hierarchy' => array( - 'CHILL_PERSON_UPDATE' => array('CHILL_PERSON_SEE'), - 'CHILL_PERSON_CREATE' => array('CHILL_PERSON_SEE'), - PersonVoter::LISTS => [ ChillExportVoter::EXPORT ], - PersonVoter::STATS => [ ChillExportVoter::EXPORT ] - ) - )); + $container->prependExtensionConfig('security', [ + 'role_hierarchy' => [ + PersonVoter::UPDATE => [PersonVoter::SEE], + PersonVoter::CREATE => [PersonVoter::SEE], + PersonVoter::LISTS => [ChillExportVoter::EXPORT], + PersonVoter::STATS => [ChillExportVoter::EXPORT], + // accompanying period + AccompanyingPeriodVoter::SEE_DETAILS => [AccompanyingPeriodVoter::SEE], + AccompanyingPeriodVoter::CREATE => [AccompanyingPeriodVoter::SEE_DETAILS], + AccompanyingPeriodVoter::DELETE => [AccompanyingPeriodVoter::SEE_DETAILS], + AccompanyingPeriodVoter::EDIT => [AccompanyingPeriodVoter::SEE_DETAILS], + // give all ACL for FULL + AccompanyingPeriodVoter::FULL => [ + AccompanyingPeriodVoter::SEE_DETAILS, + AccompanyingPeriodVoter::CREATE, + AccompanyingPeriodVoter::EDIT, + AccompanyingPeriodVoter::DELETE + ] + ] + ]); } /** diff --git a/src/Bundle/ChillPersonBundle/Menu/PersonMenuBuilder.php b/src/Bundle/ChillPersonBundle/Menu/PersonMenuBuilder.php index fdb8feb3e..0c83d8a6a 100644 --- a/src/Bundle/ChillPersonBundle/Menu/PersonMenuBuilder.php +++ b/src/Bundle/ChillPersonBundle/Menu/PersonMenuBuilder.php @@ -18,14 +18,17 @@ namespace Chill\PersonBundle\Menu; use Chill\MainBundle\Routing\LocalMenuBuilderInterface; +use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; use Knp\Menu\MenuItem; +use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface; +use Symfony\Component\Security\Core\Security; use Symfony\Contracts\Translation\TranslatorInterface; /** * Add menu entrie to person menu. - * + * * Menu entries added : - * + * * - person details ; * - accompanying period (if `visible`) * @@ -37,21 +40,25 @@ class PersonMenuBuilder implements LocalMenuBuilderInterface * @var string 'visible' or 'hidden' */ protected $showAccompanyingPeriod; - + /** * * @var TranslatorInterface */ protected $translator; - + + private Security $security; + public function __construct( - $showAccompanyingPeriod, + ParameterBagInterface $parameterBag, + Security $security, TranslatorInterface $translator ) { - $this->showAccompanyingPeriod = $showAccompanyingPeriod; + $this->showAccompanyingPeriod = $parameterBag->get('chill_person.accompanying_period'); + $this->security = $security; $this->translator = $translator; } - + public function buildMenu($menuId, MenuItem $menu, array $parameters) { $menu->addChild($this->translator->trans('Person details'), [ @@ -83,8 +90,10 @@ class PersonMenuBuilder implements LocalMenuBuilderInterface ->setExtras([ 'order' => 99999 ]); - - if ($this->showAccompanyingPeriod === 'visible') { + + if ($this->showAccompanyingPeriod === 'visible' + && $this->security->isGranted(AccompanyingPeriodVoter::SEE, $parameters['person']) + ) { $menu->addChild($this->translator->trans('Accompanying period list'), [ 'route' => 'chill_person_accompanying_period_list', 'routeParameters' => [ diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php new file mode 100644 index 000000000..cdc439e32 --- /dev/null +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php @@ -0,0 +1,58 @@ +accompanyingPeriodRepository = $accompanyingPeriodRepository; + $this->security = $security; + $this->authorizationHelper = $authorizationHelper; + $this->centerResolverDispatcher = $centerResolverDispatcher; + } + + public function findByPerson( + Person $person, + string $role, + ?array $orderBy = [], + int $limit = null, + int $offset = null + ): array { + dump(__METHOD__); + $qb = $this->accompanyingPeriodRepository->createQueryBuilder('ap'); + $scopes = $this->authorizationHelper + ->getReachableCircles($this->security->getUser(), $role, + $this->centerResolverDispatcher->resolveCenter($person)); + + if (0 === count($scopes)) { + return []; + } + + $qb + ->join('ap.participations', 'participation') + ->where($qb->expr()->eq('participation.person', ':person')) + ->setParameter('person', $person) + ; + // add join condition for scopes + $orx = $qb->expr()->orX(); + foreach ($scopes as $key => $scope) { + $orx->add($qb->expr()->in('ap.scopes', ':scope_'.$key)); + $qb->setParameter('scope_'.$key, $scope); + } + $qb->andWhere($orx); + + return $qb->getQuery()->getResult(); + } + +} diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodRepository.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodRepository.php index f9d1b060f..c9e8397a8 100644 --- a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodRepository.php +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodRepository.php @@ -59,6 +59,11 @@ final class AccompanyingPeriodRepository implements ObjectRepository return $this->findOneBy($criteria); } + public function createQueryBuilder(string $alias, ?string $indexBy = null): QueryBuilder + { + return $this->repository->createQueryBuilder($alias, $indexBy); + } + public function getClassName() { return AccompanyingPeriod::class; diff --git a/src/Bundle/ChillPersonBundle/Repository/PersonACLAwareRepository.php b/src/Bundle/ChillPersonBundle/Repository/PersonACLAwareRepository.php index 5214b38b7..d15e8cd7a 100644 --- a/src/Bundle/ChillPersonBundle/Repository/PersonACLAwareRepository.php +++ b/src/Bundle/ChillPersonBundle/Repository/PersonACLAwareRepository.php @@ -60,7 +60,7 @@ final class PersonACLAwareRepository implements PersonACLAwareRepositoryInterfac $countryCode); $this->addACLClauses($qb, 'p'); - return $this->getQueryResult($qb, $simplify, $limit, $start); + return $this->getQueryResult($qb, 'p', $simplify, $limit, $start); } /** @@ -119,7 +119,7 @@ final class PersonACLAwareRepository implements PersonACLAwareRepositoryInterfac $countryCode); $this->addACLClauses($qb, 'p'); - return $this->getCountQueryResult($qb); + return $this->getCountQueryResult($qb,'p'); } /** diff --git a/src/Bundle/ChillPersonBundle/Security/Authorization/AccompanyingPeriodVoter.php b/src/Bundle/ChillPersonBundle/Security/Authorization/AccompanyingPeriodVoter.php index 2801cb8f7..5247df505 100644 --- a/src/Bundle/ChillPersonBundle/Security/Authorization/AccompanyingPeriodVoter.php +++ b/src/Bundle/ChillPersonBundle/Security/Authorization/AccompanyingPeriodVoter.php @@ -4,73 +4,101 @@ namespace Chill\PersonBundle\Security\Authorization; use Chill\MainBundle\Security\Authorization\AbstractChillVoter; use Chill\MainBundle\Entity\User; -use Chill\MainBundle\Security\Authorization\AuthorizationHelper; +use Chill\MainBundle\Security\Authorization\VoterHelperFactoryInterface; +use Chill\MainBundle\Security\Authorization\VoterHelperInterface; use Chill\MainBundle\Security\ProvideRoleHierarchyInterface; use Chill\PersonBundle\Entity\Person; use Chill\PersonBundle\Entity\AccompanyingPeriod; -use Chill\MainBundle\Entity\Center; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; -use Symfony\Component\Security\Core\Role\Role; +use Symfony\Component\Security\Core\Security; class AccompanyingPeriodVoter extends AbstractChillVoter implements ProvideRoleHierarchyInterface { - protected AuthorizationHelper $helper; - public const SEE = 'CHILL_PERSON_ACCOMPANYING_PERIOD_SEE'; + /** + * details are for seeing: + * + * * SocialIssues + */ + public const SEE_DETAILS = 'CHILL_PERSON_ACCOMPANYING_PERIOD_SEE_DETAILS'; + public const CREATE = 'CHILL_PERSON_ACCOMPANYING_PERIOD_CREATE'; + public const EDIT = 'CHILL_PERSON_ACCOMPANYING_PERIOD_UPDATE'; + public const DELETE = 'CHILL_PERSON_ACCOMPANYING_PERIOD_DELETE'; /** - * @param AuthorizationHelper $helper + * Give all the right above */ - public function __construct(AuthorizationHelper $helper) - { - $this->helper = $helper; + public const FULL = 'CHILL_PERSON_ACCOMPANYING_PERIOD_FULL'; + + public const ALL = [ + self::SEE, + self::SEE_DETAILS, + self::CREATE, + self::EDIT, + self::DELETE, + self::FULL, + ]; + + private VoterHelperInterface $voterHelper; + + private Security $security; + + public function __construct( + Security $security, + VoterHelperFactoryInterface $voterHelperFactory + ) { + $this->security = $security; + $this->voterHelper = $voterHelperFactory + ->generate(self::class) + ->addCheckFor(null, [self::CREATE]) + ->addCheckFor(AccompanyingPeriod::class, self::ALL) + ->addCheckFor(Person::class, [self::SEE]) + ->build(); } protected function supports($attribute, $subject) { - return $subject instanceof AccompanyingPeriod; + return $this->voterHelper->supports($attribute, $subject); } - protected function voteOnAttribute($attribute, $subject, TokenInterface $token) + protected function voteOnAttribute($attribute, $subject, TokenInterface $token): bool { if (!$token->getUser() instanceof User) { return false; } - // TODO take scopes into account - if (count($subject->getPersons()) === 0) { - return true; - } - foreach ($subject->getPersons() as $person) { - // give access as soon as on center is reachable - if ($this->helper->userHasAccess($token->getUser(), $person->getCenter(), $attribute)) { - return true; + if ($subject instanceof AccompanyingPeriod) { + if (AccompanyingPeriod::STEP_DRAFT === $subject->getStep()) { + // only creator can see, edit, delete, etc. + if ($subject->getCreatedBy() === $token->getUser() + || NULL === $subject->getCreatedBy()) { + return true; + } + + return false; } - return false; + // if confidential, only the referent can see it + if ($subject->isConfidential()) { + return $token->getUser() === $subject->getUser(); + } } - } - private function getAttributes() - { - return [ - self::SEE - ]; + return $this->voterHelper->voteOnAttribute($attribute, $subject, $token); } public function getRoles() { - return $this->getAttributes(); + return self::ALL; } public function getRolesWithoutScope() { return []; } - + public function getRolesWithHierarchy() { - return [ 'Person' => $this->getRoles() ]; + return [ 'Accompanying period' => $this->getRoles() ]; } - } diff --git a/src/Bundle/ChillPersonBundle/Security/Authorization/PersonVoter.php b/src/Bundle/ChillPersonBundle/Security/Authorization/PersonVoter.php index 6338f5f45..2d585e80e 100644 --- a/src/Bundle/ChillPersonBundle/Security/Authorization/PersonVoter.php +++ b/src/Bundle/ChillPersonBundle/Security/Authorization/PersonVoter.php @@ -40,19 +40,11 @@ class PersonVoter extends AbstractChillVoter implements ProvideRoleHierarchyInte const LISTS = 'CHILL_PERSON_LISTS'; const DUPLICATE = 'CHILL_PERSON_DUPLICATE'; - protected AuthorizationHelper $helper; - - protected CenterResolverDispatcher $centerResolverDispatcher; - protected VoterHelperInterface $voter; public function __construct( - AuthorizationHelper $helper, - CenterResolverDispatcher $centerResolverDispatcher, VoterHelperFactoryInterface $voterFactory ) { - $this->helper = $helper; - $this->centerResolverDispatcher = $centerResolverDispatcher; $this->voter = $voterFactory ->generate(self::class) ->addCheckFor(Center::class, [self::STATS, self::LISTS, self::DUPLICATE]) diff --git a/src/Bundle/ChillPersonBundle/config/services/controller.yaml b/src/Bundle/ChillPersonBundle/config/services/controller.yaml index 6efdb4640..489168425 100644 --- a/src/Bundle/ChillPersonBundle/config/services/controller.yaml +++ b/src/Bundle/ChillPersonBundle/config/services/controller.yaml @@ -12,9 +12,8 @@ services: tags: ['controller.service_arguments'] Chill\PersonBundle\Controller\AccompanyingPeriodController: - arguments: - $eventDispatcher: '@Symfony\Component\EventDispatcher\EventDispatcherInterface' - $validator: '@Symfony\Component\Validator\Validator\ValidatorInterface' + autowire: true + autoconfigure: true tags: ['controller.service_arguments'] Chill\PersonBundle\Controller\PersonAddressController: diff --git a/src/Bundle/ChillPersonBundle/config/services/menu.yaml b/src/Bundle/ChillPersonBundle/config/services/menu.yaml index 344eb2a17..a9f566466 100644 --- a/src/Bundle/ChillPersonBundle/config/services/menu.yaml +++ b/src/Bundle/ChillPersonBundle/config/services/menu.yaml @@ -19,14 +19,7 @@ services: # - { name: 'chill.menu_builder' } # Chill\PersonBundle\Menu\PersonMenuBuilder: - arguments: - $showAccompanyingPeriod: '%chill_person.accompanying_period%' - $translator: '@Symfony\Contracts\Translation\TranslatorInterface' + autowire: true + autoconfigure: true tags: - { name: 'chill.menu_builder' } - - # Chill\PersonBundle\Menu\AccompanyingCourseMenuBuilder: - # arguments: - # $translator: '@Symfony\Contracts\Translation\TranslatorInterface' - # tags: - # - { name: 'chill.menu_builder' } diff --git a/src/Bundle/ChillPersonBundle/config/services/security.yaml b/src/Bundle/ChillPersonBundle/config/services/security.yaml index 8504874a3..6dc3c92d3 100644 --- a/src/Bundle/ChillPersonBundle/config/services/security.yaml +++ b/src/Bundle/ChillPersonBundle/config/services/security.yaml @@ -7,8 +7,7 @@ services: - { name: chill.role } Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter: - arguments: - - "@chill.main.security.authorization.helper" + autowire: true tags: - { name: security.voter } - { name: chill.role }