From 1cf550721b75ff0ffc0ea85b1b45cbc02dce6a56 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 10:31:10 +0200 Subject: [PATCH 01/11] docs(rbac) : spec ticket #344 - API CRUD roles & permissions --- docs/rbac/ticket-344-spec.md | 275 +++++++++++++++++++++++++++++++++++ 1 file changed, 275 insertions(+) create mode 100644 docs/rbac/ticket-344-spec.md diff --git a/docs/rbac/ticket-344-spec.md b/docs/rbac/ticket-344-spec.md new file mode 100644 index 0000000..80db2e6 --- /dev/null +++ b/docs/rbac/ticket-344-spec.md @@ -0,0 +1,275 @@ +# Ticket #344 - 2/5 - API CRUD Roles & Permissions (Backend) + +## 1. Objectif + +Exposer via API Platform le socle RBAC livre par le ticket #343 (entites `Role`, `Permission`, relations `User->roles`/`directPermissions`, flag `isAdmin`). Ce ticket livre la surface HTTP minimale permettant : + +- de lister et consulter les permissions synchronisees par `app:sync-permissions`, +- de gerer le cycle de vie des roles (CRUD) tout en protegeant les roles systeme, +- d'attribuer `isAdmin`, les roles RBAC et les permissions directes a un utilisateur sans polluer le groupe `user:write` (commit `0fc4e16`). + +Le ticket n'introduit **aucune logique d'autorisation metier** : toute la verification `is_granted('module.resource.action')` est traitee par le voter du ticket #345. A ce stade, les operations sont gardees par un simple `is_granted('ROLE_ADMIN')`, remplace au #345. + +## 2. Perimetre + +### IN + +- Exposer l'entite `Permission` en API Platform en lecture seule (`GetCollection`, `Get`), groupe `permission:read`, filtres `module` et `orphan`. +- Exposer l'entite `Role` en API Platform avec CRUD complet (`GetCollection`, `Get`, `Post`, `Patch`, `Delete`), groupes `role:read` et `role:write`, filtre `isSystem`. +- Ajouter un processor `RoleProcessor` decorant `PersistProcessor` et `RemoveProcessor` pour : + - refuser la suppression d'un role systeme en traduisant `SystemRoleDeletionException` en `403`, + - empecher la mutation de `code` et `isSystem` sur un role systeme existant. +- Ajouter une operation nommee `user_rbac_patch` (`PATCH /api/users/{id}/rbac`) sur l'entite `User` avec son propre groupe `user:rbac:write` exposant `isAdmin`, `roles` et `directPermissions`. Laisser `user:write` propre pour les champs profil (compatible avec la decision de `0fc4e16`). Le nom explicite est indispensable : API Platform 4 identifie les operations par nom, un `new Patch` sans `name:` entrerait en collision avec l'operation profil existante. +- Ajouter un processor `UserRbacProcessor` qui persiste les mutations RBAC de l'utilisateur sans toucher au password hashing (decorator de `PersistProcessor`, pas du `UserPasswordHasherProcessor`). +- Ajouter sur `Role` les contraintes Symfony Validator : `UniqueEntity(fields: ['code'])`, `Assert\NotBlank` et `Assert\Regex` sur `code`, `Assert\NotBlank` sur `label` (cf. section 6). +- Garder toutes les operations sous `is_granted('ROLE_ADMIN')` avec un commentaire `// TODO ticket #345 : remplacer par is_granted('core.roles.manage')`. +- Tests PHPUnit unitaires (processors) et fonctionnels (`ApiTestCase`) couvrant les chemins nominaux et les cas 403/422. + +### OUT + +- Ticket `#345` : voter `PermissionVoter`, remplacement du `is_granted('ROLE_ADMIN')` par les codes de permission, composable front `usePermissions`. +- Ticket `#346` : ecrans d'administration front (liste/edition des roles et permissions). +- Ticket `#347` : UX des erreurs 403 et integration front de l'ecran de gestion des permissions utilisateur. +- Endpoint d'ecriture sur `Permission` : la table reste la propriete exclusive de `app:sync-permissions` (source de verite = code). +- Lecture des permissions effectives d'un `User` via `/api/me` : traitee au #345 en meme temps que le voter. +- Exposition d'un endpoint de bulk-assign permissions sur plusieurs utilisateurs : hors scope. + +## 3. Fichiers a creer + +### Infrastructure - Processors + +- `/home/matthieu/dev_malio/Coltura/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php` + Decorator de `ApiPlatform\Doctrine\Common\State\PersistProcessor` et `RemoveProcessor`. Charge de la garde `ensureDeletable()` et de la protection des champs immuables sur un role systeme. + +- `/home/matthieu/dev_malio/Coltura/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php` + Decorator de `PersistProcessor` specifique a l'operation `PATCH /api/users/{id}/rbac`. Persiste les mutations `isAdmin`, `roles`, `directPermissions` sans passer par `UserPasswordHasherProcessor`. + +### Tests unitaires + +- `/home/matthieu/dev_malio/Coltura/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php` +- `/home/matthieu/dev_malio/Coltura/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php` + +### Tests fonctionnels + +- `/home/matthieu/dev_malio/Coltura/tests/Module/Core/Api/PermissionApiTest.php` +- `/home/matthieu/dev_malio/Coltura/tests/Module/Core/Api/RoleApiTest.php` +- `/home/matthieu/dev_malio/Coltura/tests/Module/Core/Api/UserRbacApiTest.php` + +## 4. Fichiers a modifier + +### Entite `Permission` + +`/home/matthieu/dev_malio/Coltura/src/Module/Core/Domain/Entity/Permission.php` + +- Ajouter l'attribut `#[ApiResource]` avec operations `GetCollection` + `Get` uniquement. +- Normalization context : groupe `permission:read` uniquement. +- Pas de `denormalizationContext` (lecture seule). +- Security `is_granted('ROLE_ADMIN')` sur les deux operations (TODO #345). +- Ajouter `#[Groups(['permission:read'])]` sur `$id`, `$code`, `$label`, `$module`, `$orphan`. Pas d'ajout du groupe `role:read` : on laisse API Platform serialiser la relation `Role::$permissions` en IRIs par defaut, le front resoudra les details en 2 appels si necessaire (decision explicite pour garder les payloads petits et les permissions paginable independamment). +- Ajouter les filtres API Platform `SearchFilter` sur `module` (exact) et `BooleanFilter` sur `orphan`. + +Extrait attendu : + +```php +#[ApiResource( + operations: [ + new GetCollection( + security: "is_granted('ROLE_ADMIN')", + normalizationContext: ['groups' => ['permission:read']], + ), + new Get( + security: "is_granted('ROLE_ADMIN')", + normalizationContext: ['groups' => ['permission:read']], + ), + ], +)] +#[ApiFilter(SearchFilter::class, properties: ['module' => 'exact'])] +#[ApiFilter(BooleanFilter::class, properties: ['orphan'])] +``` + +### Entite `Role` + +`/home/matthieu/dev_malio/Coltura/src/Module/Core/Domain/Entity/Role.php` + +- Ajouter l'attribut `#[ApiResource]` avec operations `GetCollection`, `Get`, `Post`, `Patch`, `Delete`. +- Normalization context : `role:read`. Denormalization context : `role:write`. +- Processor `RoleProcessor::class` sur `Post`, `Patch` et `Delete`. +- Security `is_granted('ROLE_ADMIN')` sur les 5 operations (TODO #345). +- Groupes : + - `$id` : `role:read`. + - `$code` : `role:read`, `role:write`. L'immuabilite apres creation est portee par `RoleProcessor` (variante A, cf. section 5), pas par un decoupage de groupes. + - `$label` : `role:read`, `role:write`. + - `$description` : `role:read`, `role:write`. + - `$isSystem` : `role:read` (jamais writable via API). + - `$permissions` : `role:read`, `role:write`. Serialise en IRIs (comportement API Platform par defaut sur une relation ManyToMany). +- Filtre `BooleanFilter` sur `isSystem`. +- **Important** : le constructeur actuel `public function __construct(string $code, string $label, bool $isSystem = false, ?string $description = null)` doit etre compatible avec la denormalisation API Platform sur `POST`. API Platform 4 resout les arguments du constructeur par nom de propriete denormalise. Verifier (ou adapter) que `isSystem` ne peut pas etre injecte par le POST car il n'est pas dans `role:write`. + +### Entite `User` + +`/home/matthieu/dev_malio/Coltura/src/Module/Core/Domain/Entity/User.php` + +- Ajouter dans la liste des operations `ApiResource` existantes une operation dediee : + +```php +new Patch( + name: 'user_rbac_patch', + uriTemplate: '/users/{id}/rbac', + security: "is_granted('ROLE_ADMIN')", + denormalizationContext: ['groups' => ['user:rbac:write']], + processor: UserRbacProcessor::class, +), +``` + +Le `name:` est OBLIGATOIRE : sans lui, API Platform 4 deduit un nom par defaut qui peut collisionner avec la `Patch` profil existante (meme classe, meme methode HTTP) et provoquer un ecrasement silencieux de la route `/api/users/{id}`. + +- Ajouter le groupe `user:rbac:write` sur les proprietes : + - `$isAdmin` + - `$roles` + - `$directPermissions` +- Ne PAS toucher `user:write` : la decision de `0fc4e16` est confirmee par ce ticket. + +Raison de l'endpoint dedie (option B) : +- Separation des preoccupations : un `PATCH /api/users/{id}` reste un endpoint "profil" ; la promotion admin et la gestion des permissions est un acte administratif explicite et tracable. +- Facilite future l'ajout d'un audit log dedie (`#355` audit log project) sur l'endpoint RBAC sans polluer l'audit profil. +- Contrat front simple : une seule route, un seul groupe, une seule validation. + +## 5. Regles metier et cas limites + +### Role + +- **Creation (`POST /api/roles`)** : + - `code`, `label` obligatoires. `description` optionnel. `permissions` optionnel (tableau d'IRIs). + - `isSystem` est toujours `false` pour les roles crees via API (n'est pas dans `role:write`). + - Unicite du `code` geree par la contrainte DB `uniq_role_code` → 422 via `UniqueEntity` validator a ajouter sur l'entite (voir section 6). + +- **Modification (`PATCH /api/roles/{id}`)** : + - `label`, `description`, `permissions` modifiables librement, y compris sur un role systeme (utile pour customiser l'apparence dans l'UI sans casser la relation). + - `code` **immuable apres creation** — strategie retenue (variante A) : un seul groupe `role:write` contenant `code`, et une garde centralisee dans `RoleProcessor`. Le processor compare la valeur entrante a l'etat d'origine via `UnitOfWork::getOriginalEntityData($role)['code']` ; si elle differe, leve `BadRequestHttpException` avec un message francais explicite. Regle unique et uniforme : roles systeme ET roles customs sont concernes. Justification : garder la regle metier dans le domaine applicatif plutot que dupliquer les groupes de serialisation. + +- **Suppression (`DELETE /api/roles/{id}`)** : + - `RoleProcessor` appelle `$role->ensureDeletable()` avant de deleguer au `RemoveProcessor`. + - `SystemRoleDeletionException` est catchee et re-levee en `Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException` (403). + - Les relations `user_role` et `role_permission` sur ce role sont nettoyees automatiquement par le `ON DELETE CASCADE` des contraintes `FK_2DE8C6A3D60322AC` (`user_role.role_id`) et `FK_6F7DF886D60322AC` (`role_permission.role_id`) posees dans `migrations/Version20260414150034.php`. Aucun nettoyage manuel necessaire dans `RoleProcessor`. Verifier en test fonctionnel par un DELETE d'un role custom attache a un user, puis assert que le user existe toujours et que `user_role` est vide pour ce couple. + +### Permission + +- Lecture seule via API. Aucun endpoint de mutation. +- Si un admin veut forcer une permission sur un utilisateur, il passe par `directPermissions` de `User`. + +### User (operation RBAC) + +- `PATCH /api/users/{id}/rbac` n'accepte que `isAdmin`, `roles`, `directPermissions`. Tout autre champ dans le payload est ignore (comportement par defaut d'API Platform avec un `denormalizationContext` restreint). +- **Garde minimale auto-suicide** : `UserRbacProcessor` refuse (`BadRequestHttpException` 400) toute requete ou l'user cible est egal a l'user courant du `Security::getUser()` ET `isAdmin` passe de `true` a `false`. Sans cette garde, un admin peut se degrader seul et perdre acces a l'endpoint, creant une situation de recovery penible. C'est une garde locale et pragmatique, volontairement plus stricte que "le dernier admin" : on interdit l'auto-degradation, point. La garde "plus d'un admin restant" reste reportee au #345 ou un inventaire global fera sens avec le voter. TODO a placer dans le processor avec reference a #345. +- Le password n'est jamais touche par cet endpoint (contrairement a `UserPasswordHasherProcessor` sur `PATCH /api/users/{id}`). + +## 6. Validation + +- Ajouter sur `Role` une contrainte `#[UniqueEntity(fields: ['code'])]` pour un 422 propre au lieu d'un 500 SQL en cas de conflit. +- Ajouter sur `Role::$code` un `#[Assert\NotBlank]` et un `#[Assert\Regex('/^[a-z][a-z0-9_]*$/')]` (meme convention que les permissions). +- Ajouter sur `Role::$label` un `#[Assert\NotBlank]`. + +## 7. Plan de tests + +### Unitaires + +**`RoleProcessorTest`** + +- `process()` d'un role non-systeme en DELETE delegue au `RemoveProcessor` sans lever. +- `process()` d'un role systeme en DELETE leve `AccessDeniedHttpException` (403) et n'appelle pas le decorator. +- `process()` d'un role systeme en PATCH dont le `code` a change leve `BadRequestHttpException`. +- `process()` d'un role systeme en PATCH dont seuls `label`/`permissions` changent delegue au `PersistProcessor`. +- `process()` d'un role non-systeme en POST delegue au `PersistProcessor`. + +**`UserRbacProcessorTest`** + +- `process()` persiste un user avec `isAdmin = true` via le decorator. +- `process()` persiste une collection de `roles` mise a jour. +- `process()` ne declenche jamais le hashing de password (verifier que `UserPasswordHasherProcessor` n'est pas dans la chaine). + +### Fonctionnels (`ApiTestCase`) + +**`PermissionApiTest`** + +- `GET /api/permissions` en tant qu'admin retourne la liste des permissions synchronisees. +- `GET /api/permissions?module=core` filtre par module. +- `GET /api/permissions?orphan=true` retourne uniquement les orphelines. +- `GET /api/permissions/{id}` retourne les champs attendus (groupe `permission:read`). +- `POST /api/permissions` en tant qu'admin retourne `405 Method Not Allowed`. +- `GET /api/permissions` non authentifie retourne `401`. +- `GET /api/permissions` en tant que user standard retourne `403`. + +**`RoleApiTest`** + +- `POST /api/roles` avec `{code, label, description}` retourne `201` et persiste `isSystem = false`. +- `POST /api/roles` avec un `code` deja utilise retourne `422`. +- `POST /api/roles` avec un `code` invalide (`MAJ`, `space`) retourne `422`. +- `PATCH /api/roles/{id}` sur un role custom modifie `label` et ajoute des permissions via IRIs → `200`. +- `PATCH /api/roles/{id}` sur le role `admin` (systeme) modifiant seulement `label` → `200`. +- `PATCH /api/roles/{id}` sur le role `admin` tentant de modifier `code` → `400`. +- `DELETE /api/roles/{id}` sur un role custom → `204`. +- `DELETE /api/roles/{id}` sur le role `admin` → `403` avec `SystemRoleDeletionException` traduite. +- `DELETE /api/roles/{id}` d'un role custom attache a un user : le user reste, la relation `user_role` est nettoyee par le CASCADE. +- Toute operation sans auth retourne `401`. +- Toute operation en tant que user standard retourne `403`. + +**`UserRbacApiTest`** + +- `PATCH /api/users/{id}/rbac` en tant qu'admin avec `{isAdmin: true}` promeut le user. +- `PATCH /api/users/{id}/rbac` avec `{roles: [IRI...]}` remplace la collection de roles RBAC. +- `PATCH /api/users/{id}/rbac` avec `{directPermissions: [IRI...]}` remplace les permissions directes. +- `PATCH /api/users/{id}/rbac` en tant que user standard retourne `403`. +- `PATCH /api/users/{id}/rbac` non authentifie retourne `401`. +- `PATCH /api/users/{id}/rbac` avec un champ `username` dans le payload n'est pas persiste (denormalization context restreint). +- `PATCH /api/users/{id}` sans `/rbac` avec `{isAdmin: true}` ne modifie PAS `isAdmin` (confirme la decision `0fc4e16`). + +## 8. Securite et traduction d'exceptions + +- `SystemRoleDeletionException` → `AccessDeniedHttpException` (403) dans `RoleProcessor` (pas via un listener global : on garde la traduction locale au perimetre RBAC). +- `BadRequestHttpException` pour la mutation de `code` sur un role systeme : message explicite en francais, dans le payload Hydra `hydra:description`. +- Toutes les routes ont pour l'instant `security: "is_granted('ROLE_ADMIN')"`. Un commentaire `// TODO ticket #345` doit etre present sur chaque attribut pour faciliter le remplacement. + +## 9. Conventions et architecture + +- Respect strict du modular monolith : tous les fichiers crees vivent dans `src/Module/Core/` ou `tests/Module/Core/`. Aucun import depuis un autre module. +- `declare(strict_types=1)` en tete des nouveaux fichiers. +- Commentaires PHP en francais, identifiants anglais (`CLAUDE.md`). +- Processors branches via l'autoconfiguration Symfony ; aucun wiring manuel dans `services.yaml` attendu si le constructeur est injecte proprement. +- Pattern de decorator : utiliser `#[AsDecorator]` ou `#[Autoconfigure]` pour brancher le processor en tant que decorator du `PersistProcessor` API Platform, selon le pattern deja utilise par `UserPasswordHasherProcessor`. +- Aucune nouvelle entree necessaire dans `config/modules.php` ni `config/sidebar.php`. + +## 10. Ordre d'execution recommande + +1. Ajouter l'attribut `#[ApiResource]` et les `#[Groups]` sur `Permission`. Ecrire `PermissionApiTest`. +2. Ajouter les contraintes Validator sur `Role`. Ajouter `#[ApiResource]` et les `#[Groups]` sur `Role` **sans** processor dans un premier temps pour valider le CRUD nominal. +3. Creer `RoleProcessor` et le brancher en decorator. Ajouter les gardes systeme. Ecrire `RoleProcessorTest` + cas `RoleApiTest`. +4. Creer `UserRbacProcessor`. Ajouter l'operation `/users/{id}/rbac` et le groupe `user:rbac:write` sur `User`. Ecrire `UserRbacProcessorTest` + `UserRbacApiTest`. +5. `make test` complet + `make php-cs-fixer-allow-risky`. +6. Documentation : referencer ce spec dans `docs/rbac/` et mettre a jour le fil conducteur RBAC si un index existe. + +## 11. Risques et points d'attention + +- **Constructeur de `Role` et denormalisation POST** : API Platform 4 resout les arguments du constructeur par nom ; `isSystem` est dans la signature mais pas dans `role:write`, donc un client ne peut pas l'injecter — a verifier par un test explicite ("POST avec `isSystem: true` est ignore"). +- **`code` immuable** : strategie retenue (garde dans processor) simple mais demande une lecture de l'etat initial du role avant persistance. Utiliser `UnitOfWork::getOriginalEntityData()` pour recuperer la valeur d'origine proprement. +- **Cascade de delete role → user_role** : depend de `ON DELETE CASCADE` pose par la migration #343. Verifier explicitement en test fonctionnel qu'aucune `ForeignKeyConstraintViolationException` ne remonte. +- **`UniqueEntity` sur `code`** : ne couvre pas les conflits en race condition, la DB reste la garde ultime. Acceptable. +- **Pas de filtre sur le `module` de Permission cote front** au #346 sans le filtre API : s'assurer que le filtre est bien pose ici. +- **Auto-retrait du dernier admin** : garde d'**auto-suicide** posee dans `UserRbacProcessor` (un admin ne peut pas se degrader lui-meme, cf. section 5). La garde "plus d'un admin restant" au niveau global reste reportee au voter #345. +- **Infra de test fonctionnel (fixtures et isolation)** : les tests `*ApiTest` dependent de la presence en base des roles systeme `admin` et `user`. L'infra actuelle doit fournir soit un reload des fixtures par classe de test, soit `DAMADoctrineTestBundle` pour transactionner chaque test. A verifier au debut de l'etape 1 de l'ordre d'execution ; si absent, ajouter un trait de bootstrap minimal `RbacFixturesTrait` qui insere les deux roles systeme avant chaque classe de test (pas par test, trop couteux). Ne pas bloquer le ticket sur cette question, adapter au vol. + +## 12. Criteres d'acceptation (DoD) + +- `GET /api/permissions` et `GET /api/permissions/{id}` fonctionnent, filtres `module` et `orphan` operationnels. +- CRUD complet sur `/api/roles` operationnel, avec `isSystem` en lecture seule cote API. +- `DELETE /api/roles/{admin_id}` retourne `403` avec un message metier. +- `PATCH /api/roles/{admin_id}` autorise la modification de `label`/`permissions` mais refuse la modification de `code` avec `400`. +- `PATCH /api/users/{id}/rbac` permet de modifier `isAdmin`, `roles` et `directPermissions` ; `PATCH /api/users/{id}` (profil) ne les modifie jamais. +- Les operations API sont gardees par `is_granted('ROLE_ADMIN')` et commentees avec la TODO pointant vers #345. +- `make test` passe ; `make php-cs-fixer-allow-risky` ne laisse aucun delta. +- Aucun import croise entre modules ; tous les fichiers crees vivent dans `Module/Core/` ou `tests/Module/Core/`. +- Le spec est mergee avec le code (meme PR ou PR precedente) pour rester la reference du ticket. + +## 13. Remarques de branche + +- Le ticket enonce "Branche a creer : `feat/rbac-api` depuis develop apres merge de #2". +- Branche courante locale : `feat/rbac-core`. A confirmer avec l'utilisateur si PR #2 est mergee : si oui, se rebaser sur `develop` et creer `feat/rbac-api` propre ; sinon, empiler `feat/rbac-api` sur `feat/rbac-core` en attendant le merge. -- 2.39.5 From fdb7aded82b53f6696cea5b92214b5b5342ffe7a Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 11:03:22 +0200 Subject: [PATCH 02/11] feat(core) : RBAC #344 - API Platform Permission en lecture seule - Expose l'entite Permission via ApiResource (GetCollection + Get uniquement) - Serialisation limitee au groupe permission:read (id, code, label, module, orphan) - Securite temporaire is_granted('ROLE_ADMIN'), a remplacer par is_granted('core.permissions.view') au ticket #345 - Filtres : SearchFilter exact sur module, BooleanFilter sur orphan - Configure api_platform.mapping.paths pour que le compile pass AP decouvre les ApiResource/ApiFilter declares dans src/Module/Core/Domain/Entity - Ajoute symfony/browser-kit et symfony/http-client en dev pour les tests fonctionnels API Platform, plus KERNEL_CLASS dans phpunit.dist.xml - Tests fonctionnels PermissionApiTest : collection, get item, filtres module et orphan, 405 sur POST, 401 non authentifie, 403 non-admin --- composer.json | 5 +- composer.lock | 492 ++++++++++++------- config/packages/api_platform.yaml | 7 + phpunit.dist.xml | 1 + src/Module/Core/Domain/Entity/Permission.php | 28 ++ tests/Module/Core/Api/PermissionApiTest.php | 185 +++++++ 6 files changed, 541 insertions(+), 177 deletions(-) create mode 100644 tests/Module/Core/Api/PermissionApiTest.php diff --git a/composer.json b/composer.json index de7bace..a3a9b4f 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,6 @@ "symfony/expression-language": "8.0.*", "symfony/flex": "^2", "symfony/framework-bundle": "8.0.*", - "symfony/http-client": "8.0.*", "symfony/mime": "8.0.*", "symfony/monolog-bundle": "^4.0", "symfony/property-access": "8.0.*", @@ -90,6 +89,8 @@ "require-dev": { "doctrine/doctrine-fixtures-bundle": "^4.3", "friendsofphp/php-cs-fixer": "^3.94", - "phpunit/phpunit": "^13.0" + "phpunit/phpunit": "^13.0", + "symfony/browser-kit": "8.0.*", + "symfony/http-client": "8.0.*" } } diff --git a/composer.lock b/composer.lock index 7eded7a..43d0aeb 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "bfd26e903d79f710cfe95452c05f2a25", + "content-hash": "75f8e672f2a401290886fbcf01befd3f", "packages": [ { "name": "api-platform/doctrine-common", @@ -4988,180 +4988,6 @@ ], "time": "2026-03-30T15:14:47+00:00" }, - { - "name": "symfony/http-client", - "version": "v8.0.8", - "source": { - "type": "git", - "url": "https://github.com/symfony/http-client.git", - "reference": "356e43d6994ae9d7761fd404d40f78691deabe0e" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/http-client/zipball/356e43d6994ae9d7761fd404d40f78691deabe0e", - "reference": "356e43d6994ae9d7761fd404d40f78691deabe0e", - "shasum": "" - }, - "require": { - "php": ">=8.4", - "psr/log": "^1|^2|^3", - "symfony/http-client-contracts": "~3.4.4|^3.5.2", - "symfony/service-contracts": "^2.5|^3" - }, - "conflict": { - "amphp/amp": "<3", - "php-http/discovery": "<1.15" - }, - "provide": { - "php-http/async-client-implementation": "*", - "php-http/client-implementation": "*", - "psr/http-client-implementation": "1.0", - "symfony/http-client-implementation": "3.0" - }, - "require-dev": { - "amphp/http-client": "^5.3.2", - "amphp/http-tunnel": "^2.0", - "guzzlehttp/promises": "^1.4|^2.0", - "nyholm/psr7": "^1.0", - "php-http/httplug": "^1.0|^2.0", - "psr/http-client": "^1.0", - "symfony/cache": "^7.4|^8.0", - "symfony/dependency-injection": "^7.4|^8.0", - "symfony/http-kernel": "^7.4|^8.0", - "symfony/messenger": "^7.4|^8.0", - "symfony/process": "^7.4|^8.0", - "symfony/rate-limiter": "^7.4|^8.0", - "symfony/stopwatch": "^7.4|^8.0" - }, - "type": "library", - "autoload": { - "psr-4": { - "Symfony\\Component\\HttpClient\\": "" - }, - "exclude-from-classmap": [ - "/Tests/" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Provides powerful methods to fetch HTTP resources synchronously or asynchronously", - "homepage": "https://symfony.com", - "keywords": [ - "http" - ], - "support": { - "source": "https://github.com/symfony/http-client/tree/v8.0.8" - }, - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://github.com/nicolas-grekas", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2026-03-30T15:14:47+00:00" - }, - { - "name": "symfony/http-client-contracts", - "version": "v3.6.0", - "source": { - "type": "git", - "url": "https://github.com/symfony/http-client-contracts.git", - "reference": "75d7043853a42837e68111812f4d964b01e5101c" - }, - "dist": { - "type": "zip", - "url": "https://api.github.com/repos/symfony/http-client-contracts/zipball/75d7043853a42837e68111812f4d964b01e5101c", - "reference": "75d7043853a42837e68111812f4d964b01e5101c", - "shasum": "" - }, - "require": { - "php": ">=8.1" - }, - "type": "library", - "extra": { - "thanks": { - "url": "https://github.com/symfony/contracts", - "name": "symfony/contracts" - }, - "branch-alias": { - "dev-main": "3.6-dev" - } - }, - "autoload": { - "psr-4": { - "Symfony\\Contracts\\HttpClient\\": "" - }, - "exclude-from-classmap": [ - "/Test/" - ] - }, - "notification-url": "https://packagist.org/downloads/", - "license": [ - "MIT" - ], - "authors": [ - { - "name": "Nicolas Grekas", - "email": "p@tchwork.com" - }, - { - "name": "Symfony Community", - "homepage": "https://symfony.com/contributors" - } - ], - "description": "Generic abstractions related to HTTP clients", - "homepage": "https://symfony.com", - "keywords": [ - "abstractions", - "contracts", - "decoupling", - "interfaces", - "interoperability", - "standards" - ], - "support": { - "source": "https://github.com/symfony/http-client-contracts/tree/v3.6.0" - }, - "funding": [ - { - "url": "https://symfony.com/sponsor", - "type": "custom" - }, - { - "url": "https://github.com/fabpot", - "type": "github" - }, - { - "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", - "type": "tidelift" - } - ], - "time": "2025-04-29T11:18:49+00:00" - }, { "name": "symfony/http-foundation", "version": "v8.0.8", @@ -11018,6 +10844,322 @@ ], "time": "2024-10-20T05:08:20+00:00" }, + { + "name": "symfony/browser-kit", + "version": "v8.0.8", + "source": { + "type": "git", + "url": "https://github.com/symfony/browser-kit.git", + "reference": "f5a28fca785416cf489dd579011e74c831100cc3" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/browser-kit/zipball/f5a28fca785416cf489dd579011e74c831100cc3", + "reference": "f5a28fca785416cf489dd579011e74c831100cc3", + "shasum": "" + }, + "require": { + "php": ">=8.4", + "symfony/dom-crawler": "^7.4|^8.0" + }, + "require-dev": { + "symfony/css-selector": "^7.4|^8.0", + "symfony/http-client": "^7.4|^8.0", + "symfony/mime": "^7.4|^8.0", + "symfony/process": "^7.4|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\BrowserKit\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Simulates the behavior of a web browser, allowing you to make requests, click on links and submit forms programmatically", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/browser-kit/tree/v8.0.8" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-03-30T15:14:47+00:00" + }, + { + "name": "symfony/dom-crawler", + "version": "v8.0.8", + "source": { + "type": "git", + "url": "https://github.com/symfony/dom-crawler.git", + "reference": "284ace90732b445b027728b5e0eec6418a17a364" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/dom-crawler/zipball/284ace90732b445b027728b5e0eec6418a17a364", + "reference": "284ace90732b445b027728b5e0eec6418a17a364", + "shasum": "" + }, + "require": { + "php": ">=8.4", + "symfony/polyfill-ctype": "^1.8", + "symfony/polyfill-mbstring": "^1.0" + }, + "require-dev": { + "symfony/css-selector": "^7.4|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\DomCrawler\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Eases DOM navigation for HTML and XML documents", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/dom-crawler/tree/v8.0.8" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-03-30T15:14:47+00:00" + }, + { + "name": "symfony/http-client", + "version": "v8.0.8", + "source": { + "type": "git", + "url": "https://github.com/symfony/http-client.git", + "reference": "356e43d6994ae9d7761fd404d40f78691deabe0e" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/http-client/zipball/356e43d6994ae9d7761fd404d40f78691deabe0e", + "reference": "356e43d6994ae9d7761fd404d40f78691deabe0e", + "shasum": "" + }, + "require": { + "php": ">=8.4", + "psr/log": "^1|^2|^3", + "symfony/http-client-contracts": "~3.4.4|^3.5.2", + "symfony/service-contracts": "^2.5|^3" + }, + "conflict": { + "amphp/amp": "<3", + "php-http/discovery": "<1.15" + }, + "provide": { + "php-http/async-client-implementation": "*", + "php-http/client-implementation": "*", + "psr/http-client-implementation": "1.0", + "symfony/http-client-implementation": "3.0" + }, + "require-dev": { + "amphp/http-client": "^5.3.2", + "amphp/http-tunnel": "^2.0", + "guzzlehttp/promises": "^1.4|^2.0", + "nyholm/psr7": "^1.0", + "php-http/httplug": "^1.0|^2.0", + "psr/http-client": "^1.0", + "symfony/cache": "^7.4|^8.0", + "symfony/dependency-injection": "^7.4|^8.0", + "symfony/http-kernel": "^7.4|^8.0", + "symfony/messenger": "^7.4|^8.0", + "symfony/process": "^7.4|^8.0", + "symfony/rate-limiter": "^7.4|^8.0", + "symfony/stopwatch": "^7.4|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\HttpClient\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides powerful methods to fetch HTTP resources synchronously or asynchronously", + "homepage": "https://symfony.com", + "keywords": [ + "http" + ], + "support": { + "source": "https://github.com/symfony/http-client/tree/v8.0.8" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-03-30T15:14:47+00:00" + }, + { + "name": "symfony/http-client-contracts", + "version": "v3.6.0", + "source": { + "type": "git", + "url": "https://github.com/symfony/http-client-contracts.git", + "reference": "75d7043853a42837e68111812f4d964b01e5101c" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/http-client-contracts/zipball/75d7043853a42837e68111812f4d964b01e5101c", + "reference": "75d7043853a42837e68111812f4d964b01e5101c", + "shasum": "" + }, + "require": { + "php": ">=8.1" + }, + "type": "library", + "extra": { + "thanks": { + "url": "https://github.com/symfony/contracts", + "name": "symfony/contracts" + }, + "branch-alias": { + "dev-main": "3.6-dev" + } + }, + "autoload": { + "psr-4": { + "Symfony\\Contracts\\HttpClient\\": "" + }, + "exclude-from-classmap": [ + "/Test/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nicolas Grekas", + "email": "p@tchwork.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Generic abstractions related to HTTP clients", + "homepage": "https://symfony.com", + "keywords": [ + "abstractions", + "contracts", + "decoupling", + "interfaces", + "interoperability", + "standards" + ], + "support": { + "source": "https://github.com/symfony/http-client-contracts/tree/v3.6.0" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2025-04-29T11:18:49+00:00" + }, { "name": "symfony/process", "version": "v8.0.8", diff --git a/config/packages/api_platform.yaml b/config/packages/api_platform.yaml index 4f32b21..7558fb2 100644 --- a/config/packages/api_platform.yaml +++ b/config/packages/api_platform.yaml @@ -1,6 +1,13 @@ api_platform: title: Coltura API version: 1.0.0 + # Scan des modules pour decouvrir les classes ApiResource et ApiFilter. + # Sans ces paths, le compile pass d'API Platform ne declare pas les + # services de filtres annotes (les filtres etaient silencieusement + # ignores sur Permission — cf. ticket #344). + mapping: + paths: + - '%kernel.project_dir%/src/Module/Core/Domain/Entity' formats: jsonld: ['application/ld+json'] json: ['application/json'] diff --git a/phpunit.dist.xml b/phpunit.dist.xml index 22bd879..eb794bd 100644 --- a/phpunit.dist.xml +++ b/phpunit.dist.xml @@ -15,6 +15,7 @@ + diff --git a/src/Module/Core/Domain/Entity/Permission.php b/src/Module/Core/Domain/Entity/Permission.php index e5e92a0..83a3b06 100644 --- a/src/Module/Core/Domain/Entity/Permission.php +++ b/src/Module/Core/Domain/Entity/Permission.php @@ -4,10 +4,33 @@ declare(strict_types=1); namespace App\Module\Core\Domain\Entity; +use ApiPlatform\Doctrine\Orm\Filter\BooleanFilter; +use ApiPlatform\Doctrine\Orm\Filter\SearchFilter; +use ApiPlatform\Metadata\ApiFilter; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\GetCollection; use App\Module\Core\Infrastructure\Doctrine\DoctrinePermissionRepository; use Doctrine\ORM\Mapping as ORM; use InvalidArgumentException; +use Symfony\Component\Serializer\Attribute\Groups; +#[ApiResource( + operations: [ + new GetCollection( + normalizationContext: ['groups' => ['permission:read']], + // TODO ticket #345 : remplacer par is_granted('core.permissions.view') + security: "is_granted('ROLE_ADMIN')", + ), + new Get( + normalizationContext: ['groups' => ['permission:read']], + // TODO ticket #345 : remplacer par is_granted('core.permissions.view') + security: "is_granted('ROLE_ADMIN')", + ), + ], +)] +#[ApiFilter(SearchFilter::class, properties: ['module' => 'exact'])] +#[ApiFilter(BooleanFilter::class, properties: ['orphan'])] #[ORM\Entity(repositoryClass: DoctrinePermissionRepository::class)] #[ORM\Table(name: 'permission')] #[ORM\UniqueConstraint(name: 'uniq_permission_code', columns: ['code'])] @@ -18,18 +41,23 @@ class Permission #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] + #[Groups(['permission:read'])] private ?int $id = null; #[ORM\Column(length: 255)] + #[Groups(['permission:read'])] private string $code; #[ORM\Column(length: 255)] + #[Groups(['permission:read'])] private string $label; #[ORM\Column(length: 100)] + #[Groups(['permission:read'])] private string $module; #[ORM\Column(options: ['default' => false])] + #[Groups(['permission:read'])] private bool $orphan = false; /** diff --git a/tests/Module/Core/Api/PermissionApiTest.php b/tests/Module/Core/Api/PermissionApiTest.php new file mode 100644 index 0000000..0e7fbe2 --- /dev/null +++ b/tests/Module/Core/Api/PermissionApiTest.php @@ -0,0 +1,185 @@ +em = self::getContainer()->get('doctrine')->getManager(); + + // Nettoyage defensif au cas ou un run precedent aurait laisse des restes. + $this->cleanupTestPermissions(); + + // Donnees de test : deux permissions "core" dont une orpheline, + // plus une permission d'un autre module pour verifier le filtre. + $p1 = new Permission('test.core.users.view', 'View users (test)', 'core'); + $p2 = new Permission('test.core.users.manage', 'Manage users (test)', 'core'); + $p3 = new Permission('test.commercial.clients.view', 'View clients (test)', 'commercial'); + $p2->markOrphan(); + + $this->em->persist($p1); + $this->em->persist($p2); + $this->em->persist($p3); + $this->em->flush(); + $this->em->clear(); + } + + protected function tearDown(): void + { + $this->cleanupTestPermissions(); + parent::tearDown(); + } + + public function testGetCollectionAsAdminReturns200(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/permissions'); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertArrayHasKey('member', $data); + self::assertGreaterThanOrEqual(3, $data['totalItems']); + } + + public function testCollectionFilterByModule(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/permissions', [ + 'query' => ['module' => 'core'], + ]); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + foreach ($data['member'] as $item) { + self::assertSame('core', $item['module']); + } + // Doit contenir au moins nos deux permissions core de test. + $codes = array_column($data['member'], 'code'); + self::assertContains('test.core.users.view', $codes); + self::assertContains('test.core.users.manage', $codes); + self::assertNotContains('test.commercial.clients.view', $codes); + } + + public function testCollectionFilterByOrphanFalse(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/permissions', [ + 'query' => ['orphan' => 'false'], + ]); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + foreach ($data['member'] as $item) { + self::assertFalse($item['orphan']); + } + $codes = array_column($data['member'], 'code'); + self::assertContains('test.core.users.view', $codes); + self::assertNotContains('test.core.users.manage', $codes); + } + + public function testGetItemAsAdminReturnsAllReadFields(): void + { + /** @var null|Permission $permission */ + $permission = $this->em->getRepository(Permission::class) + ->findOneBy(['code' => 'test.core.users.view']) + ; + self::assertNotNull($permission); + + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/permissions/'.$permission->getId()); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertSame($permission->getId(), $data['id']); + self::assertSame('test.core.users.view', $data['code']); + self::assertSame('View users (test)', $data['label']); + self::assertSame('core', $data['module']); + self::assertFalse($data['orphan']); + } + + public function testPostIsMethodNotAllowed(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('POST', '/api/permissions', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => ['code' => 'test.foo.bar.baz', 'label' => 'Foo', 'module' => 'foo'], + ]); + + self::assertResponseStatusCodeSame(405); + } + + public function testUnauthenticatedReturns401(): void + { + $client = self::createClient(); + $client->request('GET', '/api/permissions'); + + self::assertResponseStatusCodeSame(401); + } + + public function testNonAdminReturns403(): void + { + $client = $this->authenticatedClient('alice', 'alice'); + $client->request('GET', '/api/permissions'); + + self::assertResponseStatusCodeSame(403); + } + + private function cleanupTestPermissions(): void + { + $this->em->createQuery( + 'DELETE FROM '.Permission::class.' p WHERE p.code LIKE :prefix' + )->setParameter('prefix', self::TEST_CODE_PREFIX.'%')->execute(); + } + + /** + * Cree un client authentifie via /login_check. La configuration du projet + * pose le JWT dans un cookie HTTP-only `BEARER` (cf. lexik_jwt_authentication.yaml) + * et retire le token du body de reponse ; le client BrowserKit persiste + * automatiquement le cookie pour les requetes suivantes. + */ + private function authenticatedClient(string $username, string $password): Client + { + $client = self::createClient(); + $response = $client->request('POST', '/login_check', [ + 'headers' => ['Content-Type' => 'application/json'], + 'json' => ['username' => $username, 'password' => $password], + ]); + + self::assertContains( + $response->getStatusCode(), + [200, 204], + 'Login failed for '.$username.': '.$response->getStatusCode(), + ); + + return $client; + } +} -- 2.39.5 From f79f061131387d22a1b685a61d09062da3451cee Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 11:15:41 +0200 Subject: [PATCH 03/11] fix(test) : RBAC #344 - corrige EM stale et ajoute cas orphan=true --- config/packages/api_platform.yaml | 3 +- tests/Module/Core/Api/PermissionApiTest.php | 59 +++++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/config/packages/api_platform.yaml b/config/packages/api_platform.yaml index 7558fb2..fc3d6a9 100644 --- a/config/packages/api_platform.yaml +++ b/config/packages/api_platform.yaml @@ -1,7 +1,8 @@ api_platform: title: Coltura API version: 1.0.0 - # Scan des modules pour decouvrir les classes ApiResource et ApiFilter. + # Scan du module Core pour decouvrir les classes ApiResource et ApiFilter. + # Ajouter un chemin par module lors de l'ajout d'entites ApiResource dans d'autres modules. # Sans ces paths, le compile pass d'API Platform ne declare pas les # services de filtres annotes (les filtres etaient silencieusement # ignores sur Permission — cf. ticket #344). diff --git a/tests/Module/Core/Api/PermissionApiTest.php b/tests/Module/Core/Api/PermissionApiTest.php index 0e7fbe2..9658bf7 100644 --- a/tests/Module/Core/Api/PermissionApiTest.php +++ b/tests/Module/Core/Api/PermissionApiTest.php @@ -27,14 +27,16 @@ final class PermissionApiTest extends ApiTestCase // eviter la deprecation emise a la creation du client de test. protected static ?bool $alwaysBootKernel = true; - private EntityManagerInterface $em; - protected function setUp(): void { parent::setUp(); + // On boote le kernel une fois pour pouvoir seeder les fixtures. + // ATTENTION : ne pas stocker l'EntityManager dans une propriete, + // chaque createClient() dans les tests rebootera le kernel et + // invalidera tout EM capture ici (cf. $alwaysBootKernel = true). self::bootKernel(); - $this->em = self::getContainer()->get('doctrine')->getManager(); + $em = $this->getEm(); // Nettoyage defensif au cas ou un run precedent aurait laisse des restes. $this->cleanupTestPermissions(); @@ -46,11 +48,11 @@ final class PermissionApiTest extends ApiTestCase $p3 = new Permission('test.commercial.clients.view', 'View clients (test)', 'commercial'); $p2->markOrphan(); - $this->em->persist($p1); - $this->em->persist($p2); - $this->em->persist($p3); - $this->em->flush(); - $this->em->clear(); + $em->persist($p1); + $em->persist($p2); + $em->persist($p3); + $em->flush(); + $em->clear(); } protected function tearDown(): void @@ -66,6 +68,9 @@ final class PermissionApiTest extends ApiTestCase self::assertResponseIsSuccessful(); $data = $response->toArray(); + // API Platform 4 emet du JSON-LD 1.1 avec un @context qui utilise un + // @vocab : les cles sortent donc non prefixees (`member`, `totalItems`) + // au lieu des anciennes `hydra:member` / `hydra:totalItems`. self::assertArrayHasKey('member', $data); self::assertGreaterThanOrEqual(3, $data['totalItems']); } @@ -89,6 +94,26 @@ final class PermissionApiTest extends ApiTestCase self::assertNotContains('test.commercial.clients.view', $codes); } + public function testCollectionFilterByOrphanTrue(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/permissions', [ + 'query' => ['orphan' => 'true'], + ]); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + foreach ($data['member'] as $item) { + self::assertTrue($item['orphan']); + } + $codes = array_column($data['member'], 'code'); + // La permission marquee orpheline dans setUp() doit remonter... + self::assertContains('test.core.users.manage', $codes); + // ...et celles non orphelines doivent etre exclues. + self::assertNotContains('test.core.users.view', $codes); + self::assertNotContains('test.commercial.clients.view', $codes); + } + public function testCollectionFilterByOrphanFalse(): void { $client = $this->authenticatedClient('admin', 'admin'); @@ -109,7 +134,7 @@ final class PermissionApiTest extends ApiTestCase public function testGetItemAsAdminReturnsAllReadFields(): void { /** @var null|Permission $permission */ - $permission = $this->em->getRepository(Permission::class) + $permission = $this->getEm()->getRepository(Permission::class) ->findOneBy(['code' => 'test.core.users.view']) ; self::assertNotNull($permission); @@ -153,9 +178,23 @@ final class PermissionApiTest extends ApiTestCase self::assertResponseStatusCodeSame(403); } + /** + * Recupere l'EntityManager depuis le container courant. A utiliser a + * chaque appel : apres un createClient(), le kernel est reboote et tout + * EM precedemment capture est invalide. + */ + private function getEm(): EntityManagerInterface + { + if (!self::$kernel) { + self::bootKernel(); + } + + return self::getContainer()->get('doctrine')->getManager(); + } + private function cleanupTestPermissions(): void { - $this->em->createQuery( + $this->getEm()->createQuery( 'DELETE FROM '.Permission::class.' p WHERE p.code LIKE :prefix' )->setParameter('prefix', self::TEST_CODE_PREFIX.'%')->execute(); } -- 2.39.5 From 7be0260b2958fdfa50fc00ca13d663b3329dc589 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 11:41:21 +0200 Subject: [PATCH 04/11] feat(core) : RBAC #344 - API Platform Role CRUD nominal + validators --- src/Module/Core/Domain/Entity/Role.php | 64 +++++ tests/Module/Core/Api/RoleApiTest.php | 319 +++++++++++++++++++++++++ 2 files changed, 383 insertions(+) create mode 100644 tests/Module/Core/Api/RoleApiTest.php diff --git a/src/Module/Core/Domain/Entity/Role.php b/src/Module/Core/Domain/Entity/Role.php index 7583454..f5a2a31 100644 --- a/src/Module/Core/Domain/Entity/Role.php +++ b/src/Module/Core/Domain/Entity/Role.php @@ -4,12 +4,24 @@ declare(strict_types=1); namespace App\Module\Core\Domain\Entity; +use ApiPlatform\Doctrine\Orm\Filter\BooleanFilter; +use ApiPlatform\Metadata\ApiFilter; +use ApiPlatform\Metadata\ApiResource; +use ApiPlatform\Metadata\Delete; +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\GetCollection; +use ApiPlatform\Metadata\Patch; +use ApiPlatform\Metadata\Post; use App\Module\Core\Domain\Exception\SystemRoleDeletionException; use App\Module\Core\Infrastructure\Doctrine\DoctrineRoleRepository; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping as ORM; +use Symfony\Bridge\Doctrine\Validator\Constraints\UniqueEntity; +use Symfony\Component\Serializer\Attribute\Groups; +use Symfony\Component\Serializer\Attribute\SerializedName; +use Symfony\Component\Validator\Constraints as Assert; /** * Role RBAC : groupe nomme de permissions assignable a un utilisateur. @@ -18,27 +30,72 @@ use Doctrine\ORM\Mapping as ORM; * "personnalise" (cree par un administrateur). Seuls les roles personnalises * peuvent etre supprimes. */ +#[ApiResource( + operations: [ + new GetCollection( + normalizationContext: ['groups' => ['role:read']], + // TODO ticket #345 : remplacer par is_granted('core.roles.manage') + security: "is_granted('ROLE_ADMIN')", + ), + new Get( + normalizationContext: ['groups' => ['role:read']], + // TODO ticket #345 : remplacer par is_granted('core.roles.manage') + security: "is_granted('ROLE_ADMIN')", + ), + new Post( + normalizationContext: ['groups' => ['role:read']], + denormalizationContext: ['groups' => ['role:write']], + // TODO ticket #345 : remplacer par is_granted('core.roles.manage') + security: "is_granted('ROLE_ADMIN')", + ), + new Patch( + normalizationContext: ['groups' => ['role:read']], + denormalizationContext: ['groups' => ['role:write']], + // TODO ticket #345 : remplacer par is_granted('core.roles.manage') + security: "is_granted('ROLE_ADMIN')", + ), + new Delete( + // TODO ticket #345 : remplacer par is_granted('core.roles.manage') + security: "is_granted('ROLE_ADMIN')", + ), + ], + normalizationContext: ['groups' => ['role:read']], + denormalizationContext: ['groups' => ['role:write']], +)] +#[ApiFilter(BooleanFilter::class, properties: ['isSystem'])] #[ORM\Entity(repositoryClass: DoctrineRoleRepository::class)] #[ORM\Table(name: '`role`')] #[ORM\UniqueConstraint(name: 'uniq_role_code', columns: ['code'])] #[ORM\Index(name: 'idx_role_is_system', columns: ['is_system'])] +#[UniqueEntity(fields: ['code'], message: 'Un role avec ce code existe deja.')] class Role { #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] + #[Groups(['role:read'])] private ?int $id = null; #[ORM\Column(length: 100)] + #[Groups(['role:read', 'role:write'])] + #[Assert\NotBlank] + #[Assert\Regex(pattern: '/^[a-z][a-z0-9_]*$/', message: 'Le code doit etre en snake_case et commencer par une lettre minuscule.')] private string $code; #[ORM\Column(length: 255)] + #[Groups(['role:read', 'role:write'])] + #[Assert\NotBlank] private string $label; #[ORM\Column(type: Types::TEXT, nullable: true)] + #[Groups(['role:read', 'role:write'])] private ?string $description = null; + // Volontairement exclu du groupe `role:write` : un client ne doit jamais + // pouvoir positionner ce flag via l'API. Seules les fixtures et migrations + // creent les roles systeme. #[ORM\Column(name: 'is_system', options: ['default' => false])] + #[Groups(['role:read'])] private bool $isSystem = false; /** @var Collection */ @@ -53,6 +110,7 @@ class Role // projection cachee (ticket a ouvrir a ce moment-la). #[ORM\ManyToMany(targetEntity: Permission::class, fetch: 'EAGER')] #[ORM\JoinTable(name: 'role_permission')] + #[Groups(['role:read', 'role:write'])] private Collection $permissions; public function __construct(string $code, string $label, bool $isSystem = false, ?string $description = null) @@ -84,6 +142,12 @@ class Role return $this->description; } + // Le getter est annote directement car la convention Symfony PropertyInfo + // strip le prefixe `is` et exposerait le champ sous le nom `system`. On + // pose donc un SerializedName explicite pour garantir la sortie JSON-LD + // sous `isSystem`, nom attendu par les clients de l'API. + #[Groups(['role:read'])] + #[SerializedName('isSystem')] public function isSystem(): bool { return $this->isSystem; diff --git a/tests/Module/Core/Api/RoleApiTest.php b/tests/Module/Core/Api/RoleApiTest.php new file mode 100644 index 0000000..c5683de --- /dev/null +++ b/tests/Module/Core/Api/RoleApiTest.php @@ -0,0 +1,319 @@ +getEm(); + + // Nettoyage defensif au cas ou un run precedent aurait laisse des restes. + $this->cleanupTestData(); + + // Permissions de test reutilisables (notamment pour le PATCH). + $p1 = new Permission('test.core.roles.view', 'View roles (test)', 'core'); + $p2 = new Permission('test.core.roles.manage', 'Manage roles (test)', 'core'); + $em->persist($p1); + $em->persist($p2); + + // Role custom existant : utilise pour les GET / PATCH / DELETE. + $editor = new Role('test_editor', 'Editeur (test)', false, 'Role de test editeur'); + $em->persist($editor); + + // Deuxieme role custom : pour enrichir les collections. + $viewer = new Role('test_viewer', 'Visualisateur (test)', false); + $em->persist($viewer); + + $em->flush(); + $em->clear(); + } + + protected function tearDown(): void + { + $this->cleanupTestData(); + parent::tearDown(); + } + + public function testPostCreatesCustomRoleAsAdmin(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('POST', '/api/roles', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + 'code' => 'test_new_editor', + 'label' => 'Nouvel editeur', + 'description' => 'Role de test', + ], + ]); + + self::assertResponseStatusCodeSame(201); + $data = $response->toArray(); + self::assertSame('test_new_editor', $data['code']); + self::assertSame('Nouvel editeur', $data['label']); + self::assertFalse($data['isSystem']); + + // Verification cote base : le role existe et isSystem = false. + $persisted = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => 'test_new_editor']); + self::assertNotNull($persisted); + self::assertFalse($persisted->isSystem()); + } + + public function testPostWithDuplicateCodeReturns422(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('POST', '/api/roles', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + // `admin` est un role systeme charge par les fixtures. + 'code' => 'admin', + 'label' => 'Tentative de doublon', + ], + ]); + + self::assertResponseStatusCodeSame(422); + } + + public function testPostWithInvalidCodeReturns422(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('POST', '/api/roles', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + // Majuscules interdites par la regex snake_case. + 'code' => 'BadCode', + 'label' => 'Code invalide', + ], + ]); + + self::assertResponseStatusCodeSame(422); + } + + public function testPostWithIsSystemTrueIgnoresItAndPersistsFalse(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('POST', '/api/roles', [ + 'headers' => ['Content-Type' => 'application/ld+json'], + 'json' => [ + 'code' => 'test_sneaky', + 'label' => 'Tentative systeme', + 'isSystem' => true, + ], + ]); + + self::assertResponseStatusCodeSame(201); + $data = $response->toArray(); + self::assertFalse($data['isSystem']); + + $persisted = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => 'test_sneaky']); + self::assertNotNull($persisted); + self::assertFalse($persisted->isSystem()); + } + + public function testGetCollectionAsAdminReturnsRoles(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/roles'); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertArrayHasKey('member', $data); + // Au moins admin systeme + user systeme + test_editor + test_viewer. + self::assertGreaterThanOrEqual(2, $data['totalItems']); + $codes = array_column($data['member'], 'code'); + self::assertContains('test_editor', $codes); + } + + public function testGetCollectionFilterByIsSystemTrue(): void + { + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/roles', [ + 'query' => ['isSystem' => 'true'], + ]); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + foreach ($data['member'] as $item) { + self::assertTrue($item['isSystem']); + } + $codes = array_column($data['member'], 'code'); + self::assertNotContains('test_editor', $codes); + self::assertNotContains('test_viewer', $codes); + } + + public function testGetItemReturnsAllReadFields(): void + { + $role = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => 'test_editor']); + self::assertNotNull($role); + + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('GET', '/api/roles/'.$role->getId()); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertSame('test_editor', $data['code']); + self::assertSame('Editeur (test)', $data['label']); + self::assertSame('Role de test editeur', $data['description']); + self::assertFalse($data['isSystem']); + self::assertArrayHasKey('permissions', $data); + self::assertIsArray($data['permissions']); + } + + public function testPatchCustomRoleUpdatesLabelAndAddsPermission(): void + { + $em = $this->getEm(); + $role = $em->getRepository(Role::class)->findOneBy(['code' => 'test_editor']); + self::assertNotNull($role); + $permission = $em->getRepository(Permission::class)->findOneBy(['code' => 'test.core.roles.view']); + self::assertNotNull($permission); + + $client = $this->authenticatedClient('admin', 'admin'); + $response = $client->request('PATCH', '/api/roles/'.$role->getId(), [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [ + 'label' => 'Editeur modifie', + 'permissions' => ['/api/permissions/'.$permission->getId()], + ], + ]); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertSame('Editeur modifie', $data['label']); + self::assertCount(1, $data['permissions']); + + // Verification cote base. + $em->clear(); + + /** @var Role $reloaded */ + $reloaded = $em->getRepository(Role::class)->findOneBy(['code' => 'test_editor']); + self::assertSame('Editeur modifie', $reloaded->getLabel()); + self::assertCount(1, $reloaded->getPermissions()); + } + + public function testDeleteCustomRoleReturns204(): void + { + $role = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => 'test_viewer']); + self::assertNotNull($role); + $id = $role->getId(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('DELETE', '/api/roles/'.$id); + + self::assertResponseStatusCodeSame(204); + + $em = $this->getEm(); + $em->clear(); + self::assertNull($em->getRepository(Role::class)->find($id)); + } + + public function testUnauthenticatedGetCollectionReturns401(): void + { + $client = self::createClient(); + $client->request('GET', '/api/roles'); + + self::assertResponseStatusCodeSame(401); + } + + public function testNonAdminGetCollectionReturns403(): void + { + $client = $this->authenticatedClient('alice', 'alice'); + $client->request('GET', '/api/roles'); + + self::assertResponseStatusCodeSame(403); + } + + /** + * Recupere l'EntityManager depuis le container courant. A utiliser a + * chaque appel : apres un createClient(), le kernel est reboote et tout + * EM precedemment capture est invalide. + */ + private function getEm(): EntityManagerInterface + { + if (!self::$kernel) { + self::bootKernel(); + } + + return self::getContainer()->get('doctrine')->getManager(); + } + + /** + * Purge les donnees de test (roles et permissions prefixees `test.`). + * Ne touche JAMAIS aux roles systeme `admin` et `user` charges par les + * fixtures. + */ + private function cleanupTestData(): void + { + $em = $this->getEm(); + + // Ordre important : role_permission lie aux deux, on vide les roles + // custom d'abord (la jointure est cascade supprimee par Doctrine lors + // du remove() du cote proprietaire). En DQL bulk on passe par les + // entites, Doctrine genere les DELETE de la table de jointure. + $em->createQuery( + 'DELETE FROM '.Role::class.' r WHERE r.code LIKE :prefix' + )->setParameter('prefix', self::TEST_ROLE_PREFIX.'%')->execute(); + + $em->createQuery( + 'DELETE FROM '.Permission::class.' p WHERE p.code LIKE :prefix' + )->setParameter('prefix', self::TEST_PERMISSION_PREFIX.'%')->execute(); + } + + /** + * Cree un client authentifie via /login_check (cookie BEARER pose par + * lexik_jwt_authentication et persiste automatiquement par BrowserKit). + */ + private function authenticatedClient(string $username, string $password): Client + { + $client = self::createClient(); + $response = $client->request('POST', '/login_check', [ + 'headers' => ['Content-Type' => 'application/json'], + 'json' => ['username' => $username, 'password' => $password], + ]); + + self::assertContains( + $response->getStatusCode(), + [200, 204], + 'Login failed for '.$username.': '.$response->getStatusCode(), + ); + + return $client; + } +} -- 2.39.5 From efc12c8bdb448204adbb472a2bd6bbb863b21482 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 11:53:01 +0200 Subject: [PATCH 05/11] fix(test) : RBAC #344 - role test cleanup + SystemRoles constant + assertion seuil --- tests/Module/Core/Api/RoleApiTest.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/Module/Core/Api/RoleApiTest.php b/tests/Module/Core/Api/RoleApiTest.php index c5683de..a0b0fa4 100644 --- a/tests/Module/Core/Api/RoleApiTest.php +++ b/tests/Module/Core/Api/RoleApiTest.php @@ -8,6 +8,7 @@ use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; 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\Security\SystemRoles; use Doctrine\ORM\EntityManagerInterface; /** @@ -104,7 +105,7 @@ final class RoleApiTest extends ApiTestCase 'headers' => ['Content-Type' => 'application/ld+json'], 'json' => [ // `admin` est un role systeme charge par les fixtures. - 'code' => 'admin', + 'code' => SystemRoles::ADMIN_CODE, 'label' => 'Tentative de doublon', ], ]); @@ -157,7 +158,7 @@ final class RoleApiTest extends ApiTestCase $data = $response->toArray(); self::assertArrayHasKey('member', $data); // Au moins admin systeme + user systeme + test_editor + test_viewer. - self::assertGreaterThanOrEqual(2, $data['totalItems']); + self::assertGreaterThanOrEqual(4, $data['totalItems']); $codes = array_column($data['member'], 'code'); self::assertContains('test_editor', $codes); } @@ -283,10 +284,12 @@ final class RoleApiTest extends ApiTestCase { $em = $this->getEm(); - // Ordre important : role_permission lie aux deux, on vide les roles - // custom d'abord (la jointure est cascade supprimee par Doctrine lors - // du remove() du cote proprietaire). En DQL bulk on passe par les - // entites, Doctrine genere les DELETE de la table de jointure. + // Le cascade FK de la migration #343 (ON DELETE CASCADE sur + // role_permission.role_id et permission_id) nettoie automatiquement + // role_permission lors du DELETE SQL emis par Doctrine, meme via DQL + // bulk delete : le cascade est applique au niveau FK par PostgreSQL, + // pas par l'Unit of Work Doctrine. Verifie par comptage avant/apres + // runs successifs de la suite (stable a la ligne de base systeme). $em->createQuery( 'DELETE FROM '.Role::class.' r WHERE r.code LIKE :prefix' )->setParameter('prefix', self::TEST_ROLE_PREFIX.'%')->execute(); -- 2.39.5 From d527fbe2d1a59151a402aaaaaf37a6cddec4e220 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 11:58:37 +0200 Subject: [PATCH 06/11] feat(core) : RBAC #344 - RoleProcessor + gardes systeme et code immuable --- src/Module/Core/Domain/Entity/Role.php | 17 ++ .../State/Processor/RoleProcessor.php | 78 +++++++ tests/Module/Core/Api/RoleApiTest.php | 71 ++++++ .../State/Processor/RoleProcessorTest.php | 212 ++++++++++++++++++ 4 files changed, 378 insertions(+) create mode 100644 src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php create mode 100644 tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php diff --git a/src/Module/Core/Domain/Entity/Role.php b/src/Module/Core/Domain/Entity/Role.php index f5a2a31..14d20cd 100644 --- a/src/Module/Core/Domain/Entity/Role.php +++ b/src/Module/Core/Domain/Entity/Role.php @@ -13,6 +13,7 @@ use ApiPlatform\Metadata\GetCollection; use ApiPlatform\Metadata\Patch; use ApiPlatform\Metadata\Post; use App\Module\Core\Domain\Exception\SystemRoleDeletionException; +use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\RoleProcessor; use App\Module\Core\Infrastructure\Doctrine\DoctrineRoleRepository; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; @@ -47,16 +48,19 @@ use Symfony\Component\Validator\Constraints as Assert; denormalizationContext: ['groups' => ['role:write']], // TODO ticket #345 : remplacer par is_granted('core.roles.manage') security: "is_granted('ROLE_ADMIN')", + processor: RoleProcessor::class, ), new Patch( normalizationContext: ['groups' => ['role:read']], denormalizationContext: ['groups' => ['role:write']], // TODO ticket #345 : remplacer par is_granted('core.roles.manage') security: "is_granted('ROLE_ADMIN')", + processor: RoleProcessor::class, ), new Delete( // TODO ticket #345 : remplacer par is_granted('core.roles.manage') security: "is_granted('ROLE_ADMIN')", + processor: RoleProcessor::class, ), ], normalizationContext: ['groups' => ['role:read']], @@ -159,6 +163,19 @@ class Role return $this->permissions; } + /** + * Setter expose uniquement a la denormalisation API Platform pour + * permettre au RoleProcessor de detecter une tentative de modification + * du code (garde "code immuable"). Le code reste en pratique fige apres + * creation : le processor refuse toute modification via 400. + */ + public function setCode(string $code): static + { + $this->code = $code; + + return $this; + } + /** * Met a jour le libelle affichable du role. Le code reste immuable pour * garantir la stabilite des references cote fixtures et migrations. diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php new file mode 100644 index 0000000..70a8ba3 --- /dev/null +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php @@ -0,0 +1,78 @@ + + */ +final class RoleProcessor implements ProcessorInterface +{ + public function __construct( + #[Autowire(service: 'api_platform.doctrine.orm.state.persist_processor')] + private readonly ProcessorInterface $persistProcessor, + #[Autowire(service: 'api_platform.doctrine.orm.state.remove_processor')] + private readonly ProcessorInterface $removeProcessor, + private readonly EntityManagerInterface $entityManager, + ) {} + + public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed + { + if (!$data instanceof Role) { + // Securite : si le provider n'a pas fourni un Role, on delegue + // quand meme au processor approprie pour ne pas etouffer + // silencieusement un bug de configuration. + return $operation instanceof DeleteOperationInterface + ? $this->removeProcessor->process($data, $operation, $uriVariables, $context) + : $this->persistProcessor->process($data, $operation, $uriVariables, $context); + } + + if ($operation instanceof DeleteOperationInterface) { + try { + $data->ensureDeletable(); + } catch (SystemRoleDeletionException $e) { + // Traduction HTTP : le domaine reste pur, l'API renvoie 403. + throw new AccessDeniedHttpException($e->getMessage(), $e); + } + + return $this->removeProcessor->process($data, $operation, $uriVariables, $context); + } + + // Ecriture (POST/PATCH) : verifier l'immuabilite du `code`. + // L'UnitOfWork n'expose un etat d'origine que pour les entites deja + // managees (PATCH). Pour un POST (entite nouvelle), `getOriginalEntityData` + // retourne un tableau vide : aucune comparaison necessaire. + $originalData = $this->entityManager->getUnitOfWork()->getOriginalEntityData($data); + + if (isset($originalData['code']) && $originalData['code'] !== $data->getCode()) { + throw new BadRequestHttpException("Le code d'un role est immuable apres creation."); + } + + return $this->persistProcessor->process($data, $operation, $uriVariables, $context); + } +} diff --git a/tests/Module/Core/Api/RoleApiTest.php b/tests/Module/Core/Api/RoleApiTest.php index a0b0fa4..e204652 100644 --- a/tests/Module/Core/Api/RoleApiTest.php +++ b/tests/Module/Core/Api/RoleApiTest.php @@ -245,6 +245,77 @@ final class RoleApiTest extends ApiTestCase self::assertNull($em->getRepository(Role::class)->find($id)); } + public function testDeleteSystemRoleReturns403(): void + { + $role = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE]); + self::assertNotNull($role); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('DELETE', '/api/roles/'.$role->getId()); + + self::assertResponseStatusCodeSame(403); + + // Le role systeme doit toujours exister. + $em = $this->getEm(); + $em->clear(); + self::assertNotNull($em->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE])); + } + + public function testPatchSystemRoleLabelReturns200(): void + { + $em = $this->getEm(); + $role = $em->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE]); + self::assertNotNull($role); + $originalLabel = $role->getLabel(); + $roleId = $role->getId(); + + $client = $this->authenticatedClient('admin', 'admin'); + + try { + $response = $client->request('PATCH', '/api/roles/'.$roleId, [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['label' => 'Administrateur (modifie test)'], + ]); + + self::assertResponseIsSuccessful(); + $data = $response->toArray(); + self::assertSame('Administrateur (modifie test)', $data['label']); + self::assertSame(SystemRoles::ADMIN_CODE, $data['code']); + self::assertTrue($data['isSystem']); + } finally { + // Restauration defensive du label original pour ne pas polluer + // les tests suivants (les fixtures systeme sont partagees). + $em = $this->getEm(); + + /** @var null|Role $reloaded */ + $reloaded = $em->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE]); + if (null !== $reloaded && $reloaded->getLabel() !== $originalLabel) { + $reloaded->setLabel($originalLabel); + $em->flush(); + } + } + } + + public function testPatchRoleCodeChangeReturns400(): void + { + $role = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => 'test_editor']); + self::assertNotNull($role); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/roles/'.$role->getId(), [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['code' => 'test_editor_renamed'], + ]); + + self::assertResponseStatusCodeSame(400); + + // Verification cote base : le code d'origine n'a pas bouge. + $em = $this->getEm(); + $em->clear(); + self::assertNotNull($em->getRepository(Role::class)->findOneBy(['code' => 'test_editor'])); + self::assertNull($em->getRepository(Role::class)->findOneBy(['code' => 'test_editor_renamed'])); + } + public function testUnauthenticatedGetCollectionReturns401(): void { $client = self::createClient(); diff --git a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php new file mode 100644 index 0000000..dc890b9 --- /dev/null +++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php @@ -0,0 +1,212 @@ +persistProcessor = $this->createMock(ProcessorInterface::class); + $this->removeProcessor = $this->createMock(ProcessorInterface::class); + $this->entityManager = $this->createMock(EntityManagerInterface::class); + $this->unitOfWork = $this->createMock(UnitOfWork::class); + + $this->entityManager->method('getUnitOfWork')->willReturn($this->unitOfWork); + + $this->processor = new RoleProcessor( + $this->persistProcessor, + $this->removeProcessor, + $this->entityManager, + ); + } + + public function testDeleteCustomRoleDelegatesToRemoveProcessor(): void + { + $role = new Role('editor', 'Editor', false); + + $this->removeProcessor + ->expects(self::once()) + ->method('process') + ->with($role) + ->willReturn(null) + ; + + $this->persistProcessor->expects(self::never())->method('process'); + + $result = $this->processor->process($role, new Delete()); + + self::assertNull($result); + } + + public function testDeleteSystemRoleThrowsAccessDeniedHttpException(): void + { + $role = new Role('admin', 'Admin', true); + + $this->removeProcessor->expects(self::never())->method('process'); + $this->persistProcessor->expects(self::never())->method('process'); + + $this->expectException(AccessDeniedHttpException::class); + + $this->processor->process($role, new Delete()); + } + + public function testPostCreatesCustomRoleDelegatesToPersistProcessor(): void + { + $role = new Role('editor', 'Editor', false); + + // Entite nouvelle : l'UnitOfWork n'a pas d'etat d'origine. + $this->unitOfWork + ->expects(self::once()) + ->method('getOriginalEntityData') + ->with($role) + ->willReturn([]) + ; + + $this->persistProcessor + ->expects(self::once()) + ->method('process') + ->with($role) + ->willReturn($role) + ; + + $this->removeProcessor->expects(self::never())->method('process'); + + $result = $this->processor->process($role, new Post()); + + self::assertSame($role, $result); + } + + public function testPatchWithChangedCodeThrowsBadRequestHttpException(): void + { + // L'entite arrive avec le nouveau code deja applique par le denormalizer. + $role = new Role('editor_renamed', 'Editor', false); + $this->setRoleId($role, 42); + + $this->unitOfWork + ->expects(self::once()) + ->method('getOriginalEntityData') + ->with($role) + ->willReturn([ + 'id' => 42, + 'code' => 'editor', + 'label' => 'Editor', + 'isSystem' => false, + ]) + ; + + $this->persistProcessor->expects(self::never())->method('process'); + $this->removeProcessor->expects(self::never())->method('process'); + + $this->expectException(BadRequestHttpException::class); + $this->expectExceptionMessage("Le code d'un role est immuable apres creation."); + + $this->processor->process($role, new Patch()); + } + + public function testPatchWithUnchangedCodeDelegatesToPersistProcessor(): void + { + $role = new Role('editor', 'Editor modifie', false, 'desc'); + $this->setRoleId($role, 42); + + $this->unitOfWork + ->expects(self::once()) + ->method('getOriginalEntityData') + ->with($role) + ->willReturn([ + 'id' => 42, + 'code' => 'editor', + 'label' => 'Editor', + 'isSystem' => false, + ]) + ; + + $this->persistProcessor + ->expects(self::once()) + ->method('process') + ->with($role) + ->willReturn($role) + ; + + $this->removeProcessor->expects(self::never())->method('process'); + + $result = $this->processor->process($role, new Patch()); + + self::assertSame($role, $result); + } + + public function testPatchSystemRoleLabelDelegatesToPersistProcessor(): void + { + // Regle uniforme : un role systeme peut voir son label modifie tant + // que son code reste inchange. Seul le DELETE est bloque. + $role = new Role('admin', 'Administrateur', true); + $this->setRoleId($role, 1); + + $this->unitOfWork + ->expects(self::once()) + ->method('getOriginalEntityData') + ->with($role) + ->willReturn([ + 'id' => 1, + 'code' => 'admin', + 'label' => 'Admin', + 'isSystem' => true, + ]) + ; + + $this->persistProcessor + ->expects(self::once()) + ->method('process') + ->with($role) + ->willReturn($role) + ; + + $this->removeProcessor->expects(self::never())->method('process'); + + $result = $this->processor->process($role, new Patch()); + + self::assertSame($role, $result); + } + + /** + * Positionne l'id d'un Role via reflection pour simuler une entite deja + * persistee (les mocks d'UnitOfWork n'alimentent pas l'id tout seul). + */ + private function setRoleId(Role $role, int $id): void + { + $refl = new ReflectionClass($role); + $prop = $refl->getProperty('id'); + $prop->setAccessible(true); + $prop->setValue($role, $id); + } +} -- 2.39.5 From 87aa1d0b04de7c1b9a22b758402877aa9ba80842 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 12:05:26 +0200 Subject: [PATCH 07/11] test(core) : RBAC #344 - renforce docblock setCode + assertion message exception --- src/Module/Core/Domain/Entity/Role.php | 4 ++++ .../ApiPlatform/State/Processor/RoleProcessorTest.php | 1 + 2 files changed, 5 insertions(+) diff --git a/src/Module/Core/Domain/Entity/Role.php b/src/Module/Core/Domain/Entity/Role.php index 14d20cd..ae98b38 100644 --- a/src/Module/Core/Domain/Entity/Role.php +++ b/src/Module/Core/Domain/Entity/Role.php @@ -168,6 +168,10 @@ class Role * permettre au RoleProcessor de detecter une tentative de modification * du code (garde "code immuable"). Le code reste en pratique fige apres * creation : le processor refuse toute modification via 400. + * + * @internal Ne PAS appeler depuis le domaine, les fixtures ou les commandes. + * Hors contexte API Platform, cette methode modifie silencieusement + * le code sans aucun garde. */ public function setCode(string $code): static { diff --git a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php index dc890b9..c90fe5a 100644 --- a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php +++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php @@ -77,6 +77,7 @@ final class RoleProcessorTest extends TestCase $this->persistProcessor->expects(self::never())->method('process'); $this->expectException(AccessDeniedHttpException::class); + $this->expectExceptionMessage('Le role systeme "admin" ne peut pas etre supprime.'); $this->processor->process($role, new Delete()); } -- 2.39.5 From 168a47f2b8adfa6f8f681739b3a0fccebffa81eb Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 12:14:20 +0200 Subject: [PATCH 08/11] refactor(test) : RBAC #344 - AbstractApiTestCase pour mutualiser auth JWT Extrait l'helper authenticatedClient(), $alwaysBootKernel et getEm() dans une classe de base commune aux tests fonctionnels API Platform du module Core. Supprime la duplication entre PermissionApiTest et RoleApiTest (flaggee en code review de la Task 2). Prepare le terrain pour le nouveau UserRbacApiTest introduit avec la Task 4. --- tests/Module/Core/Api/AbstractApiTestCase.php | 66 +++++++++++++++++++ tests/Module/Core/Api/PermissionApiTest.php | 45 +------------ tests/Module/Core/Api/RoleApiTest.php | 44 +------------ 3 files changed, 68 insertions(+), 87 deletions(-) create mode 100644 tests/Module/Core/Api/AbstractApiTestCase.php diff --git a/tests/Module/Core/Api/AbstractApiTestCase.php b/tests/Module/Core/Api/AbstractApiTestCase.php new file mode 100644 index 0000000..3a15d6b --- /dev/null +++ b/tests/Module/Core/Api/AbstractApiTestCase.php @@ -0,0 +1,66 @@ +get('doctrine')->getManager(); + } + + /** + * Cree un client authentifie via /login_check. La configuration du projet + * pose le JWT dans un cookie HTTP-only `BEARER` (cf. lexik_jwt_authentication.yaml) + * et retire le token du body de reponse ; le client BrowserKit persiste + * automatiquement le cookie pour les requetes suivantes. + */ + protected function authenticatedClient(string $username, string $password): Client + { + $client = self::createClient(); + $response = $client->request('POST', '/login_check', [ + 'headers' => ['Content-Type' => 'application/json'], + 'json' => ['username' => $username, 'password' => $password], + ]); + + self::assertContains( + $response->getStatusCode(), + [200, 204], + 'Login failed for '.$username.': '.$response->getStatusCode(), + ); + + return $client; + } +} diff --git a/tests/Module/Core/Api/PermissionApiTest.php b/tests/Module/Core/Api/PermissionApiTest.php index 9658bf7..f097b95 100644 --- a/tests/Module/Core/Api/PermissionApiTest.php +++ b/tests/Module/Core/Api/PermissionApiTest.php @@ -4,10 +4,7 @@ declare(strict_types=1); namespace App\Tests\Module\Core\Api; -use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; -use ApiPlatform\Symfony\Bundle\Test\Client; use App\Module\Core\Domain\Entity\Permission; -use Doctrine\ORM\EntityManagerInterface; /** * Tests fonctionnels de l'exposition API Platform de l'entite Permission. @@ -20,12 +17,9 @@ use Doctrine\ORM\EntityManagerInterface; * * @internal */ -final class PermissionApiTest extends ApiTestCase +final class PermissionApiTest extends AbstractApiTestCase { private const TEST_CODE_PREFIX = 'test.'; - // Bascule explicite sur le nouveau comportement API Platform 5 pour - // eviter la deprecation emise a la creation du client de test. - protected static ?bool $alwaysBootKernel = true; protected function setUp(): void { @@ -178,47 +172,10 @@ final class PermissionApiTest extends ApiTestCase self::assertResponseStatusCodeSame(403); } - /** - * Recupere l'EntityManager depuis le container courant. A utiliser a - * chaque appel : apres un createClient(), le kernel est reboote et tout - * EM precedemment capture est invalide. - */ - private function getEm(): EntityManagerInterface - { - if (!self::$kernel) { - self::bootKernel(); - } - - return self::getContainer()->get('doctrine')->getManager(); - } - private function cleanupTestPermissions(): void { $this->getEm()->createQuery( 'DELETE FROM '.Permission::class.' p WHERE p.code LIKE :prefix' )->setParameter('prefix', self::TEST_CODE_PREFIX.'%')->execute(); } - - /** - * Cree un client authentifie via /login_check. La configuration du projet - * pose le JWT dans un cookie HTTP-only `BEARER` (cf. lexik_jwt_authentication.yaml) - * et retire le token du body de reponse ; le client BrowserKit persiste - * automatiquement le cookie pour les requetes suivantes. - */ - private function authenticatedClient(string $username, string $password): Client - { - $client = self::createClient(); - $response = $client->request('POST', '/login_check', [ - 'headers' => ['Content-Type' => 'application/json'], - 'json' => ['username' => $username, 'password' => $password], - ]); - - self::assertContains( - $response->getStatusCode(), - [200, 204], - 'Login failed for '.$username.': '.$response->getStatusCode(), - ); - - return $client; - } } diff --git a/tests/Module/Core/Api/RoleApiTest.php b/tests/Module/Core/Api/RoleApiTest.php index e204652..0fde030 100644 --- a/tests/Module/Core/Api/RoleApiTest.php +++ b/tests/Module/Core/Api/RoleApiTest.php @@ -4,12 +4,9 @@ declare(strict_types=1); namespace App\Tests\Module\Core\Api; -use ApiPlatform\Symfony\Bundle\Test\ApiTestCase; -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\Security\SystemRoles; -use Doctrine\ORM\EntityManagerInterface; /** * Tests fonctionnels de l'exposition API Platform de l'entite Role (CRUD nominal). @@ -24,7 +21,7 @@ use Doctrine\ORM\EntityManagerInterface; * * @internal */ -final class RoleApiTest extends ApiTestCase +final class RoleApiTest extends AbstractApiTestCase { // Prefixe pour les roles de test : `test_` (underscore) parce que les // codes de role doivent matcher `/^[a-z][a-z0-9_]*$/` (pas de point @@ -36,10 +33,6 @@ final class RoleApiTest extends ApiTestCase // module.resource.action validee dans le constructeur Permission). private const TEST_PERMISSION_PREFIX = 'test.'; - // Bascule explicite sur le nouveau comportement API Platform 5 pour - // eviter la deprecation emise a la creation du client de test. - protected static ?bool $alwaysBootKernel = true; - protected function setUp(): void { parent::setUp(); @@ -332,20 +325,6 @@ final class RoleApiTest extends ApiTestCase self::assertResponseStatusCodeSame(403); } - /** - * Recupere l'EntityManager depuis le container courant. A utiliser a - * chaque appel : apres un createClient(), le kernel est reboote et tout - * EM precedemment capture est invalide. - */ - private function getEm(): EntityManagerInterface - { - if (!self::$kernel) { - self::bootKernel(); - } - - return self::getContainer()->get('doctrine')->getManager(); - } - /** * Purge les donnees de test (roles et permissions prefixees `test.`). * Ne touche JAMAIS aux roles systeme `admin` et `user` charges par les @@ -369,25 +348,4 @@ final class RoleApiTest extends ApiTestCase 'DELETE FROM '.Permission::class.' p WHERE p.code LIKE :prefix' )->setParameter('prefix', self::TEST_PERMISSION_PREFIX.'%')->execute(); } - - /** - * Cree un client authentifie via /login_check (cookie BEARER pose par - * lexik_jwt_authentication et persiste automatiquement par BrowserKit). - */ - private function authenticatedClient(string $username, string $password): Client - { - $client = self::createClient(); - $response = $client->request('POST', '/login_check', [ - 'headers' => ['Content-Type' => 'application/json'], - 'json' => ['username' => $username, 'password' => $password], - ]); - - self::assertContains( - $response->getStatusCode(), - [200, 204], - 'Login failed for '.$username.': '.$response->getStatusCode(), - ); - - return $client; - } } -- 2.39.5 From 3c7dc88fe7297f7e81a5fa8a4bd47f6dc94dc27d Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 14:17:18 +0200 Subject: [PATCH 09/11] feat(core) : RBAC #344 - UserRbacProcessor + endpoint /users/{id}/rbac Ajoute une operation Patch dediee `PATCH /api/users/{id}/rbac` (nom `user_rbac_patch`) qui accepte exclusivement les champs RBAC isAdmin, roles et directPermissions via le groupe user:rbac:write. L'endpoint est separe volontairement du Patch profil existant pour isoler la modification des droits de celle des donnees profil (decision 0fc4e16). UserRbacProcessor delegue au PersistProcessor Doctrine decore et applique une garde auto-suicide : un admin ne peut pas retirer ses propres droits administrateur (compare l'etat entrant a l'etat UnitOfWork). La garde 'dernier admin' globale est reportee au ticket #345. La propriete Doctrine $roles est renommee $rbacRoles pour eviter la collision avec UserInterface::getRoles() (qui renvoie list) lors de la normalization API Platform. La cle JSON reste `roles` grace a SerializedName, le contrat API est inchange. Tests : 6 unitaires (UserRbacProcessorTest) + 8 fonctionnels (UserRbacApiTest) couvrant promotion admin, remplacement des collections roles/directPermissions, 401/403, filtrage du groupe denormalization (`username` ignore), preservation de isAdmin sur le Patch profil, et garde auto-suicide. --- src/Module/Core/Domain/Entity/User.php | 46 ++- .../State/Processor/UserRbacProcessor.php | 71 +++++ tests/Module/Core/Api/UserRbacApiTest.php | 271 ++++++++++++++++++ .../State/Processor/UserRbacProcessorTest.php | 216 ++++++++++++++ 4 files changed, 589 insertions(+), 15 deletions(-) create mode 100644 src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php create mode 100644 tests/Module/Core/Api/UserRbacApiTest.php create mode 100644 tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php diff --git a/src/Module/Core/Domain/Entity/User.php b/src/Module/Core/Domain/Entity/User.php index b571488..a62dbd5 100644 --- a/src/Module/Core/Domain/Entity/User.php +++ b/src/Module/Core/Domain/Entity/User.php @@ -11,6 +11,7 @@ use ApiPlatform\Metadata\GetCollection; use ApiPlatform\Metadata\Patch; use ApiPlatform\Metadata\Post; use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserPasswordHasherProcessor; +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; use DateTimeImmutable; @@ -20,6 +21,7 @@ use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Serializer\Attribute\Groups; +use Symfony\Component\Serializer\Attribute\SerializedName; #[ApiResource( operations: [ @@ -36,6 +38,15 @@ use Symfony\Component\Serializer\Attribute\Groups; ), new Post(security: "is_granted('ROLE_ADMIN')", processor: UserPasswordHasherProcessor::class), new Patch(security: "is_granted('ROLE_ADMIN')", processor: UserPasswordHasherProcessor::class), + new Patch( + name: 'user_rbac_patch', + uriTemplate: '/users/{id}/rbac', + // TODO ticket #345 : remplacer par is_granted('core.users.manage') + security: "is_granted('ROLE_ADMIN')", + normalizationContext: ['groups' => ['user:list']], + denormalizationContext: ['groups' => ['user:rbac:write']], + processor: UserRbacProcessor::class, + ), new Delete(security: "is_granted('ROLE_ADMIN')"), ], denormalizationContext: ['groups' => ['user:write']], @@ -55,7 +66,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface private ?string $username = null; #[ORM\Column(name: 'is_admin', options: ['default' => false])] - #[Groups(['me:read', 'user:list'])] + #[Groups(['me:read', 'user:list', 'user:rbac:write'])] private bool $isAdmin = false; /** @@ -70,20 +81,25 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface */ #[ORM\ManyToMany(targetEntity: Role::class, fetch: 'EAGER')] #[ORM\JoinTable(name: 'user_role')] - #[Groups(['me:read', 'user:list'])] - private Collection $roles; + #[Groups(['me:read', 'user:list', 'user:rbac:write'])] + // La propriete s'appelle `rbacRoles` cote PHP pour ne pas entrer en + // collision avec UserInterface::getRoles() (qui renvoie list) ; + // on reexpose la cle JSON sous `roles` via SerializedName pour rester + // conforme au contrat API documente dans le ticket #344. + #[SerializedName('roles')] + private Collection $rbacRoles; /** * Les permissions directes accordees hors des roles. * - * Meme justification EAGER que pour $roles : garantie que + * Meme justification EAGER que pour $rbacRoles : garantie que * getEffectivePermissions() fonctionne dans tous les contextes de chargement. * * @var Collection */ #[ORM\ManyToMany(targetEntity: Permission::class, fetch: 'EAGER')] #[ORM\JoinTable(name: 'user_permission')] - #[Groups(['me:read', 'user:list'])] + #[Groups(['me:read', 'user:list', 'user:rbac:write'])] private Collection $directPermissions; #[ORM\Column] @@ -98,7 +114,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface public function __construct() { $this->createdAt = new DateTimeImmutable(); - $this->roles = new ArrayCollection(); + $this->rbacRoles = new ArrayCollection(); $this->directPermissions = new ArrayCollection(); } @@ -131,10 +147,10 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface * ROLE_ADMIN est ajoute si l'utilisateur porte le flag is_admin — c'est le * SEUL levier technique de bypass RBAC (cf. section 11 du spec). * - * Important : ne JAMAIS iterer $this->roles (la Collection de Role) ici. - * Cette methode peut etre appelee pendant un refresh JWT, moment ou la - * Collection peut ne pas etre hydratee. On se contente d'un calcul base - * sur un scalaire. + * Important : ne JAMAIS iterer $this->rbacRoles (la Collection de Role) + * ici. Cette methode peut etre appelee pendant un refresh JWT, moment ou + * la Collection peut ne pas etre hydratee. On se contente d'un calcul + * base sur un scalaire. * * @return list */ @@ -170,13 +186,13 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface */ public function getRbacRoles(): Collection { - return $this->roles; + return $this->rbacRoles; } public function addRbacRole(Role $role): static { - if (!$this->roles->contains($role)) { - $this->roles->add($role); + if (!$this->rbacRoles->contains($role)) { + $this->rbacRoles->add($role); } return $this; @@ -184,7 +200,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface public function removeRbacRole(Role $role): static { - $this->roles->removeElement($role); + $this->rbacRoles->removeElement($role); return $this; } @@ -229,7 +245,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface { $codes = []; - foreach ($this->roles as $role) { + foreach ($this->rbacRoles as $role) { foreach ($role->getPermissions() as $permission) { $codes[$permission->getCode()] = true; } diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php new file mode 100644 index 0000000..d97f846 --- /dev/null +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php @@ -0,0 +1,71 @@ + + */ +final class UserRbacProcessor implements ProcessorInterface +{ + public function __construct( + #[Autowire(service: 'api_platform.doctrine.orm.state.persist_processor')] + private readonly ProcessorInterface $persistProcessor, + private readonly EntityManagerInterface $entityManager, + private readonly Security $security, + ) {} + + public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed + { + if (!$data instanceof User) { + // Securite : si le provider n'a pas fourni un User, on delegue + // quand meme pour ne pas etouffer un bug de configuration. + return $this->persistProcessor->process($data, $operation, $uriVariables, $context); + } + + $currentUser = $this->security->getUser(); + + // Garde auto-suicide : l'user courant ne peut pas retirer son propre + // flag admin. On ne compare que si la cible == l'user courant. + if ($currentUser instanceof User + && null !== $currentUser->getId() + && $currentUser->getId() === $data->getId() + ) { + $originalData = $this->entityManager->getUnitOfWork()->getOriginalEntityData($data); + $wasAdmin = $originalData['isAdmin'] ?? null; + + if (true === $wasAdmin && false === $data->isAdmin()) { + throw new BadRequestHttpException( + 'Vous ne pouvez pas retirer vos propres droits administrateur.' + ); + } + } + + return $this->persistProcessor->process($data, $operation, $uriVariables, $context); + } +} diff --git a/tests/Module/Core/Api/UserRbacApiTest.php b/tests/Module/Core/Api/UserRbacApiTest.php new file mode 100644 index 0000000..9984e54 --- /dev/null +++ b/tests/Module/Core/Api/UserRbacApiTest.php @@ -0,0 +1,271 @@ +getEm(); + + $this->cleanupTestData(); + + /** @var UserPasswordHasherInterface $hasher */ + $hasher = self::getContainer()->get(UserPasswordHasherInterface::class); + + // User cible standard (non admin). + $target = new User(); + $target->setUsername('test_target'); + $target->setIsAdmin(false); + $target->setPassword($hasher->hashPassword($target, 'secret')); + $em->persist($target); + + // User admin dedie pour le cas d'auto-suicide (pas l'admin fixture). + $selfAdmin = new User(); + $selfAdmin->setUsername('test_self_admin'); + $selfAdmin->setIsAdmin(true); + $selfAdmin->setPassword($hasher->hashPassword($selfAdmin, 'secret')); + $em->persist($selfAdmin); + + // Role custom pour tester le remplacement de la collection roles. + $role = new Role('test_editor', 'Editeur (test)', false); + $em->persist($role); + + // Permission custom pour tester directPermissions. + $permission = new Permission('test.core.users.view', 'View users (test)', 'core'); + $em->persist($permission); + + $em->flush(); + $em->clear(); + } + + protected function tearDown(): void + { + $this->cleanupTestData(); + parent::tearDown(); + } + + public function testPatchRbacPromotesUserToAdmin(): void + { + $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']); + self::assertNotNull($target); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['isAdmin' => true], + ]); + + self::assertResponseIsSuccessful(); + + $em = $this->getEm(); + $em->clear(); + + /** @var User $reloaded */ + $reloaded = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']); + self::assertTrue($reloaded->isAdmin()); + } + + public function testPatchRbacReplacesRolesCollection(): void + { + $em = $this->getEm(); + $target = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']); + $role = $em->getRepository(Role::class)->findOneBy(['code' => 'test_editor']); + self::assertNotNull($target); + self::assertNotNull($role); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['roles' => ['/api/roles/'.$role->getId()]], + ]); + + self::assertResponseIsSuccessful(); + + $em = $this->getEm(); + $em->clear(); + + /** @var User $reloaded */ + $reloaded = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']); + self::assertCount(1, $reloaded->getRbacRoles()); + self::assertSame('test_editor', $reloaded->getRbacRoles()->first()->getCode()); + } + + public function testPatchRbacReplacesDirectPermissionsCollection(): void + { + $em = $this->getEm(); + $target = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']); + $permission = $em->getRepository(Permission::class)->findOneBy(['code' => 'test.core.users.view']); + self::assertNotNull($target); + self::assertNotNull($permission); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['directPermissions' => ['/api/permissions/'.$permission->getId()]], + ]); + + self::assertResponseIsSuccessful(); + + $em = $this->getEm(); + $em->clear(); + + /** @var User $reloaded */ + $reloaded = $em->getRepository(User::class)->findOneBy(['username' => 'test_target']); + self::assertCount(1, $reloaded->getDirectPermissions()); + self::assertSame('test.core.users.view', $reloaded->getDirectPermissions()->first()->getCode()); + } + + public function testPatchRbacAsStandardUserReturns403(): void + { + $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']); + self::assertNotNull($target); + + $client = $this->authenticatedClient('alice', 'alice'); + $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['isAdmin' => true], + ]); + + self::assertResponseStatusCodeSame(403); + } + + public function testPatchRbacUnauthenticatedReturns401(): void + { + $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']); + self::assertNotNull($target); + + $client = self::createClient(); + $client->request('PATCH', '/api/users/'.$target->getId().'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['isAdmin' => true], + ]); + + self::assertResponseStatusCodeSame(401); + } + + public function testPatchRbacIgnoresUsernameField(): void + { + $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']); + self::assertNotNull($target); + $targetId = $target->getId(); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$targetId.'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => [ + 'username' => 'test_target_renamed', + 'isAdmin' => true, + ], + ]); + + self::assertResponseIsSuccessful(); + + $em = $this->getEm(); + $em->clear(); + + /** @var User $reloaded */ + $reloaded = $em->getRepository(User::class)->find($targetId); + // `username` n'est pas dans `user:rbac:write` : ignore en denormalization. + self::assertSame('test_target', $reloaded->getUsername()); + // `isAdmin` est bien applique. + self::assertTrue($reloaded->isAdmin()); + } + + public function testPatchProfileEndpointDoesNotModifyIsAdmin(): void + { + // Confirme la decision 0fc4e16 : `isAdmin` n'est plus dans `user:write`, + // donc `PATCH /api/users/{id}` sans `/rbac` ne peut plus promouvoir. + $target = $this->getEm()->getRepository(User::class)->findOneBy(['username' => 'test_target']); + self::assertNotNull($target); + $targetId = $target->getId(); + self::assertFalse($target->isAdmin()); + + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('PATCH', '/api/users/'.$targetId, [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['isAdmin' => true], + ]); + + // Peu importe le code : le champ ne doit tout simplement pas bouger. + $em = $this->getEm(); + $em->clear(); + + /** @var User $reloaded */ + $reloaded = $em->getRepository(User::class)->find($targetId); + self::assertFalse($reloaded->isAdmin()); + } + + public function testPatchRbacSelfRemovingAdminReturns400(): void + { + // On utilise le user admin dedie (test_self_admin) pour ne pas + // corrompre l'admin fixture en cas de bug. + $em = $this->getEm(); + $selfAdmin = $em->getRepository(User::class)->findOneBy(['username' => 'test_self_admin']); + self::assertNotNull($selfAdmin); + $selfAdminId = $selfAdmin->getId(); + + $client = $this->authenticatedClient('test_self_admin', 'secret'); + $client->request('PATCH', '/api/users/'.$selfAdminId.'/rbac', [ + 'headers' => ['Content-Type' => 'application/merge-patch+json'], + 'json' => ['isAdmin' => false], + ]); + + self::assertResponseStatusCodeSame(400); + + $em = $this->getEm(); + $em->clear(); + + /** @var User $reloaded */ + $reloaded = $em->getRepository(User::class)->find($selfAdminId); + self::assertTrue($reloaded->isAdmin()); + } + + private function cleanupTestData(): void + { + $em = $this->getEm(); + + // Ordre important : delier les collections avant de supprimer les + // entites referencees pour que les FK cascade s'appliquent via le + // schema PostgreSQL. + $em->createQuery( + 'DELETE FROM '.User::class.' u WHERE u.username LIKE :prefix' + )->setParameter('prefix', self::TEST_USER_PREFIX.'%')->execute(); + + $em->createQuery( + 'DELETE FROM '.Role::class.' r WHERE r.code LIKE :prefix' + )->setParameter('prefix', self::TEST_ROLE_PREFIX.'%')->execute(); + + $em->createQuery( + 'DELETE FROM '.Permission::class.' p WHERE p.code LIKE :prefix' + )->setParameter('prefix', self::TEST_PERMISSION_PREFIX.'%')->execute(); + } +} diff --git a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php new file mode 100644 index 0000000..6dce00b --- /dev/null +++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php @@ -0,0 +1,216 @@ +persistProcessor = $this->createMock(ProcessorInterface::class); + $this->entityManager = $this->createMock(EntityManagerInterface::class); + $this->unitOfWork = $this->createMock(UnitOfWork::class); + $this->security = $this->createMock(Security::class); + + $this->entityManager->method('getUnitOfWork')->willReturn($this->unitOfWork); + + $this->processor = new UserRbacProcessor( + $this->persistProcessor, + $this->entityManager, + $this->security, + ); + } + + public function testPatchPromotesUserToAdminDelegatesToPersistProcessor(): void + { + $target = $this->buildUser(42, 'alice', false); + $target->setIsAdmin(true); + + $currentAdmin = $this->buildUser(1, 'admin', true); + $this->security->method('getUser')->willReturn($currentAdmin); + + // Cible != user courant : pas de lecture d'UnitOfWork necessaire. + $this->unitOfWork->expects(self::never())->method('getOriginalEntityData'); + + $this->persistProcessor + ->expects(self::once()) + ->method('process') + ->with($target) + ->willReturn($target) + ; + + $result = $this->processor->process($target, new Patch()); + + self::assertSame($target, $result); + } + + public function testPatchUpdatesRolesCollectionDelegatesToPersistProcessor(): void + { + $target = $this->buildUser(42, 'alice', false); + $target->addRbacRole(new Role('editor', 'Editor', false)); + + $currentAdmin = $this->buildUser(1, 'admin', true); + $this->security->method('getUser')->willReturn($currentAdmin); + + $this->persistProcessor + ->expects(self::once()) + ->method('process') + ->with($target) + ->willReturn($target) + ; + + $result = $this->processor->process($target, new Patch()); + + self::assertSame($target, $result); + self::assertCount(1, $result->getRbacRoles()); + } + + public function testPatchUpdatesDirectPermissionsCollectionDelegatesToPersistProcessor(): void + { + $target = $this->buildUser(42, 'alice', false); + $target->addDirectPermission(new Permission('core.users.view', 'View', 'core')); + + $currentAdmin = $this->buildUser(1, 'admin', true); + $this->security->method('getUser')->willReturn($currentAdmin); + + $this->persistProcessor + ->expects(self::once()) + ->method('process') + ->with($target) + ->willReturn($target) + ; + + $result = $this->processor->process($target, new Patch()); + + self::assertSame($target, $result); + self::assertCount(1, $result->getDirectPermissions()); + } + + public function testPatchSelfRemovingAdminThrowsBadRequestHttpException(): void + { + // Meme identifiant : l'user courant PATCH sa propre ressource. + $self = $this->buildUser(1, 'admin', false); + + $this->security->method('getUser')->willReturn($self); + + $this->unitOfWork + ->expects(self::once()) + ->method('getOriginalEntityData') + ->with($self) + ->willReturn([ + 'id' => 1, + 'username' => 'admin', + 'isAdmin' => true, + ]) + ; + + $this->persistProcessor->expects(self::never())->method('process'); + + $this->expectException(BadRequestHttpException::class); + $this->expectExceptionMessage('Vous ne pouvez pas retirer vos propres droits administrateur.'); + + $this->processor->process($self, new Patch()); + } + + public function testPatchAdminDemotingAnotherUserIsAllowed(): void + { + // Un admin qui retire isAdmin a quelqu'un d'autre : autorise. + $target = $this->buildUser(42, 'alice', false); + $current = $this->buildUser(1, 'admin', true); + + $this->security->method('getUser')->willReturn($current); + + // Cible != user courant : pas de verification d'auto-suicide. + $this->unitOfWork->expects(self::never())->method('getOriginalEntityData'); + + $this->persistProcessor + ->expects(self::once()) + ->method('process') + ->with($target) + ->willReturn($target) + ; + + $result = $this->processor->process($target, new Patch()); + + self::assertSame($target, $result); + } + + public function testPatchSelfKeepingAdminIsAllowed(): void + { + // L'user courant se PATCH lui-meme mais garde isAdmin = true : + // aucun auto-suicide, on delegue au PersistProcessor. + $self = $this->buildUser(1, 'admin', true); + + $this->security->method('getUser')->willReturn($self); + + $this->unitOfWork + ->expects(self::once()) + ->method('getOriginalEntityData') + ->with($self) + ->willReturn([ + 'id' => 1, + 'username' => 'admin', + 'isAdmin' => true, + ]) + ; + + $this->persistProcessor + ->expects(self::once()) + ->method('process') + ->with($self) + ->willReturn($self) + ; + + $result = $this->processor->process($self, new Patch()); + + self::assertSame($self, $result); + } + + /** + * Construit un User avec un id force via reflection (les mocks + * d'UnitOfWork n'alimentent pas l'id tout seul). + */ + private function buildUser(int $id, string $username, bool $isAdmin): User + { + $user = new User(); + $user->setUsername($username); + $user->setIsAdmin($isAdmin); + + $refl = new ReflectionClass($user); + $prop = $refl->getProperty('id'); + $prop->setAccessible(true); + $prop->setValue($user, $id); + + return $user; + } +} -- 2.39.5 From 534bdbccdd92c8096a191f305f268e2cc2a11ac5 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 14:28:02 +0200 Subject: [PATCH 10/11] refactor(core) : RBAC #344 - polish review - narrow rbac read group + fail-fast processors --- src/Module/Core/Domain/Entity/User.php | 12 +++++++----- .../ApiPlatform/State/Processor/RoleProcessor.php | 15 +++++++++------ .../State/Processor/UserRbacProcessor.php | 12 +++++++++--- .../State/Processor/RoleProcessorTest.php | 15 +++++++++++++++ .../State/Processor/UserRbacProcessorTest.php | 14 ++++++++++++++ 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/src/Module/Core/Domain/Entity/User.php b/src/Module/Core/Domain/Entity/User.php index a62dbd5..0426121 100644 --- a/src/Module/Core/Domain/Entity/User.php +++ b/src/Module/Core/Domain/Entity/User.php @@ -43,7 +43,7 @@ use Symfony\Component\Serializer\Attribute\SerializedName; uriTemplate: '/users/{id}/rbac', // TODO ticket #345 : remplacer par is_granted('core.users.manage') security: "is_granted('ROLE_ADMIN')", - normalizationContext: ['groups' => ['user:list']], + normalizationContext: ['groups' => ['user:rbac:read']], denormalizationContext: ['groups' => ['user:rbac:write']], processor: UserRbacProcessor::class, ), @@ -58,7 +58,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - #[Groups(['me:read', 'user:list'])] + #[Groups(['me:read', 'user:list', 'user:rbac:read'])] private ?int $id = null; #[ORM\Column(length: 180, unique: true)] @@ -66,7 +66,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface private ?string $username = null; #[ORM\Column(name: 'is_admin', options: ['default' => false])] - #[Groups(['me:read', 'user:list', 'user:rbac:write'])] + #[Groups(['me:read', 'user:list', 'user:rbac:write', 'user:rbac:read'])] private bool $isAdmin = false; /** @@ -81,7 +81,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface */ #[ORM\ManyToMany(targetEntity: Role::class, fetch: 'EAGER')] #[ORM\JoinTable(name: 'user_role')] - #[Groups(['me:read', 'user:list', 'user:rbac:write'])] + #[Groups(['me:read', 'user:list', 'user:rbac:write', 'user:rbac:read'])] // La propriete s'appelle `rbacRoles` cote PHP pour ne pas entrer en // collision avec UserInterface::getRoles() (qui renvoie list) ; // on reexpose la cle JSON sous `roles` via SerializedName pour rester @@ -99,7 +99,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface */ #[ORM\ManyToMany(targetEntity: Permission::class, fetch: 'EAGER')] #[ORM\JoinTable(name: 'user_permission')] - #[Groups(['me:read', 'user:list', 'user:rbac:write'])] + #[Groups(['me:read', 'user:list', 'user:rbac:write', 'user:rbac:read'])] private Collection $directPermissions; #[ORM\Column] @@ -152,6 +152,8 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface * la Collection peut ne pas etre hydratee. On se contente d'un calcul * base sur un scalaire. * + * @see getRbacRoles() pour la collection RBAC metier (exposee en JSON sous la cle "roles"). + * * @return list */ public function getRoles(): array diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php index 70a8ba3..cb1be0d 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php @@ -10,6 +10,7 @@ use ApiPlatform\State\ProcessorInterface; use App\Module\Core\Domain\Entity\Role; use App\Module\Core\Domain\Exception\SystemRoleDeletionException; use Doctrine\ORM\EntityManagerInterface; +use LogicException; use Symfony\Component\DependencyInjection\Attribute\Autowire; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -44,12 +45,14 @@ final class RoleProcessor implements ProcessorInterface public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed { if (!$data instanceof Role) { - // Securite : si le provider n'a pas fourni un Role, on delegue - // quand meme au processor approprie pour ne pas etouffer - // silencieusement un bug de configuration. - return $operation instanceof DeleteOperationInterface - ? $this->removeProcessor->process($data, $operation, $uriVariables, $context) - : $this->persistProcessor->process($data, $operation, $uriVariables, $context); + // Ce processor est wire exclusivement sur les operations Role. + // Si on arrive ici avec autre chose, c'est une misconfiguration + // qu'il faut faire remonter fort. + throw new LogicException(sprintf( + 'RoleProcessor attend une instance de %s, %s recu.', + Role::class, + get_debug_type($data), + )); } if ($operation instanceof DeleteOperationInterface) { diff --git a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php index d97f846..4215d74 100644 --- a/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php @@ -8,6 +8,7 @@ use ApiPlatform\Metadata\Operation; use ApiPlatform\State\ProcessorInterface; use App\Module\Core\Domain\Entity\User; use Doctrine\ORM\EntityManagerInterface; +use LogicException; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\DependencyInjection\Attribute\Autowire; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -43,9 +44,14 @@ final class UserRbacProcessor implements ProcessorInterface public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed { if (!$data instanceof User) { - // Securite : si le provider n'a pas fourni un User, on delegue - // quand meme pour ne pas etouffer un bug de configuration. - return $this->persistProcessor->process($data, $operation, $uriVariables, $context); + // Ce processor est wire exclusivement sur l'operation user_rbac_patch + // qui cible User. Si on arrive ici avec autre chose, c'est une + // misconfiguration qu'il faut faire remonter fort. + throw new LogicException(sprintf( + 'UserRbacProcessor attend une instance de %s, %s recu.', + User::class, + get_debug_type($data), + )); } $currentUser = $this->security->getUser(); diff --git a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php index c90fe5a..d124e91 100644 --- a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php +++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php @@ -12,10 +12,12 @@ use App\Module\Core\Domain\Entity\Role; use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\RoleProcessor; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\UnitOfWork; +use LogicException; use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use ReflectionClass; +use stdClass; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -199,6 +201,19 @@ final class RoleProcessorTest extends TestCase self::assertSame($role, $result); } + public function testProcessNonRoleDataThrowsLogicException(): void + { + // Garde-fou contre une misconfiguration : ce processor est wire + // exclusivement sur les operations Role. + $this->persistProcessor->expects(self::never())->method('process'); + $this->removeProcessor->expects(self::never())->method('process'); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage('RoleProcessor attend une instance de'); + + $this->processor->process(new stdClass(), new Patch()); + } + /** * Positionne l'id d'un Role via reflection pour simuler une entite deja * persistee (les mocks d'UnitOfWork n'alimentent pas l'id tout seul). diff --git a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php index 6dce00b..e30dbcb 100644 --- a/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php +++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php @@ -12,10 +12,12 @@ use App\Module\Core\Domain\Entity\User; use App\Module\Core\Infrastructure\ApiPlatform\State\Processor\UserRbacProcessor; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\UnitOfWork; +use LogicException; use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use ReflectionClass; +use stdClass; use Symfony\Bundle\SecurityBundle\Security; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -196,6 +198,18 @@ final class UserRbacProcessorTest extends TestCase self::assertSame($self, $result); } + public function testProcessNonUserDataThrowsLogicException(): void + { + // Garde-fou contre une misconfiguration : ce processor est wire + // exclusivement sur l'operation user_rbac_patch (cible User). + $this->persistProcessor->expects(self::never())->method('process'); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage('UserRbacProcessor attend une instance de'); + + $this->processor->process(new stdClass(), new Patch()); + } + /** * Construit un User avec un id force via reflection (les mocks * d'UnitOfWork n'alimentent pas l'id tout seul). -- 2.39.5 From 0ccbc70f27d39757034fa17a84a6458034d719b7 Mon Sep 17 00:00:00 2001 From: Matthieu Date: Wed, 15 Apr 2026 14:53:49 +0200 Subject: [PATCH 11/11] fix(core) : RBAC #344 - ferme leak user list + test cascade delete role --- src/Module/Core/Domain/Entity/User.php | 2 + tests/Module/Core/Api/RoleApiTest.php | 51 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/Module/Core/Domain/Entity/User.php b/src/Module/Core/Domain/Entity/User.php index 0426121..96f52a0 100644 --- a/src/Module/Core/Domain/Entity/User.php +++ b/src/Module/Core/Domain/Entity/User.php @@ -31,9 +31,11 @@ use Symfony\Component\Serializer\Attribute\SerializedName; normalizationContext: ['groups' => ['me:read']], ), new Get( + security: "is_granted('ROLE_ADMIN')", // TODO ticket #345 : remplacer par is_granted('core.users.view') normalizationContext: ['groups' => ['user:list']], ), new GetCollection( + security: "is_granted('ROLE_ADMIN')", // TODO ticket #345 : remplacer par is_granted('core.users.view') normalizationContext: ['groups' => ['user:list']], ), new Post(security: "is_granted('ROLE_ADMIN')", processor: UserPasswordHasherProcessor::class), diff --git a/tests/Module/Core/Api/RoleApiTest.php b/tests/Module/Core/Api/RoleApiTest.php index 0fde030..205ad94 100644 --- a/tests/Module/Core/Api/RoleApiTest.php +++ b/tests/Module/Core/Api/RoleApiTest.php @@ -6,6 +6,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\Core\Domain\Security\SystemRoles; /** @@ -238,6 +239,48 @@ final class RoleApiTest extends AbstractApiTestCase self::assertNull($em->getRepository(Role::class)->find($id)); } + public function testDeleteCustomRoleAttachedToUserDoesNotDeleteUser(): void + { + // Scenario spec #344 sections 7 & 11 : supprimer un role custom rattache + // a un user doit laisser le user en base (la FK user_role est nettoyee + // par ON DELETE CASCADE, mais jamais le user lui-meme). + $em = $this->getEm(); + + // Creer un user de test dedie et lui rattacher le role custom `test_editor`. + $testUser = new User(); + $testUser->setUsername('test_cascade_user'); + // Le hashage du password est hors scope du test mais la colonne est NOT NULL. + $testUser->setPassword('not-hashed-ok-for-test'); + + /** @var Role $editor */ + $editor = $em->getRepository(Role::class)->findOneBy(['code' => 'test_editor']); + self::assertNotNull($editor); + $testUser->addRbacRole($editor); + + $em->persist($testUser); + $em->flush(); + $userId = $testUser->getId(); + $editorId = $editor->getId(); + $em->clear(); + + // DELETE du role editor via l'API. + $client = $this->authenticatedClient('admin', 'admin'); + $client->request('DELETE', '/api/roles/'.$editorId); + self::assertResponseStatusCodeSame(204); + + // Verification : l'user existe toujours et sa collection de roles est vide. + $em = $this->getEm(); + + /** @var null|User $refreshed */ + $refreshed = $em->getRepository(User::class)->find($userId); + self::assertNotNull($refreshed, 'L\'user ne doit PAS etre supprime par le cascade.'); + self::assertCount(0, $refreshed->getRbacRoles(), 'La relation user_role doit etre nettoyee par le cascade.'); + + // Cleanup explicite : cleanupTestData() ne purge pas les users. + $em->remove($refreshed); + $em->flush(); + } + public function testDeleteSystemRoleReturns403(): void { $role = $this->getEm()->getRepository(Role::class)->findOneBy(['code' => SystemRoles::ADMIN_CODE]); @@ -340,6 +383,14 @@ final class RoleApiTest extends AbstractApiTestCase // bulk delete : le cascade est applique au niveau FK par PostgreSQL, // pas par l'Unit of Work Doctrine. Verifie par comptage avant/apres // runs successifs de la suite (stable a la ligne de base systeme). + // Purge defensive des users de test crees par certains scenarios + // (ex: testDeleteCustomRoleAttachedToUserDoesNotDeleteUser). Doit etre + // fait AVANT la suppression des roles pour que le cascade FK ne soit + // pas sollicite en ordre inverse. + $em->createQuery( + 'DELETE FROM '.User::class.' u WHERE u.username LIKE :prefix' + )->setParameter('prefix', self::TEST_ROLE_PREFIX.'%')->execute(); + $em->createQuery( 'DELETE FROM '.Role::class.' r WHERE r.code LIKE :prefix' )->setParameter('prefix', self::TEST_ROLE_PREFIX.'%')->execute(); -- 2.39.5