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..fc3d6a9 100644 --- a/config/packages/api_platform.yaml +++ b/config/packages/api_platform.yaml @@ -1,6 +1,14 @@ api_platform: title: Coltura API version: 1.0.0 + # 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). + mapping: + paths: + - '%kernel.project_dir%/src/Module/Core/Domain/Entity' formats: jsonld: ['application/ld+json'] json: ['application/json'] 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. 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/src/Module/Core/Domain/Entity/Role.php b/src/Module/Core/Domain/Entity/Role.php index 7583454..ae98b38 100644 --- a/src/Module/Core/Domain/Entity/Role.php +++ b/src/Module/Core/Domain/Entity/Role.php @@ -4,12 +4,25 @@ 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\ApiPlatform\State\Processor\RoleProcessor; 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 +31,75 @@ 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')", + 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']], + 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 +114,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 +146,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; @@ -95,6 +163,23 @@ 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. + * + * @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 + { + $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/Domain/Entity/User.php b/src/Module/Core/Domain/Entity/User.php index b571488..96f52a0 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: [ @@ -29,13 +31,24 @@ use Symfony\Component\Serializer\Attribute\Groups; 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), 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:rbac:read']], + denormalizationContext: ['groups' => ['user:rbac:write']], + processor: UserRbacProcessor::class, + ), new Delete(security: "is_granted('ROLE_ADMIN')"), ], denormalizationContext: ['groups' => ['user:write']], @@ -47,7 +60,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)] @@ -55,7 +68,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', 'user:rbac:read'])] private bool $isAdmin = false; /** @@ -70,20 +83,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', '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 + // 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', 'user:rbac:read'])] private Collection $directPermissions; #[ORM\Column] @@ -98,7 +116,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 +149,12 @@ 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. + * + * @see getRbacRoles() pour la collection RBAC metier (exposee en JSON sous la cle "roles"). * * @return list */ @@ -170,13 +190,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 +204,7 @@ class User implements UserInterface, PasswordAuthenticatedUserInterface public function removeRbacRole(Role $role): static { - $this->roles->removeElement($role); + $this->rbacRoles->removeElement($role); return $this; } @@ -229,7 +249,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/RoleProcessor.php b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php new file mode 100644 index 0000000..cb1be0d --- /dev/null +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessor.php @@ -0,0 +1,81 @@ + + */ +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) { + // 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) { + 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/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..4215d74 --- /dev/null +++ b/src/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessor.php @@ -0,0 +1,77 @@ + + */ +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) { + // 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(); + + // 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/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 new file mode 100644 index 0000000..f097b95 --- /dev/null +++ b/tests/Module/Core/Api/PermissionApiTest.php @@ -0,0 +1,181 @@ +getEm(); + + // 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(); + + $em->persist($p1); + $em->persist($p2); + $em->persist($p3); + $em->flush(); + $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(); + // 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']); + } + + 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 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'); + $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->getEm()->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->getEm()->createQuery( + 'DELETE FROM '.Permission::class.' p WHERE p.code LIKE :prefix' + )->setParameter('prefix', self::TEST_CODE_PREFIX.'%')->execute(); + } +} diff --git a/tests/Module/Core/Api/RoleApiTest.php b/tests/Module/Core/Api/RoleApiTest.php new file mode 100644 index 0000000..205ad94 --- /dev/null +++ b/tests/Module/Core/Api/RoleApiTest.php @@ -0,0 +1,402 @@ +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' => SystemRoles::ADMIN_CODE, + '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(4, $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 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]); + 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(); + $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); + } + + /** + * 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(); + + // 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). + // 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(); + + $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/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/RoleProcessorTest.php b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php new file mode 100644 index 0000000..d124e91 --- /dev/null +++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/RoleProcessorTest.php @@ -0,0 +1,228 @@ +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->expectExceptionMessage('Le role systeme "admin" ne peut pas etre supprime.'); + + $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); + } + + 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). + */ + private function setRoleId(Role $role, int $id): void + { + $refl = new ReflectionClass($role); + $prop = $refl->getProperty('id'); + $prop->setAccessible(true); + $prop->setValue($role, $id); + } +} 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..e30dbcb --- /dev/null +++ b/tests/Module/Core/Infrastructure/ApiPlatform/State/Processor/UserRbacProcessorTest.php @@ -0,0 +1,230 @@ +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); + } + + 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). + */ + 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; + } +}