diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php index 1105c9d8a..be884de94 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php +++ b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelper.php @@ -195,10 +195,6 @@ class AuthorizationHelper implements AuthorizationHelperInterface /** * Return all reachable scope for a given user, center and role. - * - * @param Center|Center[] $center - * - * @return array|Scope[] */ public function getReachableScopes(UserInterface $user, string $role, Center|array $center): array { diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelperForCurrentUserInterface.php b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelperForCurrentUserInterface.php index f0d3f9fba..54e30c244 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelperForCurrentUserInterface.php +++ b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelperForCurrentUserInterface.php @@ -25,7 +25,8 @@ interface AuthorizationHelperForCurrentUserInterface public function getReachableCenters(string $role, ?Scope $scope = null): array; /** - * @param array|Center|Center[] $center + * @param list
|Center $center + * @return list */ - public function getReachableScopes(string $role, $center): array; + public function getReachableScopes(string $role, array|Center $center): array; } diff --git a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelperInterface.php b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelperInterface.php index 1176cf1fa..1dc9668ec 100644 --- a/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelperInterface.php +++ b/src/Bundle/ChillMainBundle/Security/Authorization/AuthorizationHelperInterface.php @@ -26,7 +26,8 @@ interface AuthorizationHelperInterface public function getReachableCenters(UserInterface $user, string $role, ?Scope $scope = null): array; /** - * @param Center|list
$center + * @param Center|array
$center + * @return list */ public function getReachableScopes(UserInterface $user, string $role, Center|array $center): array; } diff --git a/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php b/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php index b32454387..8b4c0b27a 100644 --- a/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php +++ b/src/Bundle/ChillPersonBundle/Controller/AccompanyingPeriodController.php @@ -219,13 +219,13 @@ class AccompanyingPeriodController extends AbstractController ]); $this->eventDispatcher->dispatch(PrivacyEvent::PERSON_PRIVACY_EVENT, $event); - $accompanyingPeriodsRaw = $this->accompanyingPeriodACLAwareRepository - ->findByPerson($person, AccompanyingPeriodVoter::SEE); + $accompanyingPeriods = $this->accompanyingPeriodACLAwareRepository + ->findByPerson($person, AccompanyingPeriodVoter::SEE, ["openingDate" => "DESC", "id" => "DESC"]); - usort($accompanyingPeriodsRaw, static fn ($a, $b) => $b->getOpeningDate() > $a->getOpeningDate()); + //usort($accompanyingPeriodsRaw, static fn ($a, $b) => $b->getOpeningDate() <=> $a->getOpeningDate()); // filter visible or not visible - $accompanyingPeriods = array_filter($accompanyingPeriodsRaw, fn (AccompanyingPeriod $ap) => $this->isGranted(AccompanyingPeriodVoter::SEE, $ap)); + //$accompanyingPeriods = array_filter($accompanyingPeriodsRaw, fn (AccompanyingPeriod $ap) => $this->isGranted(AccompanyingPeriodVoter::SEE, $ap)); return $this->render('@ChillPerson/AccompanyingPeriod/list.html.twig', [ 'accompanying_periods' => $accompanyingPeriods, diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php index 0aaabd05f..104f49ec7 100644 --- a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepository.php @@ -13,54 +13,51 @@ namespace Chill\PersonBundle\Repository; use Chill\MainBundle\Entity\Address; use Chill\MainBundle\Entity\Location; -use Chill\MainBundle\Entity\PostalCode; use Chill\MainBundle\Entity\Scope; use Chill\MainBundle\Entity\User; use Chill\MainBundle\Entity\UserJob; -use Chill\MainBundle\Security\Authorization\AuthorizationHelper; -use Chill\MainBundle\Security\Resolver\CenterResolverDispatcherInterface; +use Chill\MainBundle\Security\Authorization\AuthorizationHelperForCurrentUserInterface; +use Chill\MainBundle\Security\Resolver\CenterResolverManagerInterface; use Chill\PersonBundle\Entity\AccompanyingPeriod; use Chill\PersonBundle\Entity\AccompanyingPeriodParticipation; use Chill\PersonBundle\Entity\Household\PersonHouseholdAddress; use Chill\PersonBundle\Entity\Person; use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter; use DateTime; - use DateTimeImmutable; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Query\Expr\Join; use Doctrine\ORM\QueryBuilder; +use Repository\AccompanyingPeriodACLAwareRepositoryTest; use Symfony\Component\Security\Core\Security; use function count; -final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodACLAwareRepositoryInterface +/** + * @see AccompanyingPeriodACLAwareRepositoryTest + */ +final readonly class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodACLAwareRepositoryInterface { private AccompanyingPeriodRepository $accompanyingPeriodRepository; - private AuthorizationHelper $authorizationHelper; + private AuthorizationHelperForCurrentUserInterface $authorizationHelper; - private CenterResolverDispatcherInterface $centerResolverDispatcher; + private CenterResolverManagerInterface $centerResolver; private Security $security; public function __construct( AccompanyingPeriodRepository $accompanyingPeriodRepository, Security $security, - AuthorizationHelper $authorizationHelper, - CenterResolverDispatcherInterface $centerResolverDispatcher + AuthorizationHelperForCurrentUserInterface $authorizationHelper, + CenterResolverManagerInterface $centerResolverDispatcher ) { $this->accompanyingPeriodRepository = $accompanyingPeriodRepository; $this->security = $security; $this->authorizationHelper = $authorizationHelper; - $this->centerResolverDispatcher = $centerResolverDispatcher; + $this->centerResolver = $centerResolverDispatcher; } - /** - * @param array|PostalCode[] - * - * @return QueryBuilder - */ - public function buildQueryOpenedAccompanyingCourseByUser(?User $user, array $postalCodes = []) + public function buildQueryOpenedAccompanyingCourseByUser(?User $user, array $postalCodes = []): QueryBuilder { $qb = $this->accompanyingPeriodRepository->createQueryBuilder('ap'); @@ -152,10 +149,14 @@ final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodAC ): array { $qb = $this->accompanyingPeriodRepository->createQueryBuilder('ap'); $scopes = $this->authorizationHelper - ->getReachableCircles( - $this->security->getUser(), + ->getReachableScopes( $role, - $this->centerResolverDispatcher->resolveCenter($person) + $this->centerResolver->resolveCenters($person) + ); + $scopesCanSeeConfidential = $this->authorizationHelper + ->getReachableScopes( + AccompanyingPeriodVoter::SEE_CONFIDENTIAL_ALL, + $this->centerResolver->resolveCenters($person) ); if (0 === count($scopes)) { @@ -165,12 +166,42 @@ final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodAC $qb ->join('ap.participations', 'participation') ->where($qb->expr()->eq('participation.person', ':person')) - ->andWhere( - $qb->expr()->orX( - 'ap.confidential = FALSE', - $qb->expr()->eq('ap.user', ':user') - ) - ) + ->setParameter('person', $person); + + $qb = $this->addACLClauses($qb, $scopes, $scopesCanSeeConfidential); + $qb = $this->addOrderLimitClauses($qb, $orderBy, $limit, $offset); + + return $qb->getQuery()->getResult(); + } + + public function addOrderLimitClauses(QueryBuilder $qb, ?array $orderBy = null, ?int $limit = null, ?int $offset = null): QueryBuilder + { + if (null !== $orderBy) { + foreach ($orderBy as $field => $order) { + $qb->addOrderBy('ap.' . $field, $order); + } + } + + if (null !== $limit) { + $qb->setMaxResults($limit); + } + + if (null !== $offset) { + $qb->setFirstResult($offset); + } + + return $qb; + } + + /** + * @param QueryBuilder $qb where the accompanying period have the `ap` alias + * @param array $scopesCanSee + * @param array $scopesCanSeeConfidential + * @return QueryBuilder + */ + public function addACLClauses(QueryBuilder $qb, array $scopesCanSee, array $scopesCanSeeConfidential): QueryBuilder + { + $qb ->andWhere( $qb->expr()->orX( $qb->expr()->neq('ap.step', ':draft'), @@ -181,25 +212,44 @@ final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodAC ) ) ->setParameter('draft', AccompanyingPeriod::STEP_DRAFT) - ->setParameter('person', $person) ->setParameter('user', $this->security->getUser()) ->setParameter('creator', $this->security->getUser()); + // add join condition for scopes $orx = $qb->expr()->orX( + // even if the scope is not in one authorized, the user can see the course if it is in DRAFT state $qb->expr()->eq('ap.step', ':draft') ); - foreach ($scopes as $key => $scope) { - $orx->add($qb->expr()->orX( + foreach ($scopesCanSee as $key => $scope) { + // for each scope: + // - either the user is the referrer of the course + // - or the accompanying course is one of the reachable scopes + // - and the parcours is not confidential OR the user is the referrer OR the user can see the confidential course + + $orOnScope = $qb->expr()->orX( $qb->expr()->isMemberOf(':scope_' . $key, 'ap.scopes'), $qb->expr()->eq('ap.user', ':user') - )); + ); + + if (in_array($scope, $scopesCanSeeConfidential, true)) { + $orx->add($orOnScope); + } else { + // we must add a condition: the course is not confidential or the user is the referrer + $andXOnScope = $qb->expr()->andX( + $orOnScope, + $qb->expr()->orX( + 'ap.confidential = FALSE', + $qb->expr()->eq('ap.user', ':user') + ) + ); + $orx->add($andXOnScope); + } $qb->setParameter('scope_' . $key, $scope); - $qb->setParameter('user', $this->security->getUser()); } $qb->andWhere($orx); - return $qb->getQuery()->getResult(); + return $qb; } public function findByUnDispatched(array $jobs, array $services, array $administrativeLocations, ?int $limit = null, ?int $offset = null): array @@ -237,9 +287,6 @@ final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodAC return $qb->getQuery()->getResult(); } - /** - * @return array|AccompanyingPeriod[] - */ public function findByUserOpenedAccompanyingPeriod(?User $user, array $orderBy = [], int $limit = 0, int $offset = 50): array { if (null === $user) { @@ -261,7 +308,6 @@ final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodAC private function addACLByUnDispatched(QueryBuilder $qb): QueryBuilder { $centers = $this->authorizationHelper->getReachableCenters( - $this->security->getUser(), AccompanyingPeriodVoter::SEE ); @@ -273,8 +319,7 @@ final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodAC foreach ($centers as $key => $center) { $scopes = $this->authorizationHelper - ->getReachableCircles( - $this->security->getUser(), + ->getReachableScopes( AccompanyingPeriodVoter::SEE, $center ); diff --git a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php index 0cca1a5f4..ff3d89783 100644 --- a/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php +++ b/src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php @@ -33,6 +33,9 @@ interface AccompanyingPeriodACLAwareRepositoryInterface public function countByUserOpenedAccompanyingPeriod(?User $user): int; + /** + * @return array + */ public function findByPerson( Person $person, string $role, @@ -45,14 +48,19 @@ interface AccompanyingPeriodACLAwareRepositoryInterface * @param array|UserJob[] $jobs if empty, does not take this argument into account * @param array|Scope[] $services if empty, does not take this argument into account * - * @return array|AccompanyingPeriod[] + * @return list */ public function findByUnDispatched(array $jobs, array $services, array $administrativeLocations, ?int $limit = null, ?int $offset = null): array; /** * @param array|PostalCode[] $postalCodes + * @return list */ public function findByUserAndPostalCodesOpenedAccompanyingPeriod(?User $user, array $postalCodes, array $orderBy = [], int $limit = 0, int $offset = 50): array; + /** + * @deprecated + * @return list + */ public function findByUserOpenedAccompanyingPeriod(?User $user, array $orderBy = [], int $limit = 0, int $offset = 50): array; } diff --git a/src/Bundle/ChillPersonBundle/Tests/Repository/AccompanyingPeriodACLAwareRepositoryTest.php b/src/Bundle/ChillPersonBundle/Tests/Repository/AccompanyingPeriodACLAwareRepositoryTest.php new file mode 100644 index 000000000..17c12fea5 --- /dev/null +++ b/src/Bundle/ChillPersonBundle/Tests/Repository/AccompanyingPeriodACLAwareRepositoryTest.php @@ -0,0 +1,206 @@ +accompanyingPeriodRepository = self::$container->get(AccompanyingPeriodRepository::class); + $this->centerResolverManager = self::$container->get(CenterResolverManagerInterface::class); + $this->entityManager = self::$container->get(EntityManagerInterface::class); + $this->scopeRepository = self::$container->get(ScopeRepositoryInterface::class); + $this->registry = self::$container->get(Registry::class); + + } + + public static function tearDownAfterClass(): void + { + self::bootKernel(); + $em = self::$container->get(EntityManagerInterface::class); + $repository = self::$container->get(AccompanyingPeriodRepository::class); + + foreach (self::$periodsIdsToDelete as $id) { + if (null === $period = $repository->find($id)) { + throw new \RuntimeException("period not found while trying to delete it"); + } + + foreach ($period->getParticipations() as $participation) { + $em->remove($participation); + } + $em->remove($period); + } + + $em->flush(); + } + + + /** + * For testing this method, we mock the authorization helper to return different Scope that a user + * can see, or that a user can see confidential periods. + * + * @param array $scopeUserCanSee + * @param array $scopeUserCanSeeConfidential + * @param array $expectedPeriod + * @dataProvider provideDataForFindByPerson + */ + public function testFindByPersonTestUser(User $user, Person $person, array $scopeUserCanSee, array $scopeUserCanSeeConfidential, array $expectedPeriod, string $message): void + { + $security = $this->prophesize(Security::class); + $security->getUser()->willReturn($user); + + $authorizationHelper = $this->prophesize(AuthorizationHelperForCurrentUserInterface::class); + $authorizationHelper->getReachableScopes(AccompanyingPeriodVoter::SEE, Argument::any()) + ->willReturn($scopeUserCanSee); + $authorizationHelper->getReachableScopes(AccompanyingPeriodVoter::SEE_CONFIDENTIAL_ALL, Argument::any()) + ->willReturn($scopeUserCanSeeConfidential); + + $repository = new AccompanyingPeriodACLAwareRepository( + $this->accompanyingPeriodRepository, + $security->reveal(), + $authorizationHelper->reveal(), + $this->centerResolverManager + ); + + $actuals = $repository->findByPerson($person, AccompanyingPeriodVoter::SEE); + $expectedIds = array_map(fn (AccompanyingPeriod $period) => $period->getId(), $expectedPeriod); + + self::assertCount(count($expectedPeriod), $actuals, $message); + foreach ($actuals as $actual) { + self::assertContains($actual->getId(), $expectedIds); + } + } + + public function provideDataForFindByPerson(): iterable + { + $this->setUp(); + + if (null === $user = $this->entityManager->createQuery("SELECT u FROM " . User::class . " u")->setMaxResults(1)->getSingleResult()) { + throw new \RuntimeException("no user found"); + } + + if (null === $anotherUser = $this->entityManager->createQuery("SELECT u FROM " . User::class . " u WHERE u.id != :uid")->setParameter('uid', $user->getId()) + ->setMaxResults(1)->getSingleResult()) { + throw new \RuntimeException("no user found"); + } + + [$person, $anotherPerson, $person2, $person3] = $this->entityManager + ->createQuery("SELECT p FROM " . Person::class . " p WHERE SIZE(p.accompanyingPeriodParticipations) = 0") + ->setMaxResults(4) + ->getResult(); + + if (null === $person || null === $anotherPerson || null === $person2 || null === $person3) { + throw new \RuntimeException("no person found"); + } + + $scopes = $this->scopeRepository->findAll(); + + if (3 > count($scopes)) { + throw new \RuntimeException("not enough scopes for this test"); + } + $scopesCanSee = [ $scopes[0] ]; + $scopesGroup2 = [ $scopes[1] ]; + + // case: a period is in draft state + $period = $this->buildPeriod($person, $scopesCanSee, $user, false); + + yield [$user, $person, $scopesCanSee, [], [$period], "a user can see his period during draft state"]; + + // another user is not allowed to see this period, because it is in DRAFT state + yield [$anotherUser, $person, $scopesCanSee, [], [], "another user is not allowed to see the period of someone else in draft state"]; + + // the period is confirmed + $period = $this->buildPeriod($anotherPerson, $scopesCanSee, $user, true); + + // the other user can now see it + yield [$user, $anotherPerson, $scopesCanSee, [], [$period], "a user see his period when confirmed"]; + yield [$anotherUser, $anotherPerson, $scopesCanSee, [], [$period], "another user with required scopes is allowed to see the period when not draft"]; + yield [$anotherUser, $anotherPerson, $scopesGroup2, [], [], "another user without the required scopes is not allowed to see the period when not draft"]; + + // this period will be confidential + $period = $this->buildPeriod($person2, $scopesCanSee, $user, true); + $period->setConfidential(true)->setUser($user, true); + + yield [$user, $person2, $scopesCanSee, [], [$period], "a user see his period when confirmed and confidential with required scopes"]; + yield [$user, $person2, $scopesGroup2, [], [$period], "a user see his period when confirmed and confidential without required scopes"]; + yield [$anotherUser, $person2, $scopesCanSee, [], [], "a user don't see a confidential period, even if he has required scopes"]; + yield [$anotherUser, $person2, $scopesCanSee, $scopesCanSee, [$period], "a user see the period when confirmed and confidential if he has required scope to see the period"]; + + // period draft with creator = null + $period = $this->buildPeriod($person3, $scopesCanSee, null, false); + yield [$user, $person3, $scopesCanSee, [], [$period], "a user see a period when draft if no creator on the period"]; + $this->entityManager->flush(); + } + + /** + * @param Person $person + * @param array $scopes + * @return AccompanyingPeriod + */ + private function buildPeriod(Person $person, array $scopes, User|null $creator, bool $confirm): AccompanyingPeriod + { + $period = new AccompanyingPeriod(); + $period->addPerson($person); + if (null !== $creator) { + $period->setCreatedBy($creator); + } + + foreach ($scopes as $scope) { + $period->addScope($scope); + } + + $this->entityManager->persist($period); + self::$periodsIdsToDelete[] = $period->getId(); + + if ($confirm) { + $workflow = $this->registry->get($period, 'accompanying_period_lifecycle'); + $workflow->apply($period, 'confirm'); + } + + return $period; + } +}