[ERP-74] Seed RBAC idempotent (rôles + matrice § 2.7 + demo users) + RG-1.04 + test matrice (#40)
Auto Tag Develop / tag (push) Successful in 11s
Auto Tag Develop / tag (push) Successful in 11s
## Objectif
Seeder le RBAC métier de façon **rejouable et disponible en recette/prod** (commande applicative, pas fixture `require-dev`), durcir RG-1.04, et écrire le test de matrice (rôles enfin existants).
## A. `RbacSeeder` (Core) — source unique anti-drift
4 rôles (`bureau`/`compta`/`commerciale`/`usine`, isSystem=false), matrice § 2.7 (rôle → permissions) et comptes démo, définis en **un seul endroit**. Méthodes idempotentes `ensureRoles` / `attachMatrix` / `ensureDemoUsers`. `commerciale` référence `BusinessRoles::COMMERCIALE` (déjà consommé par RG-1.04).
## B. Commande `app:seed-rbac` (présente en build prod, idempotente, non destructive)
- Sans option : rôles + matrice § 2.7.
- `--with-demo-users` + `--password=<…>` ou env `RBAC_DEMO_PASSWORD` : 1 compte démo/rôle. **Aucun mot de passe en dur** côté serveur.
- Garde-fou : exit non-zéro + invite à lancer `app:sync-permissions` si les codes `commercial.clients.*` manquent.
## C. Fixture dev/test `RbacDemoFixtures` (DRY)
Appelle le **même seeder** (`ensureRoles` + `ensureDemoUsers`). La matrice est attachée juste après par l'étape `app:seed-rbac` du makefile (la table `permission` est purgée au moment du `fixtures:load`, donc `attachMatrix` ne peut pas tourner pendant le load). `make db-reset` / `test-db-setup` reproduisent l'état de recette.
## Déploiement (documenté README)
Après `migration-migrate` + `app:sync-permissions` : `app:seed-rbac` (prod) ; `app:seed-rbac --with-demo-users --password=…` (recette).
## D. Durcissement RG-1.04
Pour une Commerciale, complétude de l'onglet Information exigée sur **POST + tout PATCH** (suppression de la condition d'intersection). Conséquence : POST Commerciale → 422 (le POST n'expose pas le groupe Information), Admin → 201. Spec § 7 amendée.
## Compta ↔ onglet Comptabilité (§ 2.7)
Pour que `compta PATCH accounting → 200` (exigé par la matrice), la security du `Patch /clients/{id}` est élargie à `manage` **OU** `accounting.manage`, et un nouveau **`guardManage`** (mode strict RG-1.28) interdit à un porteur non-`manage` de modifier les onglets principal/Information (→ 403). Approche validée : élargir la security + guard in-processor (pas de nouvel endpoint).
## E. `ClientRBACMatrixTest`
Matrice § 2.7 complète via les comptes démo seedés (`app:seed-rbac --with-demo-users`) : bureau / compta / commerciale / usine (200/403 par verbe et par onglet) + RG-1.04 (POST Commerciale 422 / Admin 201).
## Tests
`make php-cs-fixer-allow-risky` OK ; `make test` **429 tests verts**. Idempotence vérifiée (rejeu de la commande : 0 rôle / 0 lien / 0 user). `test-db-setup` exécute la nouvelle étape `app:seed-rbac` sans erreur.
Cible : `develop`. Squash merge.
---------
Co-authored-by: Matthieu <contact@malio.fr>
Reviewed-on: #40
Co-authored-by: THOLOT DECHENE Matthieu <matthieu@yuno.malio.fr>
Co-committed-by: THOLOT DECHENE Matthieu <matthieu@yuno.malio.fr>
This commit was merged in pull request #40.
This commit is contained in:
@@ -0,0 +1,299 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace App\Tests\Module\Commercial\Api;
|
||||
|
||||
use ApiPlatform\Symfony\Bundle\Test\Client;
|
||||
use App\Module\Core\Infrastructure\DataFixtures\RbacDemoFixtures;
|
||||
use Symfony\Bundle\FrameworkBundle\Console\Application;
|
||||
use Symfony\Component\Console\Input\ArrayInput;
|
||||
use Symfony\Component\Console\Output\NullOutput;
|
||||
|
||||
/**
|
||||
* Matrice RBAC complete du repertoire clients par role metier (spec-back M1
|
||||
* § 2.7 + cahier ERP-74). Valide 200/403 par verbe et par onglet pour
|
||||
* bureau / compta / commerciale / usine, plus le durcissement RG-1.04
|
||||
* (Commerciale) au POST.
|
||||
*
|
||||
* Les comptes demo et la matrice sont seedes via la commande reelle
|
||||
* `app:seed-rbac --with-demo-users` (le MEME chemin qu'en recette), idempotente.
|
||||
* Pre-requis du run : `app:sync-permissions` a tourne (cf. make test-db-setup).
|
||||
*
|
||||
* @internal
|
||||
*/
|
||||
final class ClientRBACMatrixTest extends AbstractCommercialApiTestCase
|
||||
{
|
||||
private const string LD = 'application/ld+json';
|
||||
private const string MERGE = 'application/merge-patch+json';
|
||||
private const string PWD = RbacDemoFixtures::DEMO_PASSWORD;
|
||||
|
||||
protected function setUp(): void
|
||||
{
|
||||
parent::setUp();
|
||||
|
||||
// Seed idempotent via la commande applicative (roles + matrice § 2.7 +
|
||||
// comptes demo). Exerce aussi le chemin de code prod.
|
||||
self::bootKernel();
|
||||
$application = new Application(self::$kernel);
|
||||
$application->setAutoExit(false);
|
||||
$exit = $application->run(
|
||||
new ArrayInput([
|
||||
'command' => 'app:seed-rbac',
|
||||
'--with-demo-users' => true,
|
||||
'--password' => self::PWD,
|
||||
]),
|
||||
new NullOutput(),
|
||||
);
|
||||
self::assertSame(
|
||||
0,
|
||||
$exit,
|
||||
'app:seed-rbac a echoue : les permissions commercial.clients.* sont-elles synchronisees (app:sync-permissions) ?',
|
||||
);
|
||||
|
||||
// Liberer le kernel pour que authenticatedClient()/createClient() reparte propre.
|
||||
self::ensureKernelShutdown();
|
||||
}
|
||||
|
||||
public function testUsineIsForbiddenEverywhere(): void
|
||||
{
|
||||
$seed = $this->seedClient('Usine Target');
|
||||
$client = $this->authAs('usine');
|
||||
|
||||
// Aucune permission : 403 sur tous les verbes.
|
||||
$client->request('GET', '/api/clients', ['headers' => ['Accept' => self::LD]]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
|
||||
$client->request('GET', '/api/clients/'.$seed->getId(), ['headers' => ['Accept' => self::LD]]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
|
||||
$client->request('POST', '/api/clients', [
|
||||
'headers' => ['Content-Type' => self::LD],
|
||||
'json' => $this->validMainPayload('Usine Post'),
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['companyName' => 'Renamed By Usine'],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
}
|
||||
|
||||
public function testBureauHasViewAndManageButNoAccountingNoArchive(): void
|
||||
{
|
||||
$seed = $this->seedClient('Bureau Target');
|
||||
$cat = $this->createCategory('SECTEUR');
|
||||
$client = $this->authAs('bureau');
|
||||
|
||||
// view
|
||||
$client->request('GET', '/api/clients', ['headers' => ['Accept' => self::LD]]);
|
||||
self::assertResponseStatusCodeSame(200);
|
||||
|
||||
// manage : creation OK
|
||||
$client->request('POST', '/api/clients', [
|
||||
'headers' => ['Content-Type' => self::LD],
|
||||
'json' => $this->validMainPayload('Bureau Created', $cat->getId()),
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(201);
|
||||
|
||||
// manage : edition onglet principal OK
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['companyName' => 'Bureau Renamed'],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(200);
|
||||
|
||||
// PAS accounting : edition onglet Comptabilite refusee
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['siren' => '123456789'],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
|
||||
// PAS archive : archivage refuse
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['isArchived' => true],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
}
|
||||
|
||||
public function testComptaCanEditAccountingOnly(): void
|
||||
{
|
||||
$seed = $this->seedClient('Compta Target');
|
||||
$client = $this->authAs('compta');
|
||||
|
||||
// view
|
||||
$client->request('GET', '/api/clients', ['headers' => ['Accept' => self::LD]]);
|
||||
self::assertResponseStatusCodeSame(200);
|
||||
|
||||
// PAS manage : creation refusee
|
||||
$client->request('POST', '/api/clients', [
|
||||
'headers' => ['Content-Type' => self::LD],
|
||||
'json' => $this->validMainPayload('Compta Post'),
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
|
||||
// accounting.manage : edition onglet Comptabilite OK
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['siren' => '123456789'],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(200);
|
||||
|
||||
// PAS manage : edition onglet principal refusee (guardManage)
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['companyName' => 'Compta Renamed'],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
|
||||
// PAS manage : edition onglet Information refusee (guardManage)
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['description' => 'Une description'],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
|
||||
// PAS archive : archivage refuse
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['isArchived' => true],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
}
|
||||
|
||||
public function testCommercialeHasViewAndManageButNoAccountingNoArchive(): void
|
||||
{
|
||||
$seed = $this->seedClient('Commerciale Target');
|
||||
$client = $this->authAs('commerciale');
|
||||
|
||||
// view
|
||||
$client->request('GET', '/api/clients', ['headers' => ['Accept' => self::LD]]);
|
||||
self::assertResponseStatusCodeSame(200);
|
||||
|
||||
// manage : la creation passe la security d'operation (pas un 403 comme
|
||||
// Compta) mais bute sur RG-1.04 (onglet Information incomplet) -> 422.
|
||||
// C'est la preuve que Commerciale porte `manage` (sinon 403).
|
||||
$client->request('POST', '/api/clients', [
|
||||
'headers' => ['Content-Type' => self::LD],
|
||||
'json' => $this->validMainPayload('Commerciale Post'),
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(422);
|
||||
|
||||
// PAS accounting : edition onglet Comptabilite refusee
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['siren' => '123456789'],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
|
||||
// PAS archive : archivage refuse
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['isArchived' => true],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
}
|
||||
|
||||
public function testRG104CommercialePostIncompleteIs422AdminIs201(): void
|
||||
{
|
||||
$cat = $this->createCategory('SECTEUR');
|
||||
|
||||
// RG-1.04 durcie : Commerciale POST sans onglet Information complet -> 422.
|
||||
$commerciale = $this->authAs('commerciale');
|
||||
$commerciale->request('POST', '/api/clients', [
|
||||
'headers' => ['Content-Type' => self::LD],
|
||||
'json' => $this->validMainPayload('RG104 Commerciale', $cat->getId()),
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(422);
|
||||
|
||||
// Meme payload par un Admin (non gate par RG-1.04) -> 201.
|
||||
$admin = $this->createAdminClient();
|
||||
$admin->request('POST', '/api/clients', [
|
||||
'headers' => ['Content-Type' => self::LD],
|
||||
'json' => $this->validMainPayload('RG104 Admin', $cat->getId()),
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(201);
|
||||
}
|
||||
|
||||
public function testComptaFullRepresentationPatchWithUnchangedCategoriesIsNotForbidden(): void
|
||||
{
|
||||
// FIX review MR #40 : un Compta (accounting.manage, PAS manage) faisant un
|
||||
// PATCH representation complete de l'onglet Comptabilite et reincluant ses
|
||||
// categories INCHANGEES ne doit PAS prendre de 403. guardManage compare
|
||||
// desormais les categories par valeur (et non par simple presence) : seul
|
||||
// l'onglet Comptabilite change ici -> 200.
|
||||
$seed = $this->seedClient('Compta Cat Unchanged');
|
||||
$category = $seed->getCategories()->first();
|
||||
self::assertNotFalse($category);
|
||||
$catId = $category->getId();
|
||||
$client = $this->authAs('compta');
|
||||
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => [
|
||||
'siren' => '123456789',
|
||||
'categories' => ['/api/categories/'.$catId],
|
||||
],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(200);
|
||||
}
|
||||
|
||||
public function testComptaChangingCategoriesIsForbidden(): void
|
||||
{
|
||||
// Non-regression : si le Compta change REELLEMENT l'ensemble des
|
||||
// categories (sans manage) -> 403 via guardManage. La comparaison par
|
||||
// valeur detecte bien le changement.
|
||||
$seed = $this->seedClient('Compta Cat Change');
|
||||
$newCat = $this->createCategory('SECTEUR');
|
||||
$client = $this->authAs('compta');
|
||||
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['categories' => ['/api/categories/'.$newCat->getId()]],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(403);
|
||||
}
|
||||
|
||||
public function testBureauChangingCategoriesIsAllowed(): void
|
||||
{
|
||||
// Non-regression : un role porteur de `manage` (Bureau) peut changer les
|
||||
// categories -> 200.
|
||||
$seed = $this->seedClient('Bureau Cat Change');
|
||||
$newCat = $this->createCategory('SECTEUR');
|
||||
$client = $this->authAs('bureau');
|
||||
|
||||
$client->request('PATCH', '/api/clients/'.$seed->getId(), [
|
||||
'headers' => ['Content-Type' => self::MERGE],
|
||||
'json' => ['categories' => ['/api/categories/'.$newCat->getId()]],
|
||||
]);
|
||||
self::assertResponseStatusCodeSame(200);
|
||||
}
|
||||
|
||||
private function authAs(string $role): Client
|
||||
{
|
||||
return $this->authenticatedClient($role, self::PWD);
|
||||
}
|
||||
|
||||
/**
|
||||
* Payload minimal valide de l'onglet principal (RG-1.01 : un nom de contact ;
|
||||
* une categorie SECTEUR). Si $categoryId est null, une categorie est creee a
|
||||
* la volee.
|
||||
*
|
||||
* @return array<string, mixed>
|
||||
*/
|
||||
private function validMainPayload(string $companyName, ?int $categoryId = null): array
|
||||
{
|
||||
$categoryId ??= $this->createCategory('SECTEUR')->getId();
|
||||
|
||||
return [
|
||||
'companyName' => $companyName,
|
||||
'firstName' => 'Jean',
|
||||
'phonePrimary' => '0612345678',
|
||||
'email' => strtolower(str_replace(' ', '', $companyName)).'@matrix.test',
|
||||
'categories' => ['/api/categories/'.$categoryId],
|
||||
];
|
||||
}
|
||||
}
|
||||
@@ -134,7 +134,17 @@ final class ClientProcessorTest extends TestCase
|
||||
'isArchived' => false,
|
||||
],
|
||||
managed: true,
|
||||
originalData: ['isArchived' => false],
|
||||
// Etat persiste complet (valeurs normalisees) : sans les champs
|
||||
// metier, guardManage (ERP-74) les croirait modifies (companyName,
|
||||
// lastName... compares a null) et leverait un 403 parasite.
|
||||
originalData: [
|
||||
'companyName' => 'TEST CO',
|
||||
'lastName' => 'Dupont',
|
||||
'phonePrimary' => '0102030405',
|
||||
'email' => 't@test.fr',
|
||||
'triageService' => false,
|
||||
'isArchived' => false,
|
||||
],
|
||||
);
|
||||
|
||||
self::assertInstanceOf(Client::class, $processor->process($client, $this->operation()));
|
||||
@@ -153,8 +163,69 @@ final class ClientProcessorTest extends TestCase
|
||||
payload: ['companyName' => 'Test Co', 'siren' => '123456789'],
|
||||
managed: true,
|
||||
// getOriginalEntityData renvoie tous les champs mappes d'une entite
|
||||
// geree : isArchived (non-null) y figure toujours.
|
||||
originalData: ['siren' => '123456789', 'isArchived' => false],
|
||||
// geree : isArchived (non-null) y figure toujours, ainsi que les
|
||||
// champs metier (sinon guardManage les croirait modifies).
|
||||
originalData: [
|
||||
'siren' => '123456789',
|
||||
'companyName' => 'TEST CO',
|
||||
'lastName' => 'Dupont',
|
||||
'phonePrimary' => '0102030405',
|
||||
'email' => 't@test.fr',
|
||||
'triageService' => false,
|
||||
'isArchived' => false,
|
||||
],
|
||||
);
|
||||
|
||||
self::assertInstanceOf(Client::class, $processor->process($client, $this->operation()));
|
||||
}
|
||||
|
||||
public function testBusinessFieldWithoutManagePermissionIsForbidden(): void
|
||||
{
|
||||
// ERP-74 (guardManage) : modifier un champ metier (companyName) sur un
|
||||
// client existant sans `manage` -> 403, meme avec accounting.manage
|
||||
// (cas Compta qui sort de son onglet).
|
||||
$client = $this->minimalClient();
|
||||
$client->setCompanyName('Renamed Co');
|
||||
|
||||
$processor = $this->makeProcessor(
|
||||
granted: ['commercial.clients.accounting.manage'],
|
||||
payload: ['companyName' => 'Renamed Co'],
|
||||
managed: true,
|
||||
originalData: [
|
||||
'companyName' => 'TEST CO',
|
||||
'lastName' => 'Dupont',
|
||||
'phonePrimary' => '0102030405',
|
||||
'email' => 't@test.fr',
|
||||
'triageService' => false,
|
||||
'isArchived' => false,
|
||||
],
|
||||
);
|
||||
|
||||
$this->expectException(AccessDeniedHttpException::class);
|
||||
$processor->process($client, $this->operation());
|
||||
}
|
||||
|
||||
public function testAccountingOnlyPatchWithAccountingManageOnlyPasses(): void
|
||||
{
|
||||
// ERP-74 : Compta (accounting.manage, PAS manage) qui ne touche QUE
|
||||
// l'onglet Comptabilite d'un client existant -> 200. guardManage ne
|
||||
// declenche pas (aucun champ metier modifie), guardAccounting passe.
|
||||
$client = $this->minimalClient();
|
||||
$client->setSiren('999999999');
|
||||
|
||||
$processor = $this->makeProcessor(
|
||||
granted: ['commercial.clients.accounting.manage'],
|
||||
payload: ['siren' => '999999999'],
|
||||
managed: true,
|
||||
originalData: [
|
||||
'siren' => '111111111',
|
||||
'companyName' => 'TEST CO',
|
||||
'lastName' => 'Dupont',
|
||||
'phonePrimary' => '0102030405',
|
||||
'email' => 't@test.fr',
|
||||
'triageService' => false,
|
||||
'isArchived' => false,
|
||||
],
|
||||
);
|
||||
|
||||
self::assertInstanceOf(Client::class, $processor->process($client, $this->operation()));
|
||||
@@ -237,6 +308,34 @@ final class ClientProcessorTest extends TestCase
|
||||
$processor->process($client, $this->operation());
|
||||
}
|
||||
|
||||
public function testCommercialeIncompleteInformationOnNonInformationPatchIsUnprocessable(): void
|
||||
{
|
||||
// RG-1.04 durcie (ERP-74) : pour une Commerciale, la completude de
|
||||
// l'onglet Information est exigee meme quand le payload ne touche PAS
|
||||
// l'onglet Information (ici seulement companyName). L'ancienne condition
|
||||
// d'intersection avec INFORMATION_FIELDS a ete retiree.
|
||||
$client = $this->minimalClient();
|
||||
$client->setCompanyName('Renamed Co'); // onglet principal uniquement, Information vide
|
||||
|
||||
$processor = $this->makeProcessor(
|
||||
granted: ['commercial.clients.manage'],
|
||||
payload: ['companyName' => 'Renamed Co'],
|
||||
user: $this->commercialeUser(),
|
||||
managed: true,
|
||||
originalData: [
|
||||
'companyName' => 'TEST CO',
|
||||
'lastName' => 'Dupont',
|
||||
'phonePrimary' => '0102030405',
|
||||
'email' => 't@test.fr',
|
||||
'triageService' => false,
|
||||
'isArchived' => false,
|
||||
],
|
||||
);
|
||||
|
||||
$this->expectException(ValidationException::class);
|
||||
$processor->process($client, $this->operation());
|
||||
}
|
||||
|
||||
public function testNonCommercialeSkipsInformationCompleteness(): void
|
||||
{
|
||||
// Meme payload incomplet, mais user non-Commerciale -> aucun blocage.
|
||||
|
||||
Reference in New Issue
Block a user