1888b70623
Auto Tag Develop / tag (push) Successful in 11s
## Contexte
ERP-102 — Découvert pendant ERP-64. Connecté avec un rôle **métier** (bureau / compta / commerciale), `GET /api/categories` et `GET /api/sites` renvoient **403**, alors que `/tva_modes`, `/payment_delays`, `/payment_types`, `/banks` renvoient 200.
Conséquences : page **Création client** inutilisable (le `Promise.all` rejetait → **tous** les selects vides) et **filtres Catégories/Sites vides** au répertoire.
## Cause
La `security` des `GetCollection`/`Get` de `Category` et `Site` exigeait `catalog.categories.view` / `sites.view` — permissions d'**administration** du Catalogue / des Sites. Or ces référentiels sont **transverses** : tout rôle qui gère un tiers doit pouvoir les lire.
## Correctif back — Option C (permission de lecture-référentiel dédiée)
Choix d'archi retenu parmi les 3 du ticket :
- **Pourquoi pas A** (`... or is_granted('commercial.clients.view')`) : coupler `Category`/`Site` à une permission **Commercial** viole l'esprit de la règle ABSOLUE n°1 et ne scale pas (M2 Fournisseurs devrait rajouter un OR).
- **Pourquoi pas B** (donner `.view` aux rôles métier) : `.view` = accès admin → items sidebar admin Catégories/Sites exposés à une commerciale.
- **C** : nouvelle permission `catalog.categories.read_ref` / `sites.read_ref`, distincte de `.view` (pas d'item sidebar) et de `.manage`. Chaque permission appartient à **son** module → isolement inter-module préservé, **réutilisable tel quel par M2 Fournisseurs**. C'est la « permission référentiel lisible » que le ticket pointe lui-même.
Détail :
- `CatalogModule` / `SitesModule` : déclaration des deux permissions `read_ref`.
- `Category` / `Site` : security lecture (liste + item) = `view OR read_ref`.
- `RbacSeeder` (matrice § 2.7) : `read_ref` attaché à bureau / compta / commerciale ; usine reste sans accès.
## Durcissement front (résilience — requis dans tous les cas)
`useClientReferentials.loadCommon` : `Promise.all` → **`Promise.allSettled`** avec affectation isolée par référentiel. L'échec d'un endpoint ne vide plus que **son** select, plus la totalité du formulaire.
## Tests (TDD)
- `ClientRBACMatrixTest::testBusinessRolesCanReadCategoriesAndSitesReferentials` — bureau/compta/commerciale listent `/categories` et `/sites` (200), usine reste 403.
- `SitesModuleTest` — set de permissions porté à 4 codes.
- `useClientReferentials.spec` (Vitest) — un référentiel en échec ne vide que son select.
## Vérifications
- `make test` (back) : **467/467** ✓
- `make nuxt-test` (front) : **131/131** ✓
- `make php-cs-fixer` : conforme ✓
## Note miroirs RBAC
`config/sidebar.php` / `personas.ts` / `SeedE2ECommand.php` **non touchés** : `read_ref` n'ajoute aucun item sidebar, le persona E2E `user-full` lit déjà via `.view`, et aucun persona ne modélise un rôle métier seul. Pas de nouveau test E2E (règle n°7 : bug attrapé avant prod). La source de vérité de la matrice (`RbacSeeder`) est mise à jour et couverte par `ClientRBACMatrixTest`.
Closes ERP-102.
---------
Co-authored-by: Matthieu <contact@malio.fr>
Reviewed-on: #53
Co-authored-by: THOLOT DECHENE Matthieu <matthieu@yuno.malio.fr>
Co-committed-by: THOLOT DECHENE Matthieu <matthieu@yuno.malio.fr>
346 lines
14 KiB
PHP
346 lines
14 KiB
PHP
<?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 App\Module\Sites\Domain\Entity\Site;
|
|
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);
|
|
}
|
|
|
|
public function testBusinessRolesCanReadCategoriesAndSitesReferentials(): void
|
|
{
|
|
// ERP-102 : /categories et /sites sont des referentiels TRANSVERSES.
|
|
// Tout role qui gere des clients (bureau / compta / commerciale) doit
|
|
// pouvoir les LISTER pour alimenter les selects de creation/filtre client,
|
|
// via la permission de lecture-referentiel dediee (catalog.categories.read_ref
|
|
// / sites.read_ref) attachee par la matrice § 2.7 — sans pour autant porter
|
|
// la permission d'administration `.view`. Usine, sans aucune permission,
|
|
// reste interdit.
|
|
// Le referentiel /sites est TRANSVERSE et COMPLET : le cloisonnement par
|
|
// site rattache (SiteCollectionScopedExtension) est neutralise par
|
|
// `sites.read_ref` (ERP-102). Les comptes demo ne sont rattaches qu'a un
|
|
// seul site (Chatellerault) alors que la base en compte plusieurs : on
|
|
// verifie donc que le role voit la TOTALITE du referentiel, pas son seul
|
|
// site rattache. Sans le bypass de scope, totalItems vaudrait 1.
|
|
$totalSites = $this->getEm()->getRepository(Site::class)->count([]);
|
|
self::assertGreaterThan(
|
|
1,
|
|
$totalSites,
|
|
'Pre-requis du test : la base doit contenir plusieurs sites pour distinguer scope et bypass.',
|
|
);
|
|
|
|
foreach (['bureau', 'compta', 'commerciale'] as $role) {
|
|
$client = $this->authAs($role);
|
|
|
|
$client->request('GET', '/api/categories', ['headers' => ['Accept' => self::LD]]);
|
|
self::assertResponseStatusCodeSame(200, sprintf('Le role %s doit pouvoir lister /categories', $role));
|
|
|
|
$response = $client->request('GET', '/api/sites', ['headers' => ['Accept' => self::LD]]);
|
|
self::assertResponseStatusCodeSame(200, sprintf('Le role %s doit pouvoir lister /sites', $role));
|
|
self::assertSame(
|
|
$totalSites,
|
|
$response->toArray()['totalItems'] ?? null,
|
|
sprintf('Le role %s doit voir tout le referentiel sites (%d), pas seulement son site rattache', $role, $totalSites),
|
|
);
|
|
}
|
|
|
|
// Usine : aucune permission -> reste a 403 sur les referentiels.
|
|
$usine = $this->authAs('usine');
|
|
$usine->request('GET', '/api/categories', ['headers' => ['Accept' => self::LD]]);
|
|
self::assertResponseStatusCodeSame(403, 'Usine ne doit pas pouvoir lister /categories');
|
|
$usine->request('GET', '/api/sites', ['headers' => ['Accept' => self::LD]]);
|
|
self::assertResponseStatusCodeSame(403, 'Usine ne doit pas pouvoir lister /sites');
|
|
}
|
|
|
|
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],
|
|
];
|
|
}
|
|
}
|