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)
This commit is contained in:
@@ -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\Processor\UserRbacProcessor;
|
||||||
use App\Module\Core\Infrastructure\ApiPlatform\State\Provider\MeProvider;
|
use App\Module\Core\Infrastructure\ApiPlatform\State\Provider\MeProvider;
|
||||||
use App\Module\Core\Infrastructure\Doctrine\DoctrineUserRepository;
|
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\Entity\Site;
|
||||||
use App\Module\Sites\Domain\Exception\SiteNotAuthorizedException;
|
use App\Shared\Domain\Contract\SiteInterface;
|
||||||
|
use App\Shared\Domain\Exception\SiteNotAuthorizedException;
|
||||||
use DateTimeImmutable;
|
use DateTimeImmutable;
|
||||||
use Doctrine\Common\Collections\ArrayCollection;
|
use Doctrine\Common\Collections\ArrayCollection;
|
||||||
use Doctrine\Common\Collections\Collection;
|
use Doctrine\Common\Collections\Collection;
|
||||||
@@ -112,17 +118,19 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
|||||||
/**
|
/**
|
||||||
* Sites autorises pour l'utilisateur (ticket 2 du module Sites).
|
* Sites autorises pour l'utilisateur (ticket 2 du module Sites).
|
||||||
*
|
*
|
||||||
* Relation ManyToMany avec table de jointure `user_site`. Fetch EAGER
|
* Relation ManyToMany avec table de jointure `user_site`. Fetch LAZY :
|
||||||
* pour la meme raison que `$rbacRoles` : garantir que `/api/me` et les
|
* le chargement est defere jusqu'a l'acces explicite a la collection.
|
||||||
* voters futurs aient toujours la collection hydratee, meme dans un
|
* MeProvider (ou un futur provider avec JOIN FETCH) est responsable de
|
||||||
* contexte de refresh JWT hors EntityManager. Le surcout SQL reste
|
* precharger cette collection pour /api/me afin d'eviter N+1.
|
||||||
* negligeable (≤ quelques sites par user en pratique).
|
*
|
||||||
|
* 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<int, Site>
|
* @var Collection<int, Site>
|
||||||
*/
|
*/
|
||||||
#[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')]
|
#[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;
|
private Collection $sites;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -136,11 +144,15 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
|||||||
* Doit TOUJOURS pointer vers un site present dans `$sites`. L'invariant
|
* Doit TOUJOURS pointer vers un site present dans `$sites`. L'invariant
|
||||||
* est enforce par UserRbacProcessor qui bascule automatiquement a `null`
|
* est enforce par UserRbacProcessor qui bascule automatiquement a `null`
|
||||||
* si le site courant est retire des sites autorises.
|
* 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')]
|
#[ORM\JoinColumn(name: 'current_site_id', referencedColumnName: 'id', nullable: true, onDelete: 'SET NULL')]
|
||||||
#[Groups(['me:read', 'user:list'])]
|
#[Groups(['me:read'])]
|
||||||
private ?Site $currentSite = null;
|
private ?SiteInterface $currentSite = null;
|
||||||
|
|
||||||
#[ORM\Column]
|
#[ORM\Column]
|
||||||
private ?string $password = null;
|
private ?string $password = null;
|
||||||
@@ -363,11 +375,15 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
|||||||
* Synchronise la collection inverse Site::$users en memoire pour eviter
|
* Synchronise la collection inverse Site::$users en memoire pour eviter
|
||||||
* un etat incoherent entre les deux cotes de la M2M dans une meme
|
* un etat incoherent entre les deux cotes de la M2M dans une meme
|
||||||
* session Doctrine (cf. ticket 2 review point #1).
|
* 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)) {
|
if (!$this->sites->contains($site)) {
|
||||||
$this->sites->add($site);
|
$this->sites->add($site);
|
||||||
|
// @phpstan-ignore-next-line : Site concret toujours passe en pratique
|
||||||
$site->addUser($this);
|
$site->addUser($this);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -381,9 +397,10 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
|||||||
* par UserRbacProcessor (cote applicatif) ou doit etre maintenu
|
* par UserRbacProcessor (cote applicatif) ou doit etre maintenu
|
||||||
* explicitement par l'appelant. Voir Risque 2 du ticket 2 spec.
|
* 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)) {
|
if ($this->sites->removeElement($site)) {
|
||||||
|
// @phpstan-ignore-next-line : Site concret toujours passe en pratique
|
||||||
$site->removeUser($this);
|
$site->removeUser($this);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -395,12 +412,12 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
|||||||
* collection autorisee, via comparaison d'identite d'objet Doctrine.
|
* collection autorisee, via comparaison d'identite d'objet Doctrine.
|
||||||
* Utilise par CurrentSiteProcessor pour valider un switch.
|
* Utilise par CurrentSiteProcessor pour valider un switch.
|
||||||
*/
|
*/
|
||||||
public function hasSite(Site $site): bool
|
public function hasSite(SiteInterface $site): bool
|
||||||
{
|
{
|
||||||
return $this->sites->contains($site);
|
return $this->sites->contains($site);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getCurrentSite(): ?Site
|
public function getCurrentSite(): ?SiteInterface
|
||||||
{
|
{
|
||||||
return $this->currentSite;
|
return $this->currentSite;
|
||||||
}
|
}
|
||||||
@@ -412,7 +429,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
|||||||
* "selectionner un site dans la liste autorisee", utiliser
|
* "selectionner un site dans la liste autorisee", utiliser
|
||||||
* switchCurrentSite() qui porte la garde domaine.
|
* switchCurrentSite() qui porte la garde domaine.
|
||||||
*/
|
*/
|
||||||
public function setCurrentSite(?Site $currentSite): static
|
public function setCurrentSite(?SiteInterface $currentSite): static
|
||||||
{
|
{
|
||||||
$this->currentSite = $currentSite;
|
$this->currentSite = $currentSite;
|
||||||
|
|
||||||
@@ -427,7 +444,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface
|
|||||||
*
|
*
|
||||||
* @throws SiteNotAuthorizedException si $site n'appartient pas a $this->sites
|
* @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)) {
|
if (!$this->hasSite($site)) {
|
||||||
throw SiteNotAuthorizedException::forSite($site);
|
throw SiteNotAuthorizedException::forSite($site);
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ use ApiPlatform\Metadata\Patch;
|
|||||||
use ApiPlatform\Metadata\Post;
|
use ApiPlatform\Metadata\Post;
|
||||||
use App\Module\Core\Domain\Entity\User;
|
use App\Module\Core\Domain\Entity\User;
|
||||||
use App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository;
|
use App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository;
|
||||||
|
use App\Shared\Domain\Contract\SiteInterface;
|
||||||
use DateTimeImmutable;
|
use DateTimeImmutable;
|
||||||
use Doctrine\Common\Collections\ArrayCollection;
|
use Doctrine\Common\Collections\ArrayCollection;
|
||||||
use Doctrine\Common\Collections\Collection;
|
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\UniqueConstraint(name: 'uniq_site_name', columns: ['name'])]
|
||||||
#[ORM\HasLifecycleCallbacks]
|
#[ORM\HasLifecycleCallbacks]
|
||||||
#[UniqueEntity(fields: ['name'], message: 'Un site avec ce nom existe deja.')]
|
#[UniqueEntity(fields: ['name'], message: 'Un site avec ce nom existe deja.')]
|
||||||
class Site
|
class Site implements SiteInterface
|
||||||
{
|
{
|
||||||
#[ORM\Id]
|
#[ORM\Id]
|
||||||
#[ORM\GeneratedValue]
|
#[ORM\GeneratedValue]
|
||||||
|
|||||||
@@ -4,24 +4,15 @@ declare(strict_types=1);
|
|||||||
|
|
||||||
namespace App\Module\Sites\Domain\Exception;
|
namespace App\Module\Sites\Domain\Exception;
|
||||||
|
|
||||||
use App\Module\Sites\Domain\Entity\Site;
|
use App\Shared\Domain\Exception\SiteNotAuthorizedException as SharedSiteNotAuthorizedException;
|
||||||
use DomainException;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Levee lorsqu'un utilisateur tente de selectionner comme site courant un
|
* Alias de retrocompatibilite vers Shared\Domain\Exception\SiteNotAuthorizedException.
|
||||||
* site qui ne fait pas partie de ses sites autorises.
|
|
||||||
*
|
*
|
||||||
* Exception purement domaine : la traduction HTTP (403) est faite par le
|
* La classe canonique a ete deplacee dans Shared pour rompre le couplage
|
||||||
* CurrentSiteProcessor via try/catch, aligne sur le pattern
|
* Core → Sites. Les consommateurs existants dans le module Sites
|
||||||
* SystemRoleDeletionException du module Core.
|
* (CurrentSiteProcessor) continuent de l'attraper ici sans modification.
|
||||||
|
*
|
||||||
|
* @see SharedSiteNotAuthorizedException
|
||||||
*/
|
*/
|
||||||
final class SiteNotAuthorizedException extends DomainException
|
final class SiteNotAuthorizedException extends SharedSiteNotAuthorizedException {}
|
||||||
{
|
|
||||||
public static function forSite(Site $site): self
|
|
||||||
{
|
|
||||||
return new self(sprintf(
|
|
||||||
'Le site "%s" ne fait pas partie de vos sites autorises.',
|
|
||||||
$site->getName(),
|
|
||||||
));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -4,8 +4,6 @@ declare(strict_types=1);
|
|||||||
|
|
||||||
namespace App\Shared\Domain\Contract;
|
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.
|
* 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.
|
* si le payload ne precise pas de site.
|
||||||
*
|
*
|
||||||
* L'implementation concrete doit :
|
* 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).
|
* - 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 :
|
* Ne PAS implementer cette interface pour :
|
||||||
* - Des entites globales (catalogue partage, roles, permissions, users).
|
* - Des entites globales (catalogue partage, roles, permissions, users).
|
||||||
* - Des entites dont le scope est "par tenant" plus large que le site
|
* - 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
|
interface SiteAwareInterface
|
||||||
{
|
{
|
||||||
public function getSite(): ?Site;
|
public function getSite(): ?SiteInterface;
|
||||||
|
|
||||||
public function setSite(Site $site): void;
|
public function setSite(SiteInterface $site): void;
|
||||||
}
|
}
|
||||||
|
|||||||
20
src/Shared/Domain/Contract/SiteInterface.php
Normal file
20
src/Shared/Domain/Contract/SiteInterface.php
Normal file
@@ -0,0 +1,20 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Shared\Domain\Contract;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Interface minimale exposant ce que le noyau (Shared/Core) doit connaitre
|
||||||
|
* d'un Site, sans creer de couplage direct vers le module Sites.
|
||||||
|
*
|
||||||
|
* Implemente par App\Module\Sites\Domain\Entity\Site.
|
||||||
|
* Utilisee comme type-hint dans SiteAwareInterface, User et toute entite
|
||||||
|
* Shared/Core qui manipule un site sans avoir besoin des details metier.
|
||||||
|
*/
|
||||||
|
interface SiteInterface
|
||||||
|
{
|
||||||
|
public function getId(): ?int;
|
||||||
|
|
||||||
|
public function getName(): ?string;
|
||||||
|
}
|
||||||
31
src/Shared/Domain/Exception/SiteNotAuthorizedException.php
Normal file
31
src/Shared/Domain/Exception/SiteNotAuthorizedException.php
Normal file
@@ -0,0 +1,31 @@
|
|||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace App\Shared\Domain\Exception;
|
||||||
|
|
||||||
|
use App\Shared\Domain\Contract\SiteInterface;
|
||||||
|
use DomainException;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Levee lorsqu'un utilisateur tente de selectionner comme site courant un
|
||||||
|
* site qui ne fait pas partie de ses sites autorises.
|
||||||
|
*
|
||||||
|
* Exception purement domaine : la traduction HTTP (403) est faite par le
|
||||||
|
* CurrentSiteProcessor via try/catch, aligne sur le pattern
|
||||||
|
* SystemRoleDeletionException du module Core.
|
||||||
|
*
|
||||||
|
* Deplacee dans Shared/Domain/Exception/ pour eviter que le module Core
|
||||||
|
* n'importe directement depuis le module Sites (violation du principe de
|
||||||
|
* non-couplage inter-modules).
|
||||||
|
*/
|
||||||
|
class SiteNotAuthorizedException extends DomainException
|
||||||
|
{
|
||||||
|
public static function forSite(SiteInterface $site): self
|
||||||
|
{
|
||||||
|
return new self(sprintf(
|
||||||
|
'Le site "%s" ne fait pas partie de vos sites autorises.',
|
||||||
|
$site->getName(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -6,7 +6,9 @@ namespace App\Tests\Fixtures\SiteAware;
|
|||||||
|
|
||||||
use App\Module\Sites\Domain\Entity\Site;
|
use App\Module\Sites\Domain\Entity\Site;
|
||||||
use App\Shared\Domain\Contract\SiteAwareInterface;
|
use App\Shared\Domain\Contract\SiteAwareInterface;
|
||||||
|
use App\Shared\Domain\Contract\SiteInterface;
|
||||||
use Doctrine\ORM\Mapping as ORM;
|
use Doctrine\ORM\Mapping as ORM;
|
||||||
|
use InvalidArgumentException;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Entite fictive utilisee UNIQUEMENT en tests (ticket 4 module Sites).
|
* Entite fictive utilisee UNIQUEMENT en tests (ticket 4 module Sites).
|
||||||
@@ -57,13 +59,16 @@ class FakeSiteAwareEntity implements SiteAwareInterface
|
|||||||
$this->name = $name;
|
$this->name = $name;
|
||||||
}
|
}
|
||||||
|
|
||||||
public function getSite(): ?Site
|
public function getSite(): ?SiteInterface
|
||||||
{
|
{
|
||||||
return $this->site;
|
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;
|
$this->site = $site;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user