From 8bedab407d627c6fa67b3a069077a39459b7c84a Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 20 Apr 2026 16:46:57 +0200 Subject: [PATCH] feat(sites) : scope /api/sites et /api/users aux sites autorises du caller - SiteCollectionScopedExtension filtre /api/sites aux sites du user (name/adresse/CP/ville plus lisibles par un delegataire sites.view qui n'appartient pas a ces sites). Bypass via sites.bypass_scope. - UserSiteScopedExtension filtre /api/users aux users partageant au moins un site avec le caller. Empeche un delegataire de core.users.view d'enumerer l'organigramme complet + les sites de tous les tenants. - Helper createUserWithPermission rattache le user jetable a tous les sites fixtures, sinon le scoping le rend aveugle aux cibles. - test_target de UserRbacApiTest attache de meme aux sites pour rester visible depuis un caller non-admin. - testUserCannotSwitchToUnauthorizedSite : 403 -> 400 (anti-enumeration). --- .../SiteCollectionScopedExtension.php | 110 +++++++++++++++++ .../Extension/UserSiteScopedExtension.php | 116 ++++++++++++++++++ tests/Module/Core/Api/AbstractApiTestCase.php | 14 +++ tests/Module/Core/Api/UserRbacApiTest.php | 10 +- .../Sites/Api/CurrentSiteSwitchApiTest.php | 11 +- 5 files changed, 258 insertions(+), 3 deletions(-) create mode 100644 src/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteCollectionScopedExtension.php create mode 100644 src/Module/Sites/Infrastructure/ApiPlatform/Extension/UserSiteScopedExtension.php diff --git a/src/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteCollectionScopedExtension.php b/src/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteCollectionScopedExtension.php new file mode 100644 index 0000000..8339159 --- /dev/null +++ b/src/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteCollectionScopedExtension.php @@ -0,0 +1,110 @@ +applyScope($queryBuilder, $queryNameGenerator, $resourceClass); + } + + public function applyToItem( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + array $identifiers, + ?Operation $operation = null, + array $context = [], + ): void { + $this->applyScope($queryBuilder, $queryNameGenerator, $resourceClass); + } + + /** + * Applique le filtre IN sur les IDs de sites autorises si les conditions + * d'application sont remplies. No-op sinon. + */ + private function applyScope( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + ): void { + // 1) Cette extension cible uniquement la resource Site. + if (Site::class !== $resourceClass) { + return; + } + + // 2) Admin ou user avec bypass explicite : visibilite globale. + if ($this->security->isGranted('sites.bypass_scope')) { + return; + } + + // 3) Pas d'user authentifie -> no-op (API Platform gere le 401 en amont). + $user = $this->security->getUser(); + if (!$user instanceof User) { + return; + } + + $rootAlias = $queryBuilder->getRootAliases()[0]; + + // 4) User sans aucun site rattache -> aucun acces possible. + $siteIds = $user->getSites()->map(fn (Site $s) => $s->getId())->toArray(); + if (empty($siteIds)) { + $queryBuilder->andWhere('1 = 0'); + + return; + } + + // 5) Cas normal : restriction aux sites autorises de l'utilisateur. + $param = $queryNameGenerator->generateParameterName('allowedSites'); + $queryBuilder + ->andWhere(sprintf('%s.id IN (:%s)', $rootAlias, $param)) + ->setParameter($param, $siteIds) + ; + } +} diff --git a/src/Module/Sites/Infrastructure/ApiPlatform/Extension/UserSiteScopedExtension.php b/src/Module/Sites/Infrastructure/ApiPlatform/Extension/UserSiteScopedExtension.php new file mode 100644 index 0000000..3da6750 --- /dev/null +++ b/src/Module/Sites/Infrastructure/ApiPlatform/Extension/UserSiteScopedExtension.php @@ -0,0 +1,116 @@ +applyScope($queryBuilder, $queryNameGenerator, $resourceClass); + } + + public function applyToItem( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + array $identifiers, + ?Operation $operation = null, + array $context = [], + ): void { + $this->applyScope($queryBuilder, $queryNameGenerator, $resourceClass); + } + + /** + * Applique le filtre de partage de site si les conditions d'application + * sont remplies. No-op sinon. + */ + private function applyScope( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + ): void { + // 1) Cette extension cible uniquement la resource User. + if (User::class !== $resourceClass) { + return; + } + + // 2) Admin ou bypass explicite : visibilite totale. + if ($this->security->isGranted('sites.bypass_scope')) { + return; + } + + // 3) Pas d'user authentifie -> no-op (API Platform gere le 401 en amont). + $user = $this->security->getUser(); + if (!$user instanceof User) { + return; + } + + $rootAlias = $queryBuilder->getRootAliases()[0]; + $callerSiteIds = $user->getSites()->map(fn (Site $s) => $s->getId())->toArray(); + + // 4) Appelant sans site : comportement defensif -> il ne voit que lui-meme. + if (empty($callerSiteIds)) { + $queryBuilder + ->andWhere(sprintf('%s.id = :self', $rootAlias)) + ->setParameter('self', $user->getId()) + ; + + return; + } + + // 5) Cas normal : garder uniquement les users qui partagent au moins + // un site avec l'appelant. JOIN sur la relation ManyToMany `.sites` + // + filtre IN + DISTINCT pour eviter les lignes dupliquees. + $param = $queryNameGenerator->generateParameterName('callerSites'); + $queryBuilder + ->innerJoin(sprintf('%s.sites', $rootAlias), 's_scope') + ->andWhere(sprintf('s_scope.id IN (:%s)', $param)) + ->setParameter($param, $callerSiteIds) + ->distinct() + ; + } +} diff --git a/tests/Module/Core/Api/AbstractApiTestCase.php b/tests/Module/Core/Api/AbstractApiTestCase.php index 6d1b7b4..9f7bcc2 100644 --- a/tests/Module/Core/Api/AbstractApiTestCase.php +++ b/tests/Module/Core/Api/AbstractApiTestCase.php @@ -9,6 +9,7 @@ use ApiPlatform\Symfony\Bundle\Test\Client; use App\Module\Core\Domain\Entity\Permission; use App\Module\Core\Domain\Entity\Role; use App\Module\Core\Domain\Entity\User; +use App\Module\Sites\Domain\Entity\Site; use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; @@ -123,6 +124,19 @@ abstract class AbstractApiTestCase extends ApiTestCase $user->setIsAdmin(false); $user->setPassword($hasher->hashPassword($user, $password)); $user->addRbacRole($role); + + // Le helper attache le user jetable a tous les sites existants pour + // neutraliser le filtrage par UserSiteScopedExtension : la plupart + // des tests assume une visibilite globale sur les users cibles. Les + // tests qui valident le comportement "sans sites" doivent creer leur + // user a la main (pas via ce helper). + $siteRepository = $em->getRepository(Site::class); + if (null !== $siteRepository) { + foreach ($siteRepository->findAll() as $site) { + $user->addSite($site); + } + } + $em->persist($user); $em->flush(); diff --git a/tests/Module/Core/Api/UserRbacApiTest.php b/tests/Module/Core/Api/UserRbacApiTest.php index 5d825c1..cc9e866 100644 --- a/tests/Module/Core/Api/UserRbacApiTest.php +++ b/tests/Module/Core/Api/UserRbacApiTest.php @@ -7,6 +7,7 @@ namespace App\Tests\Module\Core\Api; use App\Module\Core\Domain\Entity\Permission; use App\Module\Core\Domain\Entity\Role; use App\Module\Core\Domain\Entity\User; +use App\Module\Sites\Domain\Entity\Site; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; /** @@ -41,11 +42,18 @@ final class UserRbacApiTest extends AbstractApiTestCase /** @var UserPasswordHasherInterface $hasher */ $hasher = self::getContainer()->get(UserPasswordHasherInterface::class); - // User cible standard (non admin). + // User cible standard (non admin). On lui attache tous les sites + // fixtures pour rester visible depuis les callers non-admin munis de + // sites (cf. UserSiteScopedExtension qui filtre `/api/users` par + // intersection de sites). Sans cela, un user `core.users.manage` + // sans site commun avec test_target recevrait un 404 sur le PATCH. $target = new User(); $target->setUsername('test_target'); $target->setIsAdmin(false); $target->setPassword($hasher->hashPassword($target, 'secret')); + foreach ($em->getRepository(Site::class)->findAll() as $site) { + $target->addSite($site); + } $em->persist($target); // User admin dedie pour le cas d'auto-suicide (pas l'admin fixture). diff --git a/tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php b/tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php index e8bf214..7d02c34 100644 --- a/tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php +++ b/tests/Module/Sites/Api/CurrentSiteSwitchApiTest.php @@ -39,7 +39,14 @@ final class CurrentSiteSwitchApiTest extends AbstractApiTestCase public function testUserCannotSwitchToUnauthorizedSite(): void { - // alice n'a que Chatellerault. Tenter Pommevic → 403. + // alice n'a que Chatellerault. Tenter Pommevic → 400 (anti-enumeration). + // + // Depuis l'ajout de SiteCollectionScopedExtension, les sites hors + // du scope de l'user sont filtres a la source : l'IriConverter ne + // peut pas resoudre `/api/sites/{id}` pour un site non autorise et + // leve 400 "Item not found". Reponse identique a "site inexistant", + // ce qui empeche l'enumeration des ids de sites tiers. Avant la PR + // scope, le processor traduisait SiteNotAuthorizedException → 403. $em = $this->getEm(); $pommevic = $em->getRepository(Site::class)->findOneBy(['name' => 'Pommevic']); self::assertNotNull($pommevic); @@ -50,7 +57,7 @@ final class CurrentSiteSwitchApiTest extends AbstractApiTestCase 'json' => ['site' => '/api/sites/'.$pommevic->getId()], ]); - self::assertResponseStatusCodeSame(403); + self::assertResponseStatusCodeSame(400); } public function testSwitchWithMissingSiteFieldReturns400(): void