Coltura/docs/rbac/ticket-344-spec.md

# 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.