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).
2026-04-20 16:46:57 +02:00
parent fd5d3fe36f
commit 8bedab407d
5 changed files with 258 additions and 3 deletions
--- a/src/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteCollectionScopedExtension.php
+++ b/src/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteCollectionScopedExtension.php
@@ -0,0 +1,110 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Module\Sites\Infrastructure\ApiPlatform\Extension;
+
+use ApiPlatform\Doctrine\Orm\Extension\QueryCollectionExtensionInterface;
+use ApiPlatform\Doctrine\Orm\Extension\QueryItemExtensionInterface;
+use ApiPlatform\Doctrine\Orm\Util\QueryNameGeneratorInterface;
+use ApiPlatform\Metadata\Operation;
+use App\Module\Core\Domain\Entity\User;
+use App\Module\Sites\Domain\Entity\Site;
+use Doctrine\ORM\QueryBuilder;
+use Symfony\Bundle\SecurityBundle\Security;
+
+use function sprintf;
+
+/**
+ * Extension API Platform qui restreint les collections et items de la
+ * resource Site (/api/sites) aux seuls sites auxquels l'utilisateur
+ * authentifie est rattache (ticket module Sites — prevention de la fuite
+ * de donnees cross-tenant).
+ *
+ * `Site` n'implemente pas `SiteAwareInterface` (ce serait circulaire : un
+ * site ne s'appartient pas a lui-meme). Cette extension complementaire
+ * cible specifiquement `Site::class` et applique un filtre IN sur les IDs
+ * des sites de l'utilisateur.
+ *
+ * Comportement selon les cas :
+ *  - resource != Site::class          → no-op (les autres resources sont
+ *                                        gerees par SiteScopedQueryExtension) ;
+ *  - is_granted('sites.bypass_scope') → pas de filtre (admin / bypass) ;
+ *  - user non authentifie             → no-op (API Platform renvoie 401 avant) ;
+ *  - user sans aucun site             → WHERE 1 = 0 (aucun acces) ;
+ *  - cas normal                       → WHERE site.id IN (:allowedSites).
+ *
+ * Consequence anti-enumeration : GET /api/sites/{id} retourne 404 et non
+ * 403 si l'item existe mais n'appartient pas aux sites de l'utilisateur
+ * (comportement natif API Platform quand Doctrine retourne null).
+ */
+final class SiteCollectionScopedExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface
+{
+    public function __construct(
+        private readonly Security $security,
+    ) {}
+
+    public function applyToCollection(
+        QueryBuilder $queryBuilder,
+        QueryNameGeneratorInterface $queryNameGenerator,
+        string $resourceClass,
+        ?Operation $operation = null,
+        array $context = [],
+    ): void {
+        $this->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)
+        ;
+    }
+}
--- a/src/Module/Sites/Infrastructure/ApiPlatform/Extension/UserSiteScopedExtension.php
+++ b/src/Module/Sites/Infrastructure/ApiPlatform/Extension/UserSiteScopedExtension.php
@@ -0,0 +1,116 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Module\Sites\Infrastructure\ApiPlatform\Extension;
+
+use ApiPlatform\Doctrine\Orm\Extension\QueryCollectionExtensionInterface;
+use ApiPlatform\Doctrine\Orm\Extension\QueryItemExtensionInterface;
+use ApiPlatform\Doctrine\Orm\Util\QueryNameGeneratorInterface;
+use ApiPlatform\Metadata\Operation;
+use App\Module\Core\Domain\Entity\User;
+use App\Module\Sites\Domain\Entity\Site;
+use Doctrine\ORM\QueryBuilder;
+use Symfony\Bundle\SecurityBundle\Security;
+
+use function sprintf;
+
+/**
+ * Extension API Platform qui restreint /api/users (collection + item) aux
+ * utilisateurs partageant au moins un site commun avec l'appelant.
+ *
+ * Objectif : empecher l'enumeration cross-site des utilisateurs. Sans ce
+ * filtre, un user du site A pourrait lister tous les users du site B via
+ * GET /api/users.
+ *
+ * Conditions de bypass :
+ *  - is_granted('sites.bypass_scope') → visibilite totale (admin ou bypass
+ *    explicite) ;
+ *  - user non authentifie             → no-op (API Platform renvoie 401) ;
+ *
+ * Cas particulier — appelant sans aucun site rattache :
+ *  Comportement defensif : l'utilisateur ne voit que lui-meme. Cela evite
+ *  de bloquer completement un user mal configure tout en ne lui revelant
+ *  aucun autre utilisateur.
+ *
+ * Strategie DQL : JOIN sur la relation ManyToMany `.sites` + DISTINCT pour
+ * eviter les doublons si un user partage plusieurs sites avec l'appelant.
+ * Le alias `s_scope` est utilise pour la jointure intermediaire.
+ */
+final class UserSiteScopedExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface
+{
+    public function __construct(
+        private readonly Security $security,
+    ) {}
+
+    public function applyToCollection(
+        QueryBuilder $queryBuilder,
+        QueryNameGeneratorInterface $queryNameGenerator,
+        string $resourceClass,
+        ?Operation $operation = null,
+        array $context = [],
+    ): void {
+        $this->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()
+        ;
+    }
+}
--- 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();
--- 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).
--- 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