diff --git a/config/packages/doctrine.yaml b/config/packages/doctrine.yaml index 2a0cbb6..c21cc4f 100644 --- a/config/packages/doctrine.yaml +++ b/config/packages/doctrine.yaml @@ -32,6 +32,20 @@ when@test: doctrine: dbal: dbname_suffix: '_test%env(default::TEST_TOKEN)%' + orm: + mappings: + # Entite fictive SiteAware utilisee uniquement en tests du + # module Sites (ticket 4). Le mapping n'est charge qu'en + # environnement test, donc aucun impact sur les schemas + # dev/prod. La table est creee a la volee par les tests + # d'integration (via `SchemaTool::createSchema`) dans le + # setUp de SiteScopedQueryExtensionTest. + TestFixtures: + type: attribute + is_bundle: false + dir: '%kernel.project_dir%/tests/Fixtures/SiteAware' + prefix: 'App\Tests\Fixtures\SiteAware' + alias: TestFixtures when@prod: doctrine: diff --git a/config/services.yaml b/config/services.yaml index 358541c..0bb0ab0 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -27,3 +27,6 @@ services: App\Module\Sites\Domain\Repository\SiteRepositoryInterface: alias: App\Module\Sites\Infrastructure\Doctrine\DoctrineSiteRepository + + App\Module\Sites\Application\Service\CurrentSiteProviderInterface: + alias: App\Module\Sites\Application\Service\CurrentSiteProvider diff --git a/docs/modules/site-aware.md b/docs/modules/site-aware.md new file mode 100644 index 0000000..5c460a1 --- /dev/null +++ b/docs/modules/site-aware.md @@ -0,0 +1,287 @@ +# Guide développeur — `SiteAwareInterface` (opt-in) + +Ce guide explique comment adopter le pattern **site-aware** sur une entité +d'un module métier pour que ses données soient automatiquement filtrées +par le site courant de l'utilisateur connecté, et pour que les créations +soient rattachées implicitement au site courant. + +Ce pattern est **opt-in strict** : aucune entité n'est affectée tant qu'un +module ne choisit pas explicitement d'implémenter `SiteAwareInterface`. + +Livré par le ticket 4/4 du module Sites (cf. `docs/sites/ticket-04-spec.md`). + +## 1. Quand adopter ? + +Adopte le pattern si : + +- Chaque ligne de l'entité appartient à **un et un seul site**. +- Les utilisateurs du site A ne doivent **jamais** voir les lignes du site B. +- Créer une ligne sans connaître le site n'a pas de sens métier. + +Exemples typiques : `Supplier`, `Order`, `StockEntry`, `Employee` (si chaque +site a sa propre équipe), `Invoice` (si facturation par site). + +## 2. Quand NE PAS adopter ? + +**Entités globales** : partagées par tous les sites, pas de notion de +propriétaire. Ne pas adopter. + +- `Role`, `Permission`, `User` (les users sont transverses, rattachés à + plusieurs sites via la relation M2M `user_site`). +- Catalogues mutualisés : produits, catégories, taxes — sauf si chaque + site a son propre catalogue. +- Documents / contrats multi-site (ex: contrat-cadre qui couvre plusieurs + sites). + +**Entités "par tenant"** : si le scope naturel est plus large que le site +(ex: un groupe qui possède plusieurs sites comme entités filiales +juridiquement distinctes), utilise plutôt `TenantAwareInterface` (déjà +présent dans `src/Shared/Domain/Contract/`). + +**Entités hybrides** : certaines lignes globales, d'autres par site. Le +pattern ne supporte pas ce cas — crée deux entités distinctes si +nécessaire. + +## 3. Comment adopter ? Check-list + +### 3.1 Entité + +```php +use App\Module\Sites\Domain\Entity\Site; +use App\Shared\Domain\Contract\SiteAwareInterface; + +class Supplier implements SiteAwareInterface +{ + #[ORM\ManyToOne(targetEntity: Site::class)] + #[ORM\JoinColumn(name: 'site_id', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')] + private ?Site $site = null; + + public function getSite(): ?Site + { + return $this->site; + } + + public function setSite(Site $site): void + { + $this->site = $site; + } +} +``` + +Points critiques : + +- `nullable: false` au niveau de la `JoinColumn` — la table n'accepte + jamais `site_id IS NULL` en régime nominal. +- `onDelete: 'CASCADE'` — la suppression d'un site entraîne la suppression + de toutes les lignes associées. À remplacer par `RESTRICT` (blocage) si + ton métier exige d'empêcher la suppression d'un site contenant des + données. +- Le getter retourne `?Site` (nullable) pour permettre des états + transitoires pré-persist (entité construite avant injection du site). + +### 3.2 Migration + +**Cas 1 — Nouvelle table** : ajoute directement `site_id INT NOT NULL` +avec FK et index. + +**Cas 2 — Table existante avec données legacy** : migration en trois étapes +distinctes. + +```php +// Version1.php +$this->addSql('ALTER TABLE supplier ADD site_id INT DEFAULT NULL'); +$this->addSql('CREATE INDEX IDX_supplier_site ON supplier (site_id)'); +$this->addSql('ALTER TABLE supplier ADD CONSTRAINT FK_supplier_site FOREIGN KEY (site_id) REFERENCES site (id) ON DELETE CASCADE'); + +// Backfill (manuellement ou via script custom selon ton métier) +$this->addSql("UPDATE supplier SET site_id = (SELECT id FROM site WHERE name = 'Chatellerault') WHERE site_id IS NULL"); + +// Version2.php — après backfill confirmé +$this->addSql('ALTER TABLE supplier ALTER COLUMN site_id SET NOT NULL'); +``` + +**Index obligatoire** : le filtre généré par `SiteScopedQueryExtension` +est `WHERE x.site = :currentSite`. Sans index sur `site_id`, chaque +requête fait un full-scan de la table. Ajoute-le dans la migration. + +### 3.3 Sérialisation API + +Expose la relation `site` dans le groupe de lecture de la ressource pour +que le frontend sache à quel site appartient chaque ligne : + +```php +#[Groups(['supplier:read'])] +private ?Site $site = null; +``` + +Si tu veux aussi permettre à un admin de créer une ligne sur un autre +site que son `currentSite` (ex: admin multi-site), ajoute aussi le groupe +d'écriture : + +```php +#[Groups(['supplier:read', 'supplier:write'])] +``` + +Dans ce cas, `SiteAwareInjectionProcessor` respecte la valeur explicite +envoyée par le client (voir §4). + +### 3.4 Processor custom + +Si le module a déjà un processor custom sur les opérations POST/PATCH, +assure-toi qu'il délègue à `api_platform.doctrine.orm.state.persist_processor` +(et non à `$em->persist()` direct) pour que le decorator +`SiteAwareInjectionProcessor` s'applique. + +Pattern aligné sur `UserRbacProcessor` : + +```php +use Symfony\Component\DependencyInjection\Attribute\Autowire; + +public function __construct( + #[Autowire(service: 'api_platform.doctrine.orm.state.persist_processor')] + private readonly ProcessorInterface $persistProcessor, +) {} + +public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed +{ + // Gardes métier custom ici... + + // Délègue au persist processor décoré : SiteAwareInjectionProcessor + // interceptera l'appel et injectera le currentSite si besoin. + return $this->persistProcessor->process($data, $operation, $uriVariables, $context); +} +``` + +## 4. Comportement du processor d'injection + +Le decorator `SiteAwareInjectionProcessor` s'applique automatiquement à +**toute** persistance API Platform. Son comportement : + +| Cas | Action | +|---|---| +| `$data` n'implémente pas `SiteAwareInterface` | Délégation directe (no-op). | +| `$data` est `SiteAware` avec `$site` déjà positionné (ex: payload POST avec `site` explicite) | Délégation directe, **la valeur explicite est préservée**. | +| `$data` est `SiteAware` sans site, `CurrentSiteProvider::get()` retourne un `Site` | Injection `$data->setSite($currentSite)` puis délégation. | +| `$data` est `SiteAware` sans site, `CurrentSiteProvider::get()` retourne `null` | **Throw `BadRequestHttpException`** avec message "aucun site sélectionné". | + +Conséquence : un user sans `currentSite` ne peut **pas** créer de ligne +sur une entité `SiteAware`. C'est intentionnel : mieux vaut un 400 clair +que persister une ligne incohérente. + +## 5. Comportement en mode dégradé + +### 5.1 Module Sites désactivé + +Si `SitesModule::class` est retiré de `config/modules.php`, +`CurrentSiteProvider::get()` retourne **toujours `null`** : + +- `SiteScopedQueryExtension` → no-op. Toutes les lignes visibles, comme + si le filtre n'existait pas. +- `SiteAwareInjectionProcessor` → **throw 400 sur tout POST/PATCH** sans + site explicite. L'écriture d'entités `SiteAware` nécessite que le + client envoie systématiquement `site` dans le payload. + +**Conséquence** : un module qui adopte le pattern **ne peut pas vivre** +sans le module Sites actif pour les opérations d'écriture. À documenter +fortement dans le README du module adopté. + +### 5.2 User sans site (sites = [], currentSite = null) + +Même comportement qu'un module désactivé : lecture no-op (tout visible), +écriture bloquée par 400. L'UX doit gérer ce cas (ex: écran d'onboarding +qui force l'assignation d'un site avant d'accéder aux écrans métier). + +### 5.3 Bypass admin via `sites.bypass_scope` + +Un utilisateur avec la permission `sites.bypass_scope` (ou admin par +bypass total via `isAdmin = true`) voit **toutes** les lignes, tous +sites confondus. Pratique pour audit, reporting, consolidation groupe. + +Le processor d'injection ne respecte **pas** ce bypass : même un user +avec `bypass_scope` verra son `currentSite` injecté à la création s'il +n'envoie pas de `site` explicite. Le bypass est un droit de lecture, +pas d'écriture multi-site. + +## 6. Anti-patterns et gotchas + +### 6.1 Sous-collections (`/api/clients/{id}/contacts`) + +Si seul `Client` est `SiteAware` (et `Contact` hérite du scope via son +parent), le filtre **ne se propage pas automatiquement** aux contacts. +Deux options : + +- Rendre `Contact` aussi `SiteAware` (redondance mais simple). +- Ajouter un filtre custom qui joint sur `contact.client.site` et compare + au `currentSite`. + +Ce ticket ne couvre pas le second cas : à implémenter par le module +concerné. + +### 6.2 Repositories custom + +Le filtre API Platform ne s'applique **qu'aux requêtes** générées par +API Platform (via `ItemProvider` / `CollectionProvider` Doctrine). Si un +repository custom fait une requête DQL manuelle (ex: `findTopRated()` +appelé depuis un service), **aucun filtre n'est appliqué**. + +Responsabilité du développeur du module : injecter `CurrentSiteProvider` +dans le repository / service et ajouter manuellement la clause WHERE. + +### 6.3 Tests d'intégration + +Les tests qui persistent des entités `SiteAware` doivent : + +- Soit logger un user avec un `currentSite` positionné (cas nominal). +- Soit utiliser un user avec `sites.bypass_scope` pour voir toutes les + lignes (cas reporting). +- Soit positionner le site **explicitement** sur chaque entité persistée + via fixture (bypass du processor d'injection qui n'est pas actif hors + contexte HTTP). + +### 6.4 Cascade delete d'un site + +La migration type du §3.2 déclare `onDelete: 'CASCADE'` sur la FK +`site_id`. **Conséquence** : supprimer un site détruit **toutes** les +lignes de **toutes** les tables `SiteAware` rattachées à ce site, en +cascade. Pour un `Supplier`, ça signifie perte de l'historique fournisseur +du site supprimé. + +Alternatives selon le besoin métier : + +- `onDelete: 'RESTRICT'` : bloque la suppression du site tant qu'il reste + des lignes. L'admin doit nettoyer manuellement avant delete. +- `onDelete: 'SET NULL'` : transforme les lignes en "globales" après + suppression du site — mais incompatible avec `nullable: false`, donc + nécessite de relâcher la contrainte. Généralement à éviter. + +## 7. Exemple d'adoption minimale (pseudo-ticket) + +Pour un futur ticket qui adopte `SiteAwareInterface` sur `Supplier` du +module Commercial : + +1. Modifier `src/Module/Commercial/Domain/Entity/Supplier.php` : ajouter + `implements SiteAwareInterface` + relation `$site`. +2. Créer migration `Version.php` : `ALTER TABLE supplier ADD + site_id ...`, backfill, `SET NOT NULL`, `CREATE INDEX`. +3. Ajouter `#[Groups(['supplier:read'])]` sur `$site`. +4. Mettre à jour les fixtures `CommercialFixtures` pour rattacher chaque + supplier à un site (`setSite(...)`). +5. Ajouter un test d'intégration qui vérifie que la collection + `/api/suppliers` retourne bien uniquement les suppliers du site + courant pour un user donné. +6. Documenter dans le README du module Commercial que les opérations + d'écriture sur Supplier nécessitent le module Sites actif + un user + avec `currentSite`. + +## 8. Permission `sites.bypass_scope` + +Déclarée par `SitesModule::permissions()`, synchronisée automatiquement +en base par `app:sync-permissions`. Une fois synchronisée, elle est +assignable : + +- Directement à un user via `/admin/users` (drawer RBAC, section + "Permissions directes"). +- Via un rôle personnalisé (ex: rôle "Auditeur groupe") qui la porte. + +Les admins l'obtiennent automatiquement par bypass total (`isAdmin`), pas +besoin d'assignation explicite. diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 539a0bd..6c174d7 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -7,7 +7,7 @@ "name": "coltura-frontend", "hasInstallScript": true, "dependencies": { - "@malio/layer-ui": "^1.4.0", + "@malio/layer-ui": "^1.4.2", "@nuxt/icon": "^2.2.1", "@nuxtjs/i18n": "^10.2.3", "@nuxtjs/tailwindcss": "^6.14.0", @@ -1820,9 +1820,9 @@ "license": "MIT" }, "node_modules/@malio/layer-ui": { - "version": "1.4.0", - "resolved": "https://gitea.malio.fr/api/packages/MALIO-DEV/npm/%40malio%2Flayer-ui/-/1.4.0/layer-ui-1.4.0.tgz", - "integrity": "sha512-2LBe/WqOwNw61Y+9y2SDgsB3/JCTS7VOYfQHFLMb6GXOIj1Vmjxqf8GEzQOzre4pGI+n8w2o+VVn6ttQIkBtzA==", + "version": "1.4.2", + "resolved": "https://gitea.malio.fr/api/packages/MALIO-DEV/npm/%40malio%2Flayer-ui/-/1.4.2/layer-ui-1.4.2.tgz", + "integrity": "sha512-H8f5FJXHFH9ZI1Jx4u9XE7w6VlR/d9Zr2encfQyMax1I0UZ3SiGBUjictcL33r0OhgsrgSmPq0J9aF6aab85Nw==", "dependencies": { "@nuxt/icon": "^2.2.1", "@nuxtjs/tailwindcss": "^6.14.0", diff --git a/frontend/package.json b/frontend/package.json index 4b12611..f5f8f94 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -15,7 +15,7 @@ "test:watch": "vitest" }, "dependencies": { - "@malio/layer-ui": "^1.4.0", + "@malio/layer-ui": "^1.4.2", "@nuxt/icon": "^2.2.1", "@nuxtjs/i18n": "^10.2.3", "@nuxtjs/tailwindcss": "^6.14.0", diff --git a/frontend/tailwind.config.ts b/frontend/tailwind.config.ts index bbd44cc..0730ee3 100644 --- a/frontend/tailwind.config.ts +++ b/frontend/tailwind.config.ts @@ -1,13 +1,26 @@ import type {Config} from 'tailwindcss' +/** + * Config Tailwind du projet Coltura. + * + * @nuxtjs/tailwindcss merge automatiquement les configs de chaque layer + * Nuxt declare dans `nuxt.config.ts:extends`. Le layer `@malio/layer-ui` + * apporte deja : + * - borderRadius.malio (var CSS --m-radius) + * - colors.m.{primary,surface,border,text,muted,bg,disabled,danger, + * success,btn-*,site-blue,site-yellow,site-green} + * - fontFamily.sans (Helvetica Neue) + * + * Cette config locale ne redeclare QUE ce qui est specifique a Coltura + * ou absent de la config Malio — evite la duplication et les derives. + */ export default >{ darkMode: 'class', theme: { extend: { - fontFamily: { - sans: ['"Helvetica Neue"', 'Helvetica', 'Arial', 'sans-serif'] - }, colors: { + // Couleurs applicatives Coltura (hors namespace `m` reserve + // au design system Malio partage). primary: { 500: '#222783', }, @@ -20,27 +33,10 @@ export default >{ blue: { 500: '#056CF2' }, + // Extensions au namespace `m` non couvertes par Malio 1.4.1. m: { - primary: 'rgb(var(--m-primary) / )', secondary: 'rgb(var(--m-secondary, 75 77 237) / )', tertiary: 'rgb(var(--m-tertiary, 243 244 248) / )', - border: 'rgb(var(--m-border) / )', - text: 'rgb(var(--m-text) / )', - muted: 'rgb(var(--m-muted) / )', - bg: 'rgb(var(--m-bg) / )', - surface: 'rgb(var(--m-surface) / )', - disabled: 'rgb(var(--m-disabled) / )', - danger: 'rgb(var(--m-danger) / )', - success: 'rgb(var(--m-success) / )', - 'btn-primary': 'rgb(var(--m-btn-primary) / )', - 'btn-primary-hover': 'rgb(var(--m-btn-primary-hover) / )', - 'btn-primary-active': 'rgb(var(--m-btn-primary-active) / )', - 'btn-secondary': 'rgb(var(--m-btn-secondary) / )', - 'btn-secondary-hover': 'rgb(var(--m-btn-secondary-hover) / )', - 'btn-secondary-active': 'rgb(var(--m-btn-secondary-active) / )', - 'btn-danger': 'rgb(var(--m-btn-danger) / )', - 'btn-danger-hover': 'rgb(var(--m-btn-danger-hover) / )', - 'btn-danger-active': 'rgb(var(--m-btn-danger-active) / )', } } } diff --git a/makefile b/makefile index 157084a..c1e0965 100644 --- a/makefile +++ b/makefile @@ -85,10 +85,23 @@ migration-migrate: # Cree et initialise la base de test utilisee par PHPUnit # (le suffixe "_test" est applique automatiquement par Doctrine en APP_ENV=test) -# Ordre : fixtures -> sync-permissions, car fixtures:load purge la table permission +# +# Ordre : +# 1. migrations : crees le schema metier reel. +# 2. schema:update : cree les tables mappees en `when@test` uniquement +# (ex: fake_site_aware_entity du ticket 4) qui n'ont pas de migration. +# `--force` sans `--complete` : ajoute les tables manquantes aux +# mappings sans drop les tables DB non mappees (no-op sur un schema +# deja aligne avec les migrations). Necessaire car le purger +# doctrine:fixtures:load essaie de DELETE toutes les tables connues +# via les mappings — si fake_site_aware_entity est mappe mais absent +# en DB, le purger crash. +# 3. fixtures -> sync-permissions : fixtures:load purge la table permission, +# donc sync doit passer apres. test-db-setup: $(SYMFONY_CONSOLE) doctrine:database:create --env=test --if-not-exists $(SYMFONY_CONSOLE) doctrine:migrations:migrate --env=test --no-interaction + $(SYMFONY_CONSOLE) doctrine:schema:update --env=test --force $(SYMFONY_CONSOLE) --env=test --no-interaction doctrine:fixtures:load $(SYMFONY_CONSOLE) --env=test --no-interaction app:sync-permissions diff --git a/src/Module/Sites/Application/Service/CurrentSiteProvider.php b/src/Module/Sites/Application/Service/CurrentSiteProvider.php new file mode 100644 index 0000000..0cce459 --- /dev/null +++ b/src/Module/Sites/Application/Service/CurrentSiteProvider.php @@ -0,0 +1,69 @@ +sitesActive = in_array(SitesModule::class, $moduleClasses, true); + } + + /** + * Retourne le site courant de l'utilisateur authentifie, ou null si + * l'une des 3 conditions de desactivation est remplie (cf. docblock + * de classe). + */ + public function get(): ?Site + { + if (!$this->sitesActive) { + return null; + } + + $user = $this->security->getUser(); + if (!$user instanceof User) { + return null; + } + + return $user->getCurrentSite(); + } +} diff --git a/src/Module/Sites/Application/Service/CurrentSiteProviderInterface.php b/src/Module/Sites/Application/Service/CurrentSiteProviderInterface.php new file mode 100644 index 0000000..deff2ed --- /dev/null +++ b/src/Module/Sites/Application/Service/CurrentSiteProviderInterface.php @@ -0,0 +1,25 @@ +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); + } + + /** + * Ajoute la clause `WHERE .site = :currentSite` au query builder + * si les 3 conditions d'application sont remplies. No-op sinon. + */ + private function applyScope( + QueryBuilder $queryBuilder, + QueryNameGeneratorInterface $queryNameGenerator, + string $resourceClass, + ): void { + // 1) Filtrer uniquement les resources qui ont opt-in via l'interface. + // `is_subclass_of` gere a la fois `implements` direct et herite. + if (!is_subclass_of($resourceClass, SiteAwareInterface::class)) { + return; + } + + // 2) Admin ou user avec bypass explicite : visibilite globale. + // is_granted('sites.bypass_scope') retourne true pour les admins + // (bypass total via isAdmin) meme sans permission explicite. + if ($this->security->isGranted('sites.bypass_scope')) { + return; + } + + // 3) Pas de site courant -> no-op plutot que collection vide. + // Decision assumee (cf. ticket 4 spec Risque 1) : un user sans + // currentSite voit tout. L'alternative "collection vide" est + // rejetee car elle rendrait l'app inutilisable pour un user + // mal configure. + $currentSite = $this->currentSiteProvider->get(); + if (null === $currentSite) { + return; + } + + // Application du filtre : alias racine du QueryBuilder, parametre + // genere pour eviter les collisions avec d'autres extensions. + $rootAlias = $queryBuilder->getRootAliases()[0]; + $parameterName = $queryNameGenerator->generateParameterName('currentSite'); + + $queryBuilder + ->andWhere(sprintf('%s.site = :%s', $rootAlias, $parameterName)) + ->setParameter($parameterName, $currentSite) + ; + } +} diff --git a/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php b/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php new file mode 100644 index 0000000..8d31125 --- /dev/null +++ b/src/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessor.php @@ -0,0 +1,64 @@ +persistProcessor->process()` (pattern UserRbacProcessor) passent + * aussi par ce decorator, transparent pour les entites non-SiteAware. + * + * 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 sans site, provider retourne un Site -> injection + * puis delegation. + * - $data SiteAware sans site, provider retourne null -> throw 400 + * BadRequestHttpException avec message explicite. + * + * Volontairement HTTP-only : ne couvre pas les persistances hors API + * Platform (fixtures, commandes CLI, imports). Ces contextes doivent + * positionner le site explicitement — c'est assume dans la doc + * d'adoption (`docs/modules/site-aware.md`). + * + * @implements ProcessorInterface + */ +#[AsDecorator('api_platform.doctrine.orm.state.persist_processor')] +final class SiteAwareInjectionProcessor implements ProcessorInterface +{ + public function __construct( + private readonly ProcessorInterface $inner, + private readonly CurrentSiteProviderInterface $currentSiteProvider, + ) {} + + public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed + { + if ($data instanceof SiteAwareInterface && null === $data->getSite()) { + $currentSite = $this->currentSiteProvider->get(); + + if (null === $currentSite) { + throw new BadRequestHttpException( + 'Impossible de creer l\'enregistrement : aucun site selectionne.', + ); + } + + $data->setSite($currentSite); + } + + return $this->inner->process($data, $operation, $uriVariables, $context); + } +} diff --git a/src/Module/Sites/SitesModule.php b/src/Module/Sites/SitesModule.php index 2cdf26c..d1cceb5 100644 --- a/src/Module/Sites/SitesModule.php +++ b/src/Module/Sites/SitesModule.php @@ -32,6 +32,7 @@ final class SitesModule return [ ['code' => 'sites.view', 'label' => 'Voir les sites'], ['code' => 'sites.manage', 'label' => 'Gerer les sites (creer, editer, supprimer)'], + ['code' => 'sites.bypass_scope', 'label' => 'Voir les donnees site-scoped de tous les sites (bypass du filtrage)'], ]; } } diff --git a/src/Shared/Domain/Contract/SiteAwareInterface.php b/src/Shared/Domain/Contract/SiteAwareInterface.php new file mode 100644 index 0000000..178f4c6 --- /dev/null +++ b/src/Shared/Domain/Contract/SiteAwareInterface.php @@ -0,0 +1,35 @@ +name = $name; + } + + public function getId(): ?int + { + return $this->id; + } + + public function getName(): string + { + return $this->name; + } + + public function setName(string $name): void + { + $this->name = $name; + } + + public function getSite(): ?Site + { + return $this->site; + } + + public function setSite(Site $site): void + { + $this->site = $site; + } +} diff --git a/tests/Module/Sites/Application/Service/CurrentSiteProviderTest.php b/tests/Module/Sites/Application/Service/CurrentSiteProviderTest.php new file mode 100644 index 0000000..335c42a --- /dev/null +++ b/tests/Module/Sites/Application/Service/CurrentSiteProviderTest.php @@ -0,0 +1,110 @@ +setCurrentSite(new Site('Site', 'Rue', null, '12345', 'Ville', '#000000')); + + $security = $this->createStub(Security::class); + $security->method('getUser')->willReturn($user); + + $provider = $this->makeProvider($security, sitesActive: false); + + self::assertNull($provider->get()); + } + + public function testReturnsNullIfNoUser(): void + { + $security = $this->createStub(Security::class); + $security->method('getUser')->willReturn(null); + + $provider = $this->makeProvider($security, sitesActive: true); + + self::assertNull($provider->get()); + } + + public function testReturnsNullIfUserIsNotAppUser(): void + { + // Un InMemoryUser Symfony n'est pas une instance de App\User donc + // le provider ne peut pas lire son currentSite -> null defensif. + $security = $this->createStub(Security::class); + $security->method('getUser')->willReturn(new InMemoryUser('foo', 'bar')); + + $provider = $this->makeProvider($security, sitesActive: true); + + self::assertNull($provider->get()); + } + + public function testReturnsNullIfUserHasNoCurrentSite(): void + { + $user = new User(); + // Pas d'appel a setCurrentSite, donc null par defaut. + + $security = $this->createStub(Security::class); + $security->method('getUser')->willReturn($user); + + $provider = $this->makeProvider($security, sitesActive: true); + + self::assertNull($provider->get()); + } + + public function testReturnsSiteWhenAllConditionsMet(): void + { + $site = new Site('Chatellerault', 'Rue', null, '86100', 'Chatellerault', '#056CF2'); + $user = new User(); + $user->setCurrentSite($site); + + $security = $this->createStub(Security::class); + $security->method('getUser')->willReturn($user); + + $provider = $this->makeProvider($security, sitesActive: true); + + self::assertSame($site, $provider->get()); + } + + /** + * Factory helper : construit un provider avec `$sitesActive` force a + * la valeur donnee, bypassant la lecture reelle de config/modules.php. + */ + private function makeProvider(Security $security, bool $sitesActive): CurrentSiteProvider + { + // Instance via reflection pour eviter l'appel reel au constructeur + // qui require config/modules.php (non deterministe en test unit). + $reflection = new ReflectionClass(CurrentSiteProvider::class); + $provider = $reflection->newInstanceWithoutConstructor(); + + $securityProp = $reflection->getProperty('security'); + $securityProp->setValue($provider, $security); + + $sitesActiveProp = $reflection->getProperty('sitesActive'); + $sitesActiveProp->setValue($provider, $sitesActive); + + return $provider; + } +} diff --git a/tests/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteScopedQueryExtensionTest.php b/tests/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteScopedQueryExtensionTest.php new file mode 100644 index 0000000..cfc100b --- /dev/null +++ b/tests/Module/Sites/Infrastructure/ApiPlatform/Extension/SiteScopedQueryExtensionTest.php @@ -0,0 +1,237 @@ +get(EntityManagerInterface::class); + $this->em = $em; + + // Creation de la table fake_site_aware_entity uniquement. + // La base de test partage deja les autres tables (site, user, etc.). + $metadata = $this->em->getClassMetadata(FakeSiteAwareEntity::class); + $schema = new SchemaTool($this->em); + // Drop si existe deja (re-run des tests), puis create. + $schema->dropSchema([$metadata]); + $schema->createSchema([$metadata]); + + // Fixtures locales : 2 entites sur siteA, 1 sur siteB. + $this->siteA = $this->em->getRepository(Site::class)->findOneBy(['name' => 'Chatellerault']); + $this->siteB = $this->em->getRepository(Site::class)->findOneBy(['name' => 'Saint-Jean']); + self::assertNotNull($this->siteA); + self::assertNotNull($this->siteB); + + $e1 = new FakeSiteAwareEntity('A-in-site-A'); + $e1->setSite($this->siteA); + $e2 = new FakeSiteAwareEntity('B-in-site-A'); + $e2->setSite($this->siteA); + $e3 = new FakeSiteAwareEntity('C-in-site-B'); + $e3->setSite($this->siteB); + + $this->em->persist($e1); + $this->em->persist($e2); + $this->em->persist($e3); + $this->em->flush(); + $this->em->clear(); + } + + protected function tearDown(): void + { + // Drop de la table fake entre tests pour eviter toute pollution. + if (isset($this->em)) { + $metadata = $this->em->getClassMetadata(FakeSiteAwareEntity::class); + $schema = new SchemaTool($this->em); + $schema->dropSchema([$metadata]); + $this->em->close(); + } + + parent::tearDown(); + } + + public function testCollectionFilteredByCurrentSite(): void + { + $extension = $this->makeExtension($this->siteA); + + $results = $this->runQuery($extension, FakeSiteAwareEntity::class); + + self::assertCount(2, $results, '2 entites sur siteA doivent etre retournees.'); + foreach ($results as $entity) { + self::assertSame($this->siteA->getId(), $entity->getSite()->getId()); + } + } + + public function testCollectionSwitchesToSiteB(): void + { + $extension = $this->makeExtension($this->siteB); + + $results = $this->runQuery($extension, FakeSiteAwareEntity::class); + + self::assertCount(1, $results); + self::assertSame($this->siteB->getId(), $results[0]->getSite()->getId()); + } + + public function testNoOpIfNoCurrentSite(): void + { + // Decision assumee (ticket 4 spec Risque 1) : no-op plutot que + // collection vide. L'user sans currentSite voit TOUTES les entites. + $extension = $this->makeExtension(currentSite: null); + + $results = $this->runQuery($extension, FakeSiteAwareEntity::class); + + self::assertCount(3, $results); + } + + public function testNoOpIfBypassScopePermission(): void + { + // User avec sites.bypass_scope voit TOUTES les entites, meme + // avec un currentSite positionne. Comportement admin / audit. + $extension = $this->makeExtension($this->siteA, bypassScope: true); + + $results = $this->runQuery($extension, FakeSiteAwareEntity::class); + + self::assertCount(3, $results); + } + + public function testNoOpIfResourceClassNotSiteAware(): void + { + // Une resource qui n'implemente pas SiteAwareInterface ne doit + // jamais etre filtree (l'extension se contente d'un `return` tot). + $extension = $this->makeExtension($this->siteA); + + // On query les users (non SiteAware). Verification robuste : on + // inspecte la partie WHERE du QueryBuilder avant et apres l'appel + // a l'extension. Le before/after doit etre identique (idealement + // null dans les deux cas vu qu'on n'a pas ajoute de WHERE). + $qb = $this->em->createQueryBuilder()->select('u')->from(User::class, 'u'); + $nameGen = new QueryNameGenerator(); + + $whereBefore = $qb->getDQLPart('where'); + $extension->applyToCollection($qb, $nameGen, User::class); + $whereAfter = $qb->getDQLPart('where'); + + self::assertEquals( + $whereBefore, + $whereAfter, + 'La clause WHERE du QueryBuilder doit etre intacte pour une resource non SiteAware.', + ); + } + + public function testItemNotFoundIfWrongSite(): void + { + // GET /api/entity/{id} pour un item du siteB alors que l'user est + // sur siteA -> le filtre ajoute `WHERE site = siteA`, la requete + // retourne null -> API Platform renverra 404. + $em = $this->em; + $entityB = $em->getRepository(FakeSiteAwareEntity::class) + ->findOneBy(['name' => 'C-in-site-B']) + ; + self::assertNotNull($entityB); + $idB = $entityB->getId(); + $em->clear(); + + $extension = $this->makeExtension($this->siteA); + + $qb = $this->em->createQueryBuilder() + ->select('e') + ->from(FakeSiteAwareEntity::class, 'e') + ->andWhere('e.id = :id') + ->setParameter('id', $idB) + ; + $nameGen = new QueryNameGenerator(); + + $extension->applyToItem($qb, $nameGen, FakeSiteAwareEntity::class, ['id' => $idB]); + + self::assertNull($qb->getQuery()->getOneOrNullResult()); + } + + public function testItemFoundIfCorrectSite(): void + { + $entityA = $this->em->getRepository(FakeSiteAwareEntity::class) + ->findOneBy(['name' => 'A-in-site-A']) + ; + self::assertNotNull($entityA); + $idA = $entityA->getId(); + $this->em->clear(); + + $extension = $this->makeExtension($this->siteA); + + $qb = $this->em->createQueryBuilder() + ->select('e') + ->from(FakeSiteAwareEntity::class, 'e') + ->andWhere('e.id = :id') + ->setParameter('id', $idA) + ; + $nameGen = new QueryNameGenerator(); + + $extension->applyToItem($qb, $nameGen, FakeSiteAwareEntity::class, ['id' => $idA]); + + $result = $qb->getQuery()->getOneOrNullResult(); + self::assertNotNull($result); + self::assertSame('A-in-site-A', $result->getName()); + } + + /** + * Construit une extension avec un provider et un security mockes selon + * le scenario testé. Passe par reflection pour forcer le flag + * sitesActive du provider sans toucher au filesystem. + */ + private function makeExtension(?Site $currentSite, bool $bypassScope = false): SiteScopedQueryExtension + { + // createStub : pas d'attentes sur le nombre d'appels, juste fixer + // les valeurs de retour des methodes sollicitees. Evite les notices + // PHPUnit "No expectations configured". + $security = $this->createStub(Security::class); + $security->method('isGranted')->willReturnCallback( + fn (string $perm): bool => 'sites.bypass_scope' === $perm && $bypassScope, + ); + + $provider = $this->createStub(CurrentSiteProviderInterface::class); + $provider->method('get')->willReturn($currentSite); + + return new SiteScopedQueryExtension($provider, $security); + } + + private function runQuery(SiteScopedQueryExtension $extension, string $resourceClass): array + { + $qb = $this->em->createQueryBuilder()->select('e')->from($resourceClass, 'e'); + $nameGen = new QueryNameGenerator(); + + $extension->applyToCollection($qb, $nameGen, $resourceClass); + + return $qb->getQuery()->getResult(); + } +} diff --git a/tests/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessorTest.php b/tests/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessorTest.php new file mode 100644 index 0000000..65c1e54 --- /dev/null +++ b/tests/Module/Sites/Infrastructure/ApiPlatform/State/Processor/SiteAwareInjectionProcessorTest.php @@ -0,0 +1,152 @@ +process est + * toujours invoque sauf en cas de throw, (c) le throw 400 explicite si + * $data SiteAware sans site + provider null. + * + * @internal + */ +final class SiteAwareInjectionProcessorTest extends TestCase +{ + public function testInjectsCurrentSiteOnSiteAwareEntityWithoutSite(): void + { + $currentSite = new Site('Chatellerault', 'Rue', null, '86100', 'Chatellerault', '#056CF2'); + $data = $this->makeSiteAwareStub(null); + + $inner = $this->createMock(ProcessorInterface::class); + $inner->expects(self::once()) + ->method('process') + ->willReturnArgument(0) + ; + + $processor = $this->makeProcessor($inner, $currentSite); + $processor->process($data, $this->makeOperation()); + + self::assertSame($currentSite, $data->getSite()); + } + + public function testDoesNotOverrideExistingSite(): void + { + $existingSite = new Site('Existing', 'Rue', null, '12345', 'Ville', '#000000'); + $currentSite = new Site('Chatellerault', 'Rue', null, '86100', 'Chatellerault', '#056CF2'); + $data = $this->makeSiteAwareStub($existingSite); + + $inner = $this->createMock(ProcessorInterface::class); + $inner->expects(self::once())->method('process'); + + $processor = $this->makeProcessor($inner, $currentSite); + $processor->process($data, $this->makeOperation()); + + self::assertSame( + $existingSite, + $data->getSite(), + 'Un site deja positionne doit etre preserve, pas ecrase par le currentSite.', + ); + } + + public function testSkipsNonSiteAwareData(): void + { + $nonSiteAware = new stdClass(); + + $inner = $this->createMock(ProcessorInterface::class); + $inner->expects(self::once()) + ->method('process') + ->with($nonSiteAware) + ; + + $processor = $this->makeProcessor( + $inner, + new Site('Any', 'Rue', null, '12345', 'Ville', '#000000'), + ); + $processor->process($nonSiteAware, $this->makeOperation()); + } + + public function testThrowsBadRequestIfSiteAwareAndNoCurrentSite(): void + { + $data = $this->makeSiteAwareStub(null); + + $inner = $this->createMock(ProcessorInterface::class); + $inner->expects(self::never()) + ->method('process') + ; + + $processor = $this->makeProcessor($inner, currentSite: null); + + $this->expectException(BadRequestHttpException::class); + $this->expectExceptionMessage('aucun site selectionne'); + + $processor->process($data, $this->makeOperation()); + } + + public function testDelegatesToInnerProcessorAlwaysWhenNoThrow(): void + { + $data = new stdClass(); + $inner = $this->createMock(ProcessorInterface::class); + $inner->expects(self::once()) + ->method('process') + ->willReturn('delegated-result') + ; + + $processor = $this->makeProcessor( + $inner, + new Site('Any', 'Rue', null, '12345', 'Ville', '#000000'), + ); + $result = $processor->process($data, $this->makeOperation()); + + self::assertSame('delegated-result', $result); + } + + private function makeProcessor( + ProcessorInterface $inner, + ?Site $currentSite, + ): SiteAwareInjectionProcessor { + // createStub : on n'a besoin que de fixer la valeur de retour, pas + // d'attentes sur le nombre d'appels. Evite la notice PHPUnit + // "No expectations were configured for the mock object". + $provider = $this->createStub(CurrentSiteProviderInterface::class); + $provider->method('get')->willReturn($currentSite); + + return new SiteAwareInjectionProcessor($inner, $provider); + } + + private function makeSiteAwareStub(?Site $initialSite): SiteAwareInterface + { + return new class($initialSite) implements SiteAwareInterface { + public function __construct(private ?Site $site) {} + + public function getSite(): ?Site + { + return $this->site; + } + + public function setSite(Site $site): void + { + $this->site = $site; + } + }; + } + + private function makeOperation(): Operation + { + return new Post(); + } +} diff --git a/tests/Module/Sites/SitesModuleTest.php b/tests/Module/Sites/SitesModuleTest.php new file mode 100644 index 0000000..be9e330 --- /dev/null +++ b/tests/Module/Sites/SitesModuleTest.php @@ -0,0 +1,53 @@ +get('api_platform.doctrine.orm.state.persist_processor'); + + self::assertInstanceOf( + SiteAwareInjectionProcessor::class, + $persistProcessor, + 'Le service api_platform.doctrine.orm.state.persist_processor doit etre decore par SiteAwareInjectionProcessor (#[AsDecorator]).', + ); + } +}