docs(rbac) : spec ticket #344 - API CRUD roles & permissions

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.