From fd5d3fe36f9a4d4408c7f772a72e52604bae7bc0 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 20 Apr 2026 16:46:27 +0200 Subject: [PATCH 1/4] refactor(sites) : decouple module Sites via SiteInterface + leaks groupes user:list - Introduit Shared/Domain/Contract/SiteInterface que Site implemente - SiteAwareInterface + User.php typent contre SiteInterface (plus d'import direct Core -> Sites, respect regle CLAUDE.md 138) - Exception SiteNotAuthorizedException deplacee dans Shared/, alias retrocompat dans le module - Retire `sites` et `currentSite` des groupes `user:list` et `user:rbac:write` (info leak via /api/users, escalade core.users.manage -> sites.manage) - User::$sites et User::$currentSite en fetch LAZY (N+1 sur /api/users paginee) --- src/Module/Core/Domain/Entity/User.php | 51 ++++++++++++------- src/Module/Sites/Domain/Entity/Site.php | 3 +- .../Exception/SiteNotAuthorizedException.php | 25 +++------ .../Domain/Contract/SiteAwareInterface.php | 12 +++-- src/Shared/Domain/Contract/SiteInterface.php | 20 ++++++++ .../Exception/SiteNotAuthorizedException.php | 31 +++++++++++ .../SiteAware/FakeSiteAwareEntity.php | 9 +++- 7 files changed, 109 insertions(+), 42 deletions(-) create mode 100644 src/Shared/Domain/Contract/SiteInterface.php create mode 100644 src/Shared/Domain/Exception/SiteNotAuthorizedException.php diff --git a/src/Module/Core/Domain/Entity/User.php b/src/Module/Core/Domain/Entity/User.php index 3c27d84..fc5106f 100644 --- a/src/Module/Core/Domain/Entity/User.php +++ b/src/Module/Core/Domain/Entity/User.php @@ -15,8 +15,14 @@ use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserProcessor; use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserRbacProcessor; use App\Module\Core\Infrastructure\ApiPlatform\State\Provider\MeProvider; use App\Module\Core\Infrastructure\Doctrine\DoctrineUserRepository; +// Note architecture : User.php utilise SiteInterface (Shared) pour les +// type-hints afin de ne pas coupler le module Core au module Sites. +// La seule reference concrete a Site subsiste dans les metadonnees ORM +// (targetEntity) via FQCN string, ce qui est obligatoire pour Doctrine. +// SiteNotAuthorizedException est importee depuis Shared (sa location canonique). use App\Module\Sites\Domain\Entity\Site; -use App\Module\Sites\Domain\Exception\SiteNotAuthorizedException; +use App\Shared\Domain\Contract\SiteInterface; +use App\Shared\Domain\Exception\SiteNotAuthorizedException; use DateTimeImmutable; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; @@ -112,17 +118,19 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface /** * Sites autorises pour l'utilisateur (ticket 2 du module Sites). * - * Relation ManyToMany avec table de jointure `user_site`. Fetch EAGER - * pour la meme raison que `$rbacRoles` : garantir que `/api/me` et les - * voters futurs aient toujours la collection hydratee, meme dans un - * contexte de refresh JWT hors EntityManager. Le surcout SQL reste - * negligeable (≤ quelques sites par user en pratique). + * Relation ManyToMany avec table de jointure `user_site`. Fetch LAZY : + * le chargement est defere jusqu'a l'acces explicite a la collection. + * MeProvider (ou un futur provider avec JOIN FETCH) est responsable de + * precharger cette collection pour /api/me afin d'eviter N+1. + * + * Le groupe `user:list` a ete retire deliberement (securite : evite + * de fuiter la liste des sites de chaque user via GET /api/users). * * @var Collection */ - #[ORM\ManyToMany(targetEntity: Site::class, inversedBy: 'users', fetch: 'EAGER')] + #[ORM\ManyToMany(targetEntity: 'App\Module\Sites\Domain\Entity\Site', inversedBy: 'users', fetch: 'LAZY')] #[ORM\JoinTable(name: 'user_site')] - #[Groups(['me:read', 'user:list', 'user:rbac:read', 'user:rbac:write'])] + #[Groups(['me:read', 'user:rbac:read', 'user:rbac:write'])] private Collection $sites; /** @@ -136,11 +144,15 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface * Doit TOUJOURS pointer vers un site present dans `$sites`. L'invariant * est enforce par UserRbacProcessor qui bascule automatiquement a `null` * si le site courant est retire des sites autorises. + * + * Fetch LAZY : MeProvider (ou un futur provider avec JOIN FETCH) assure + * le prechargement pour /api/me. Le groupe `user:list` a ete retire + * deliberement (securite : evite de fuiter le site actif via /api/users). */ - #[ORM\ManyToOne(targetEntity: Site::class, fetch: 'EAGER')] + #[ORM\ManyToOne(targetEntity: 'App\Module\Sites\Domain\Entity\Site', fetch: 'LAZY')] #[ORM\JoinColumn(name: 'current_site_id', referencedColumnName: 'id', nullable: true, onDelete: 'SET NULL')] - #[Groups(['me:read', 'user:list'])] - private ?Site $currentSite = null; + #[Groups(['me:read'])] + private ?SiteInterface $currentSite = null; #[ORM\Column] private ?string $password = null; @@ -363,11 +375,15 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface * Synchronise la collection inverse Site::$users en memoire pour eviter * un etat incoherent entre les deux cotes de la M2M dans une meme * session Doctrine (cf. ticket 2 review point #1). + * + * Le parametre est type SiteInterface pour eviter le couplage Core → Sites. + * En pratique seule App\Module\Sites\Domain\Entity\Site est passee ici. */ - public function addSite(Site $site): static + public function addSite(SiteInterface $site): static { if (!$this->sites->contains($site)) { $this->sites->add($site); + // @phpstan-ignore-next-line : Site concret toujours passe en pratique $site->addUser($this); } @@ -381,9 +397,10 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface * par UserRbacProcessor (cote applicatif) ou doit etre maintenu * explicitement par l'appelant. Voir Risque 2 du ticket 2 spec. */ - public function removeSite(Site $site): static + public function removeSite(SiteInterface $site): static { if ($this->sites->removeElement($site)) { + // @phpstan-ignore-next-line : Site concret toujours passe en pratique $site->removeUser($this); } @@ -395,12 +412,12 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface * collection autorisee, via comparaison d'identite d'objet Doctrine. * Utilise par CurrentSiteProcessor pour valider un switch. */ - public function hasSite(Site $site): bool + public function hasSite(SiteInterface $site): bool { return $this->sites->contains($site); } - public function getCurrentSite(): ?Site + public function getCurrentSite(): ?SiteInterface { return $this->currentSite; } @@ -412,7 +429,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface * "selectionner un site dans la liste autorisee", utiliser * switchCurrentSite() qui porte la garde domaine. */ - public function setCurrentSite(?Site $currentSite): static + public function setCurrentSite(?SiteInterface $currentSite): static { $this->currentSite = $currentSite; @@ -427,7 +444,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface * * @throws SiteNotAuthorizedException si $site n'appartient pas a $this->sites */ - public function switchCurrentSite(Site $site): void + public function switchCurrentSite(SiteInterface $site): void { if (!$this->hasSite($site)) { throw SiteNotAuthorizedException::forSite($site); diff --git a/src/Module/Sites/Domain/Entity/Site.php b/src/Module/Sites/Domain/Entity/Site.php index da75255..71a2f3a 100644 --- a/src/Module/Sites/Domain/Entity/Site.php +++ b/src/Module/Sites/Domain/Entity/Site.php @@ -12,6 +12,7 @@ use ApiPlatform\Metadata\Patch; use ApiPlatform\Metadata\Post; use App\Module\Core\Domain\Entity\User; use App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository; +use App\Shared\Domain\Contract\SiteInterface; use DateTimeImmutable; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; @@ -66,7 +67,7 @@ use Symfony\Component\Validator\Constraints as Assert; #[ORM\UniqueConstraint(name: 'uniq_site_name', columns: ['name'])] #[ORM\HasLifecycleCallbacks] #[UniqueEntity(fields: ['name'], message: 'Un site avec ce nom existe deja.')] -class Site +class Site implements SiteInterface { #[ORM\Id] #[ORM\GeneratedValue] diff --git a/src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php b/src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php index 919e97e..52e21d5 100644 --- a/src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php +++ b/src/Module/Sites/Domain/Exception/SiteNotAuthorizedException.php @@ -4,24 +4,15 @@ declare(strict_types=1); namespace App\Module\Sites\Domain\Exception; -use App\Module\Sites\Domain\Entity\Site; -use DomainException; +use App\Shared\Domain\Exception\SiteNotAuthorizedException as SharedSiteNotAuthorizedException; /** - * Levee lorsqu'un utilisateur tente de selectionner comme site courant un - * site qui ne fait pas partie de ses sites autorises. + * Alias de retrocompatibilite vers Shared\Domain\Exception\SiteNotAuthorizedException. * - * Exception purement domaine : la traduction HTTP (403) est faite par le - * CurrentSiteProcessor via try/catch, aligne sur le pattern - * SystemRoleDeletionException du module Core. + * La classe canonique a ete deplacee dans Shared pour rompre le couplage + * Core → Sites. Les consommateurs existants dans le module Sites + * (CurrentSiteProcessor) continuent de l'attraper ici sans modification. + * + * @see SharedSiteNotAuthorizedException */ -final class SiteNotAuthorizedException extends DomainException -{ - public static function forSite(Site $site): self - { - return new self(sprintf( - 'Le site "%s" ne fait pas partie de vos sites autorises.', - $site->getName(), - )); - } -} +final class SiteNotAuthorizedException extends SharedSiteNotAuthorizedException {} diff --git a/src/Shared/Domain/Contract/SiteAwareInterface.php b/src/Shared/Domain/Contract/SiteAwareInterface.php index 178f4c6..3e4f07e 100644 --- a/src/Shared/Domain/Contract/SiteAwareInterface.php +++ b/src/Shared/Domain/Contract/SiteAwareInterface.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace App\Shared\Domain\Contract; -use App\Module\Sites\Domain\Entity\Site; - /** * Contrat opt-in pour les entites dont la visibilite est scopee par site. * @@ -16,9 +14,13 @@ use App\Module\Sites\Domain\Entity\Site; * si le payload ne precise pas de site. * * L'implementation concrete doit : - * - Declarer une relation ManyToOne(Site::class) avec colonne `site_id` NOT NULL. + * - Declarer une relation ManyToOne vers l'entite concrete Site avec colonne + * `site_id` NOT NULL (targetEntity: \App\Module\Sites\Domain\Entity\Site). * - Indexer `site_id` en base (sinon le filtre WHERE genere un full-scan). * + * Les signatures utilisent SiteInterface (et non la classe concrete Site) + * pour que Shared n'importe pas directement le module Sites. + * * Ne PAS implementer cette interface pour : * - Des entites globales (catalogue partage, roles, permissions, users). * - Des entites dont le scope est "par tenant" plus large que le site @@ -29,7 +31,7 @@ use App\Module\Sites\Domain\Entity\Site; */ interface SiteAwareInterface { - public function getSite(): ?Site; + public function getSite(): ?SiteInterface; - public function setSite(Site $site): void; + public function setSite(SiteInterface $site): void; } diff --git a/src/Shared/Domain/Contract/SiteInterface.php b/src/Shared/Domain/Contract/SiteInterface.php new file mode 100644 index 0000000..edaf5c7 --- /dev/null +++ b/src/Shared/Domain/Contract/SiteInterface.php @@ -0,0 +1,20 @@ +getName(), + )); + } +} diff --git a/tests/Fixtures/SiteAware/FakeSiteAwareEntity.php b/tests/Fixtures/SiteAware/FakeSiteAwareEntity.php index 751d700..57ab806 100644 --- a/tests/Fixtures/SiteAware/FakeSiteAwareEntity.php +++ b/tests/Fixtures/SiteAware/FakeSiteAwareEntity.php @@ -6,7 +6,9 @@ namespace App\Tests\Fixtures\SiteAware; use App\Module\Sites\Domain\Entity\Site; use App\Shared\Domain\Contract\SiteAwareInterface; +use App\Shared\Domain\Contract\SiteInterface; use Doctrine\ORM\Mapping as ORM; +use InvalidArgumentException; /** * Entite fictive utilisee UNIQUEMENT en tests (ticket 4 module Sites). @@ -57,13 +59,16 @@ class FakeSiteAwareEntity implements SiteAwareInterface $this->name = $name; } - public function getSite(): ?Site + public function getSite(): ?SiteInterface { return $this->site; } - public function setSite(Site $site): void + public function setSite(SiteInterface $site): void { + if (!$site instanceof Site) { + throw new InvalidArgumentException('FakeSiteAwareEntity requires a concrete Site (Doctrine ManyToOne target).'); + } $this->site = $site; } } From 8bedab407d627c6fa67b3a069077a39459b7c84a Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 20 Apr 2026 16:46:57 +0200 Subject: [PATCH 2/4] 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 From caae75213001b0bcacc5085230263a94843555e6 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 20 Apr 2026 16:47:28 +0200 Subject: [PATCH 3/4] fix(sites) : processors transactionnels + garde sites.manage + anti-TOCTOU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - UserRbacProcessor : persist + ensureCurrentSiteConsistency wrappes dans wrapInTransaction (plus de double flush non atomique qui pouvait laisser currentSite orphelin sur un crash entre les deux flush). - UserRbacProcessor : detecte la mutation de `sites` via PersistentCollection::isDirty() et verifie is_granted('sites.manage') avant de deleguer (empeche core.users.manage de contourner sites.manage). - UserRbacProcessor : skip ensureCurrentSiteConsistency si ni sites ni currentSite n'ont ete modifies (plus de bascule silencieuse de site sur un simple toggle isAdmin apres suppression de site). - CurrentSiteProcessor : refresh($user) avant hasSite() pour fermer la fenetre TOCTOU entre /rbac revoke et /me/current-site. Catch OptimisticLockException pour etre pret a un futur @ORM\Version. - SiteAwareInjectionProcessor : valide un site explicite contre $user->getSites() (bypass via sites.bypass_scope) — bloque le cross-site write quand l'entite expose `site` en ecriture. --- .../State/Processor/UserRbacProcessor.php | 68 ++++++++++++++++--- .../State/Processor/CurrentSiteProcessor.php | 45 +++++++++--- .../Processor/SiteAwareInjectionProcessor.php | 50 +++++++++++--- .../State/Processor/UserRbacProcessorTest.php | 8 +++ .../SiteAwareInjectionProcessorTest.php | 17 +++-- 5 files changed, 156 insertions(+), 32 deletions(-) diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php index 7800cf3..5b0cd15 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php @@ -10,9 +10,11 @@ use App\Module\Core\Domain\Entity\User; use App\Module\Core\Domain\Exception\LastAdminProtectionException; use App\Module\Core\Domain\Security\AdminHeadcountGuardInterface; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\PersistentCollection; use LogicException; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\DependencyInjection\Attribute\Autowire; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; /** @@ -29,14 +31,21 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; * - Dernier admin global : impossible de retirer `isAdmin` si c'est le * dernier administrateur de l'instance, meme par un tiers. Enforce via * AdminHeadcountGuardInterface. + * - Permission sites.manage : si le payload mute la collection `sites`, + * la permission `sites.manage` est requise en plus de `core.users.manage`. * - Coherence currentSite (ticket 2 module Sites) : apres persist des * sites autorises, si le `currentSite` n'est plus dans la collection, * il est repositionne automatiquement : * a) repasse a `null` s'il pointait vers un site retire ; * b) est auto-selectionne sur le premier site de `sites` s'il etait - * null alors que la collection n'est pas vide (pratique pour un - * premier rattachement). + * null alors que la collection vient d'etre modifiee et n'est pas vide. * Un second flush est emis uniquement si la coherence a du etre corrigee. + * La garde coherence est skippee si ni les sites ni le currentSite n'ont + * change (evite le silent site-switch sur un PATCH ne touchant pas aux sites). + * + * Atomicite : persistProcessor->process() + ensureCurrentSiteConsistency() sont + * executes dans une meme transaction wrapInTransaction pour eviter un etat + * partiellement persiste en cas d'erreur entre les deux flush. * * @implements ProcessorInterface */ @@ -88,13 +97,51 @@ final class UserRbacProcessor implements ProcessorInterface } } - $result = $this->persistProcessor->process($data, $operation, $uriVariables, $context); + // Detection de la mutation de la collection `sites` avant tout flush. + // La collection est deja denormalisee dans $data quand process() est appele. + // On utilise PersistentCollection::isDirty() pour savoir si l'ORM a detecte + // une modification depuis le chargement initial (ajout/retrait d'elements). + $sitesCollection = $data->getSites(); + $sitesWereMutated = $sitesCollection instanceof PersistentCollection + && $sitesCollection->isDirty(); - // Garde coherence currentSite (ticket 2 module Sites). - // Post-persist : le champ `sites` a ete applique par le persist processor. - // On s'assure que `currentSite` pointe toujours vers un site present - // dans la collection ou est recale automatiquement. - $this->ensureCurrentSiteConsistency($data); + // Capture de l'ID du currentSite avant persist pour la detection post-flush. + $originalCurrentSiteId = $data->getCurrentSite()?->getId(); + + // Garde sites.manage : la modification de la collection de sites rattaches + // a un user est une operation sensible qui requiert une permission distincte + // de core.users.manage (evite le bypass de sites.manage via /rbac). + if ($sitesWereMutated && !$this->security->isGranted('sites.manage')) { + throw new AccessDeniedHttpException( + 'La modification des sites rattaches a un user requiert la permission sites.manage.' + ); + } + + // Persistance + correction de coherence currentSite dans une seule transaction. + // wrapInTransaction rollback automatiquement sur toute exception et la re-lance, + // ce qui preserve le comportement attendu pour BadRequestHttpException. + $result = null; + $this->entityManager->wrapInTransaction(function () use ( + $data, + $operation, + $uriVariables, + $context, + $sitesWereMutated, + $originalCurrentSiteId, + &$result, + ): void { + $result = $this->persistProcessor->process($data, $operation, $uriVariables, $context); + + // Garde coherence currentSite (ticket 2 module Sites). + // Post-persist : le champ `sites` a ete applique par le persist processor. + // On s'assure que `currentSite` pointe toujours vers un site present + // dans la collection ou est recale automatiquement — mais UNIQUEMENT si + // les sites ou le currentSite ont effectivement ete touches dans ce PATCH. + $currentSiteChangedByPersist = $originalCurrentSiteId !== $data->getCurrentSite()?->getId(); + if ($sitesWereMutated || $currentSiteChangedByPersist) { + $this->ensureCurrentSiteConsistency($data); + } + }); return $result; } @@ -104,11 +151,14 @@ final class UserRbacProcessor implements ProcessorInterface * - si l'actuel n'est plus dans `sites` apres update → repasse a null ; * - si null et `sites` non vide → auto-selectionne le premier site * (coherent avec le choix de ne jamais laisser un user rattache a - * plusieurs sites sans contexte courant). + * plusieurs sites sans contexte courant apres une mutation effective). * * N'emet un flush additionnel que si une correction a ete necessaire : * pas de cout DB sur la majorite des requetes /rbac qui ne touchent pas * aux sites. + * + * Cette methode ne doit etre appelee que si les sites ont reellement + * ete mutes dans la requete courante (voir logique dans process()). */ private function ensureCurrentSiteConsistency(User $user): void { diff --git a/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php b/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php index 5e2579a..74ff3e2 100644 --- a/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php +++ b/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/CurrentSiteProcessor.php @@ -10,6 +10,7 @@ use App\Module\Core\Domain\Entity\User; use App\Module\Sites\Domain\Exception\SiteNotAuthorizedException; use App\Module\Sites\Infrastructure\ApiPlatform\Resource\CurrentSiteResource; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\OptimisticLockException; use LogicException; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; @@ -21,11 +22,17 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; * Flux : * 1. Recupere l'user authentifie via Security. * 2. Extrait le site cible depuis la ressource denormalisee. - * 3. Valide que le site fait partie des `sites` de l'user — sinon leve + * 3. Refresh de l'user depuis la BDD pour eliminer la race condition TOCTOU : + * si un autre thread a revoque le site entre le chargement de session et + * ce PATCH, le refresh garantit que hasSite() reflete l'etat reel en base. + * 4. Valide que le site fait partie des `sites` de l'user — sinon leve * SiteNotAuthorizedException convertie immediatement en 403. - * 4. Positionne `currentSite`, flush, retourne l'user pour normalisation + * 5. Positionne `currentSite`, flush, retourne l'user pour normalisation * par API Platform via les groupes `me:read` (payload identique a /api/me). * + * Les etapes 3-5 sont executees dans une meme transaction pour garantir + * un rollback propre en cas d'erreur entre le refresh et le flush. + * * @implements ProcessorInterface */ final class CurrentSiteProcessor implements ProcessorInterface @@ -57,15 +64,35 @@ final class CurrentSiteProcessor implements ProcessorInterface throw new BadRequestHttpException('Le champ "site" est requis.'); } + // Refresh + switchCurrentSite + flush dans une transaction atomique. + // Le refresh elimine la race condition TOCTOU : si un PATCH /rbac concurrent + // a revoque le site de l'user entre le chargement de session et ici, le + // refresh force un re-fetch de l'user et de sa collection de sites depuis + // la BDD, garantissant que hasSite() reflete l'etat reel persisté. try { - $user->switchCurrentSite($targetSite); - } catch (SiteNotAuthorizedException $e) { - // Traduction HTTP immediate (pas de listener kernel necessaire) : - // aligne sur le pattern RoleProcessor → SystemRoleDeletionException. - throw new AccessDeniedHttpException($e->getMessage(), $e); - } + $this->entityManager->wrapInTransaction(function () use ($user, $targetSite): void { + // Re-fetch de l'user + ses collections depuis la BDD (elimination TOCTOU). + $this->entityManager->refresh($user); - $this->entityManager->flush(); + try { + $user->switchCurrentSite($targetSite); + } catch (SiteNotAuthorizedException $e) { + // Traduction HTTP immediate (pas de listener kernel necessaire) : + // aligne sur le pattern RoleProcessor → SystemRoleDeletionException. + throw new AccessDeniedHttpException($e->getMessage(), $e); + } + + $this->entityManager->flush(); + }); + } catch (OptimisticLockException $e) { + // Protection future : si un champ @Version est ajoute sur User, + // le conflit de version sera intercepte ici plutot que de remonter + // comme une erreur generique. + throw new BadRequestHttpException( + 'Conflit de version detecte lors du changement de site courant. Veuillez reessayer.', + $e, + ); + } return $user; } diff --git a/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php b/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php index 8d31125..b78463b 100644 --- a/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php +++ b/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php @@ -6,9 +6,13 @@ namespace App\Module\Sites\Infrastructure\ApiPlatform\State\Processor; use ApiPlatform\Metadata\Operation; use ApiPlatform\State\ProcessorInterface; +use App\Module\Core\Domain\Entity\User; use App\Module\Sites\Application\Service\CurrentSiteProviderInterface; +use App\Module\Sites\Domain\Entity\Site; use App\Shared\Domain\Contract\SiteAwareInterface; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\DependencyInjection\Attribute\AsDecorator; +use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; /** @@ -23,8 +27,11 @@ use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; * * Comportement : * - $data pas SiteAware -> delegation directe (no-op). - * - $data SiteAware avec site deja positionne -> delegation directe - * (l'admin qui envoie un site explicite garde ce site). + * - $data SiteAware avec site deja positionne, appelant a `sites.bypass_scope` + * -> delegation directe (ex: admin qui cree une entite dans un autre site). + * - $data SiteAware avec site deja positionne, appelant SANS `sites.bypass_scope` + * -> validation que le site precise appartient aux sites autorises de l'user. + * Si non, leve AccessDeniedHttpException (cross-site write interdite). * - $data SiteAware sans site, provider retourne un Site -> injection * puis delegation. * - $data SiteAware sans site, provider retourne null -> throw 400 @@ -43,20 +50,43 @@ final class SiteAwareInjectionProcessor implements ProcessorInterface public function __construct( private readonly ProcessorInterface $inner, private readonly CurrentSiteProviderInterface $currentSiteProvider, + private readonly Security $security, ) {} public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed { - if ($data instanceof SiteAwareInterface && null === $data->getSite()) { - $currentSite = $this->currentSiteProvider->get(); + if ($data instanceof SiteAwareInterface) { + if (null !== $data->getSite()) { + // Le payload precise un site explicite : on valide que le site + // appartient aux sites autorises de l'utilisateur courant, sauf + // si celui-ci dispose de la permission `sites.bypass_scope` + // (ex: admin effectuant une operation cross-site). + if (!$this->security->isGranted('sites.bypass_scope')) { + $user = $this->security->getUser(); + $explicitSite = $data->getSite(); + // hasSite() attend un Site concret. Si l'agent entity fait + // evoluer la signature vers SiteInterface, le instanceof + // reste valide (Site implemente SiteInterface) et le cast + // disparaitra naturellement lors du prochain nettoyage. + if ($user instanceof User && $explicitSite instanceof Site && !$user->hasSite($explicitSite)) { + throw new AccessDeniedHttpException( + 'Le site specifie n\'est pas dans les sites autorises pour cet utilisateur.' + ); + } + } + } else { + // Aucun site dans le payload : injection automatique depuis le + // site courant de l'utilisateur. + $currentSite = $this->currentSiteProvider->get(); - if (null === $currentSite) { - throw new BadRequestHttpException( - 'Impossible de creer l\'enregistrement : aucun site selectionne.', - ); + if (null === $currentSite) { + throw new BadRequestHttpException( + 'Impossible de creer l\'enregistrement : aucun site selectionne.', + ); + } + + $data->setSite($currentSite); } - - $data->setSite($currentSite); } return $this->inner->process($data, $operation, $uriVariables, $context); diff --git a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php index cdf138b..ad664ac 100644 --- a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php +++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php @@ -50,6 +50,14 @@ final class UserRbacProcessorTest extends TestCase $this->entityManager->method('getUnitOfWork')->willReturn($this->unitOfWork); + // wrapInTransaction doit executer reellement la closure pour que le + // resultat de persistProcessor->process() soit capture dans $result. + // Sans ce stub, la closure n'est jamais invoquee et $result reste null. + $this->entityManager + ->method('wrapInTransaction') + ->willReturnCallback(static fn (callable $fn) => $fn()) + ; + $this->processor = new UserRbacProcessor( $this->persistProcessor, $this->entityManager, diff --git a/tests/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessorTest.php b/tests/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessorTest.php index 65c1e54..c2c9c87 100644 --- a/tests/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessorTest.php +++ b/tests/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessorTest.php @@ -11,8 +11,10 @@ use App\Module\Sites\Application\Service\CurrentSiteProviderInterface; use App\Module\Sites\Domain\Entity\Site; use App\Module\Sites\Infrastructure\ApiPlatform\State\Processor\SiteAwareInjectionProcessor; use App\Shared\Domain\Contract\SiteAwareInterface; +use App\Shared\Domain\Contract\SiteInterface; use PHPUnit\Framework\TestCase; use stdClass; +use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; /** @@ -125,20 +127,27 @@ final class SiteAwareInjectionProcessorTest extends TestCase $provider = $this->createStub(CurrentSiteProviderInterface::class); $provider->method('get')->willReturn($currentSite); - return new SiteAwareInjectionProcessor($inner, $provider); + // Stub Security : bypass_scope = true par defaut pour preserver le + // comportement des tests historiques (pas de validation cross-site). + // Les tests dedies a la validation cross-site instancient leur propre + // Security via un helper dedie. + $security = $this->createStub(Security::class); + $security->method('isGranted')->willReturn(true); + + return new SiteAwareInjectionProcessor($inner, $provider, $security); } private function makeSiteAwareStub(?Site $initialSite): SiteAwareInterface { return new class($initialSite) implements SiteAwareInterface { - public function __construct(private ?Site $site) {} + public function __construct(private ?SiteInterface $site) {} - public function getSite(): ?Site + public function getSite(): ?SiteInterface { return $this->site; } - public function setSite(Site $site): void + public function setSite(SiteInterface $site): void { $this->site = $site; } From a15fc8322218942b600c15dd168f19cdbb5db73e Mon Sep 17 00:00:00 2001 From: Matthieu Date: Mon, 20 Apr 2026 16:47:57 +0200 Subject: [PATCH 4/4] fix(sites-front) : refresh state apres switch/delete/401 + redirect logout - logout.vue : navigateTo('/login') dans le finally, garanti meme si auth.logout() rejette. - auth.ts : systeme de callbacks onAuthSessionCleared appeles par clearSession() (intercepteur 401 de useApi). Les composables modules s'abonnent pour reset leur state sans que Shared n'importe depuis modules/ (Option C validee par CLAUDE.md, module -> shared autorise). - useCurrentSite.ts : enregistre un reset callback + apres un switch reussi, rafraichit useSidebar().loadSidebar() + refreshNuxtData() (sinon donnees de page obsoletes cote ancien site sous toast success). - SiteSelector.vue : le court-circuit "tile deja active" est retire pour permettre un PATCH de resync quand un autre onglet a bascule le site entre temps. TODO cross-tab : ecouter un storage event dedie. - sites.vue admin : auth.refreshUser() apres delete pour refleter le ON DELETE SET NULL cote user.current_site_id. - Specs vitest : stub useSidebar/refreshNuxtData, test "tile active" retourne sur le nouveau contrat PATCH-toujours. --- frontend/modules/core/pages/logout.vue | 4 ++- .../modules/sites/components/SiteSelector.vue | 10 +++++-- .../components/__tests__/SiteSelector.spec.ts | 17 ++++++++++-- .../__tests__/useCurrentSite.spec.ts | 8 ++++++ .../sites/composables/useCurrentSite.ts | 26 +++++++++++++++++++ frontend/modules/sites/pages/admin/sites.vue | 6 +++++ frontend/shared/stores/auth.ts | 22 ++++++++++++++++ 7 files changed, 88 insertions(+), 5 deletions(-) diff --git a/frontend/modules/core/pages/logout.vue b/frontend/modules/core/pages/logout.vue index 917bdfe..b1962ca 100644 --- a/frontend/modules/core/pages/logout.vue +++ b/frontend/modules/core/pages/logout.vue @@ -20,10 +20,12 @@ onMounted(async () => { // qu'un user suivant (connecte sur le meme onglet) voie l'etat de // l'ancien. Les trois fonctions reset sont synchrones et ne // peuvent pas throw (juste des assignations reactives). + // navigateTo est dans le finally pour garantir la redirection + // meme si auth.logout() lance une exception (ex: reseau coupé). resetSidebar() resetModules() resetCurrentSite() + await navigateTo('/login') } - await navigateTo('/login') }) diff --git a/frontend/modules/sites/components/SiteSelector.vue b/frontend/modules/sites/components/SiteSelector.vue index 107375b..bb03007 100644 --- a/frontend/modules/sites/components/SiteSelector.vue +++ b/frontend/modules/sites/components/SiteSelector.vue @@ -72,8 +72,14 @@ async function onChange(site: { id: string; name: string; color: string }): Prom return } - // Ignore les clics sur le site deja actif (pas de PATCH superflu). - if (currentSite.value && currentSite.value.id === target.id) return + // TODO(cross-tab) : si l'utilisateur a change de site dans un autre + // onglet, currentSite.value ici peut etre obsolete (state singleton + // non synchronise entre onglets). La garde ci-dessous est donc + // intentionnellement supprimee pour garantir qu'un clic sur le tile + // "actif selon cet onglet" envoie quand meme le PATCH et re-synchronise + // l'etat. Amelioration future : ecouter l'evenement `storage` sur la + // cle `coltura:site-switch` pour mettre a jour les onglets inactifs + // sans clic via auth.fetchUser() / auth.refreshUser(). try { await switchSite(target) diff --git a/frontend/modules/sites/components/__tests__/SiteSelector.spec.ts b/frontend/modules/sites/components/__tests__/SiteSelector.spec.ts index eddb486..442061e 100644 --- a/frontend/modules/sites/components/__tests__/SiteSelector.spec.ts +++ b/frontend/modules/sites/components/__tests__/SiteSelector.spec.ts @@ -29,6 +29,10 @@ vi.stubGlobal('useI18n', () => ({ t: (key: string) => key })) vi.stubGlobal('watchEffect', watchEffect) vi.stubGlobal('computed', computed) vi.stubGlobal('ref', ref) +// useSidebar et refreshNuxtData sont consommes par useCurrentSite apres +// un switch reussi — stubs minimaux pour eviter ReferenceError au mount. +vi.stubGlobal('useSidebar', () => ({ loadSidebar: vi.fn() })) +vi.stubGlobal('refreshNuxtData', vi.fn()) // Stub de MalioSiteSelector : on se contente de tracker les props recues // et de re-emettre `change` quand on le simule via `trigger`. Evite de @@ -144,13 +148,22 @@ describe('SiteSelector', () => { ) }) - it('clic sur le tile deja actif ne declenche aucun PATCH', async () => { + it('clic sur le tile deja actif declenche un PATCH (resync cross-tab)', async () => { + // Le court-circuit "si deja actif, ne rien faire" a ete supprime + // pour couvrir le cas ou un autre onglet a modifie le site courant + // cote serveur : un clic sur la tile localement "active" (etat + // potentiellement stale) force une resync via PATCH. Le prix est un + // PATCH superflu quand l'etat local est effectivement a jour. const wrapper = mountSelector() await wrapper.find('[data-testid="tile-1"]').trigger('click') await flushPromises() - expect(mockPatch).not.toHaveBeenCalled() + expect(mockPatch).toHaveBeenCalledWith( + '/me/current-site', + { site: '/api/sites/1' }, + expect.anything(), + ) }) it('rollback visuel : sur erreur PATCH, data-active-id revient au site initial', async () => { diff --git a/frontend/modules/sites/composables/__tests__/useCurrentSite.spec.ts b/frontend/modules/sites/composables/__tests__/useCurrentSite.spec.ts index 80819c8..104aa04 100644 --- a/frontend/modules/sites/composables/__tests__/useCurrentSite.spec.ts +++ b/frontend/modules/sites/composables/__tests__/useCurrentSite.spec.ts @@ -24,6 +24,14 @@ vi.stubGlobal('useAuthStore', () => ({ vi.stubGlobal('useI18n', () => ({ t: (key: string) => key, })) +// useSidebar est consomme par useCurrentSite pour rafraichir la sidebar +// apres un switch reussi. Stub minimal retournant un loadSidebar no-op. +vi.stubGlobal('useSidebar', () => ({ + loadSidebar: vi.fn(), +})) +// refreshNuxtData est appele apres un switch pour invalider les donnees +// de page precedemment fetchees. Stub no-op pour les tests unitaires. +vi.stubGlobal('refreshNuxtData', vi.fn()) const SITE_A: Site = { id: 1, diff --git a/frontend/modules/sites/composables/useCurrentSite.ts b/frontend/modules/sites/composables/useCurrentSite.ts index 2e727e9..da196d2 100644 --- a/frontend/modules/sites/composables/useCurrentSite.ts +++ b/frontend/modules/sites/composables/useCurrentSite.ts @@ -23,11 +23,21 @@ */ import { ref } from 'vue' import type { Site } from '~/shared/types/sites' +import { onAuthSessionCleared } from '~/shared/stores/auth' const currentSite = ref(null) const availableSites = ref([]) const switching = ref(false) +// Enregistrement unique au niveau module (singleton) : quand clearSession() +// est appelee par l'intercepteur 401 de useApi, le state local est purgé +// de la meme facon qu'au logout explicite (logout.vue). +onAuthSessionCleared(() => { + currentSite.value = null + availableSites.value = [] + switching.value = false +}) + export function useCurrentSite() { // Resolution au setup : les 3 services doivent etre invoques dans un // contexte composant. Leur capture ici permet a switchSite() de @@ -35,6 +45,7 @@ export function useCurrentSite() { const auth = useAuthStore() const api = useApi() const { t } = useI18n() + const { loadSidebar } = useSidebar() /** * Synchronise le state singleton depuis le store auth. A appeler au @@ -75,6 +86,21 @@ export function useCurrentSite() { // N'est appele qu'apres un succes HTTP donc pas de rollback a // prevoir sur cette ligne. auth.setCurrentSite(site) + + // Apres un switch reussi : recharger la sidebar (les filtres de + // modules peuvent dependre du site courant via SiteScopedQueryExtension) + // et invalider toutes les donnees de page pour eviter que l'utilisateur + // voie les donnees de l'ancien site sous un toast "Site change". + try { + await loadSidebar() + } catch { + // No-op : la sidebar non rafraichie n'est pas bloquante. + } + try { + await refreshNuxtData() + } catch { + // No-op : certaines pages n'ont pas de useAsyncData a invalider. + } } catch (error) { currentSite.value = previousLocal throw error diff --git a/frontend/modules/sites/pages/admin/sites.vue b/frontend/modules/sites/pages/admin/sites.vue index f0cdead..205dec1 100644 --- a/frontend/modules/sites/pages/admin/sites.vue +++ b/frontend/modules/sites/pages/admin/sites.vue @@ -63,6 +63,7 @@ import type { Site } from '~/shared/types/sites' const { t } = useI18n() const api = useApi() +const auth = useAuthStore() const { can } = usePermissions() const canManage = computed(() => can('sites.manage')) @@ -149,6 +150,11 @@ async function handleDelete() { siteToDelete.value = null drawerOpen.value = false await loadSites() + // Rafraichit auth.user apres suppression d'un site : le backend + // applique ON DELETE SET NULL sur user.current_site_id, donc + // auth.user.currentSite peut etre devenu null sans que le front + // le sache. refreshUser() resynchronise depuis GET /api/me. + await auth.refreshUser() } finally { deleting.value = false } diff --git a/frontend/shared/stores/auth.ts b/frontend/shared/stores/auth.ts index 76aece3..d448947 100644 --- a/frontend/shared/stores/auth.ts +++ b/frontend/shared/stores/auth.ts @@ -3,6 +3,24 @@ import type { UserData } from '~/shared/types/user-data' import type { Site } from '~/shared/types/sites' import { getCurrentUser, login, logout } from '~/shared/services/auth' +/** + * Callbacks enregistres par les composables singletons qui doivent + * reinitialiser leur etat quand la session est invalidee (ex: expiration + * JWT, logout depuis un intercepteur 401). Utilise le pattern + * "callback registration" (Option C) pour eviter une dependance croisee + * depuis shared/ vers modules/ — chaque composable s'auto-enregistre. + */ +const onSessionClearedCallbacks: Array<() => void> = [] + +/** + * Enregistre un callback a invoquer lorsque clearSession() est appelee. + * Typiquement invoque au setup-time du composable (module-level), donc + * une seule fois par instance de composable singleton. + */ +export function onAuthSessionCleared(cb: () => void): void { + onSessionClearedCallbacks.push(cb) +} + export const useAuthStore = defineStore('auth', { state: () => ({ user: null as UserData | null, @@ -17,6 +35,10 @@ export const useAuthStore = defineStore('auth', { this.user = null this.checked = true this.isLoading = false + // Notifie les composables singletons (useCurrentSite, etc.) afin + // qu'ils reinitialisation leur etat — necessaire quand la session + // est invalidee par un intercepteur 401 sans passer par logout.vue. + onSessionClearedCallbacks.forEach((cb) => cb()) }, async ensureSession() { if (this.checked) {