Implements right "see confidential course" on method findByPerson

Add unit tests for that

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
     {

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 $center
+     * @return list<Scope>
      */
-    public function getReachableScopes(string $role, $center): array;
+    public function getReachableScopes(string $role, array|Center $center): array;
 }

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> $center
+     * @param Center|array<Center> $center
+     * @return list<Scope>
      */
     public function getReachableScopes(UserInterface $user, string $role, Center|array $center): array;
 }

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
+        $accompanyingPeriods = $this->accompanyingPeriodACLAwareRepository
-            ->findByPerson($person, AccompanyingPeriodVoter::SEE);
+            ->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,

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\Authorization\AuthorizationHelperForCurrentUserInterface;
-use Chill\MainBundle\Security\Resolver\CenterResolverDispatcherInterface;
+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,
+        AuthorizationHelperForCurrentUserInterface $authorizationHelper,
-        CenterResolverDispatcherInterface $centerResolverDispatcher
+        CenterResolverManagerInterface $centerResolverDispatcher
     ) {
         $this->accompanyingPeriodRepository = $accompanyingPeriodRepository;
         $this->security = $security;
         $this->authorizationHelper = $authorizationHelper;
-        $this->centerResolverDispatcher = $centerResolverDispatcher;
+        $this->centerResolver = $centerResolverDispatcher;
     }
 
-    /**
+    public function buildQueryOpenedAccompanyingCourseByUser(?User $user, array $postalCodes = []): QueryBuilder
-     * @param array|PostalCode[]
-     *
-     * @return QueryBuilder
-     */
-    public function buildQueryOpenedAccompanyingCourseByUser(?User $user, array $postalCodes = [])
     {
         $qb = $this->accompanyingPeriodRepository->createQueryBuilder('ap');
 
@@ -152,10 +149,14 @@ final class AccompanyingPeriodACLAwareRepository implements AccompanyingPeriodAC
     ): array {
         $qb = $this->accompanyingPeriodRepository->createQueryBuilder('ap');
         $scopes = $this->authorizationHelper
-            ->getReachableCircles(
+            ->getReachableScopes(
-                $this->security->getUser(),
                 $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(
+            ->setParameter('person', $person);
-                $qb->expr()->orX(
+
-                    'ap.confidential = FALSE',
+        $qb = $this->addACLClauses($qb, $scopes, $scopesCanSeeConfidential);
-                    $qb->expr()->eq('ap.user', ':user')
+        $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<Scope> $scopesCanSee
+     * @param array<Scope> $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) {
+        foreach ($scopesCanSee as $key => $scope) {
-            $orx->add($qb->expr()->orX(
+            // 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(
+                ->getReachableScopes(
-                    $this->security->getUser(),
                     AccompanyingPeriodVoter::SEE,
                     $center
                 );

src/Bundle/ChillPersonBundle/Repository/AccompanyingPeriodACLAwareRepositoryInterface.php

@@ -33,6 +33,9 @@ interface AccompanyingPeriodACLAwareRepositoryInterface
 
     public function countByUserOpenedAccompanyingPeriod(?User $user): int;
 
+    /**
+     * @return array<AccompanyingPeriod>
+     */
     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<AccompanyingPeriod>
      */
     public function findByUnDispatched(array $jobs, array $services, array $administrativeLocations, ?int $limit = null, ?int $offset = null): array;
 
     /**
      * @param array|PostalCode[] $postalCodes
+     * @return list<AccompanyingPeriod>
      */
     public function findByUserAndPostalCodesOpenedAccompanyingPeriod(?User $user, array $postalCodes, array $orderBy = [], int $limit = 0, int $offset = 50): array;
 
+    /**
+     * @deprecated
+     * @return list<AccompanyingPeriod>
+     */
     public function findByUserOpenedAccompanyingPeriod(?User $user, array $orderBy = [], int $limit = 0, int $offset = 50): array;
 }

src/Bundle/ChillPersonBundle/Tests/Repository/AccompanyingPeriodACLAwareRepositoryTest.php

@@ -0,0 +1,206 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * Chill is a software for social workers
+ *
+ * For the full copyright and license information, please view
+ * the LICENSE file that was distributed with this source code.
+ */
+
+namespace Repository;
+
+use Chill\MainBundle\Entity\Scope;
+use Chill\MainBundle\Entity\User;
+use Chill\MainBundle\Repository\ScopeRepositoryInterface;
+use Chill\MainBundle\Security\Authorization\AuthorizationHelperForCurrentUserInterface;
+use Chill\MainBundle\Security\Resolver\CenterResolverManagerInterface;
+use Chill\PersonBundle\Entity\AccompanyingPeriod;
+use Chill\PersonBundle\Entity\Person;
+use Chill\PersonBundle\Repository\AccompanyingPeriodACLAwareRepository;
+use Chill\PersonBundle\Repository\AccompanyingPeriodRepository;
+use Chill\PersonBundle\Security\Authorization\AccompanyingPeriodVoter;
+use Doctrine\ORM\EntityManagerInterface;
+use Prophecy\Argument;
+use Prophecy\PhpUnit\ProphecyTrait;
+use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
+use Symfony\Component\Security\Core\Security;
+use Symfony\Component\Workflow\Registry;
+
+/**
+ * @internal
+ * @coversNothing
+ */
+class AccompanyingPeriodACLAwareRepositoryTest extends KernelTestCase
+{
+    use ProphecyTrait;
+    private AccompanyingPeriodRepository $accompanyingPeriodRepository;
+
+    private CenterResolverManagerInterface $centerResolverManager;
+
+    private EntityManagerInterface $entityManager;
+
+    private ScopeRepositoryInterface $scopeRepository;
+
+    private Registry $registry;
+
+    private static array $periodsIdsToDelete = [];
+
+    protected function setUp(): void
+    {
+        self::bootKernel();
+        $this->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<Scope> $scopeUserCanSee
+     * @param array<Scope> $scopeUserCanSeeConfidential
+     * @param array<AccompanyingPeriod> $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<Scope> $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;
+    }
+}