diff --git a/config/version.yaml b/config/version.yaml index 54eb4dc..ad1140f 100644 --- a/config/version.yaml +++ b/config/version.yaml @@ -1,2 +1,2 @@ parameters: - app.version: '0.1.64' + app.version: '0.1.65' diff --git a/src/Module/Commercial/Domain/Entity/Client.php b/src/Module/Commercial/Domain/Entity/Client.php index 4ce07e4..bdcac63 100644 --- a/src/Module/Commercial/Domain/Entity/Client.php +++ b/src/Module/Commercial/Domain/Entity/Client.php @@ -68,15 +68,21 @@ use Symfony\Component\Validator\Constraints as Assert; ), new Get( security: "is_granted('commercial.clients.view')", - // Detail : client + sous-collections embarquees. Le groupe - // client:read:accounting est ajoute par le context builder selon la - // permission, donc absent ici volontairement. + // Detail : client + sous-collections embarquees. + // - client:read:accounting est ajoute par le context builder selon la + // permission (gate les scalaires comptables ET les RIB embarques), + // donc absent ici volontairement. + // - client_rib:read N'EST PLUS dans le contexte : le contenu des RIB + // embarques est desormais porte par client:read:accounting (gate), + // ce qui retire la fuite IBAN/BIC vers les users sans accounting.view. + // - category:read et site:read sont indispensables pour embarquer le + // code/libelle des categories et des sites (sinon stub IRI nu) : + // Category.code/name vivent sous category:read, Site.name sous site:read. normalizationContext: ['groups' => [ 'client:read', 'client:item:read', 'client_contact:read', 'client_address:read', - 'client_rib:read', 'category:read', 'site:read', 'default:read', @@ -682,8 +688,14 @@ class Client implements TimestampableInterface, BlamableInterface return array_values($sites); } + // Embed gate sur le groupe COMPTABLE (et non client:item:read comme contacts/ + // adresses) : client:read:accounting n'est ajoute au contexte que si l'user a + // accounting.view (ClientReadGroupContextBuilder). Resultat : la cle `ribs` est + // TOTALEMENT ABSENTE du detail pour un user sans accounting.view (ex. Commerciale), + // au meme titre que les scalaires comptables — corrige la fuite de RIB ou la + // Commerciale recevait IBAN/BIC en clair. /** @return Collection */ - #[Groups(['client:item:read'])] + #[Groups(['client:read:accounting'])] public function getRibs(): Collection { return $this->ribs; diff --git a/src/Module/Commercial/Domain/Entity/ClientAddress.php b/src/Module/Commercial/Domain/Entity/ClientAddress.php index bfceeac..c359088 100644 --- a/src/Module/Commercial/Domain/Entity/ClientAddress.php +++ b/src/Module/Commercial/Domain/Entity/ClientAddress.php @@ -22,6 +22,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping as ORM; use Symfony\Component\Serializer\Attribute\Groups; +use Symfony\Component\Serializer\Attribute\SerializedName; use Symfony\Component\Validator\Constraints as Assert; use Symfony\Component\Validator\Context\ExecutionContextInterface; @@ -104,16 +105,23 @@ class ClientAddress implements TimestampableInterface, BlamableInterface #[ORM\JoinColumn(name: 'client_id', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')] private ?Client $client = null; + // Groupe d'ECRITURE uniquement sur la propriete (denormalisation PATCH/POST). + // Le groupe de LECTURE est porte par le getter isProspect()/isDelivery()/ + // isBilling() avec SerializedName : sans cela, Symfony strip le prefixe "is" + // des getters booleens et exposerait les cles "prospect"/"delivery"/"billing" + // — en pratique le #[Groups] etant sur la propriete `isX` et le getter + // derivant l'attribut `x`, la cle etait totalement DROPPEE du JSON (meme bug + // que Client::isArchived). Pattern corrige : Groups + SerializedName sur le getter. #[ORM\Column(name: 'is_prospect', options: ['default' => false])] - #[Groups(['client_address:read', 'client_address:write'])] + #[Groups(['client_address:write'])] private bool $isProspect = false; #[ORM\Column(name: 'is_delivery', options: ['default' => false])] - #[Groups(['client_address:read', 'client_address:write'])] + #[Groups(['client_address:write'])] private bool $isDelivery = false; #[ORM\Column(name: 'is_billing', options: ['default' => false])] - #[Groups(['client_address:read', 'client_address:write'])] + #[Groups(['client_address:write'])] private bool $isBilling = false; #[ORM\Column(length: 80, options: ['default' => 'France'])] @@ -276,6 +284,12 @@ class ClientAddress implements TimestampableInterface, BlamableInterface return $this; } + // Groupe de lecture + nom serialise explicite (cf. note sur la propriete) : + // sans SerializedName, Symfony exposerait la cle "prospect" (strip du prefixe + // "is" sur les getters) et, le groupe etant declare sur la propriete `isProspect`, + // droppait silencieusement la cle du JSON. + #[Groups(['client_address:read'])] + #[SerializedName('isProspect')] public function isProspect(): bool { return $this->isProspect; @@ -288,6 +302,8 @@ class ClientAddress implements TimestampableInterface, BlamableInterface return $this; } + #[Groups(['client_address:read'])] + #[SerializedName('isDelivery')] public function isDelivery(): bool { return $this->isDelivery; @@ -300,6 +316,8 @@ class ClientAddress implements TimestampableInterface, BlamableInterface return $this; } + #[Groups(['client_address:read'])] + #[SerializedName('isBilling')] public function isBilling(): bool { return $this->isBilling; diff --git a/src/Module/Commercial/Domain/Entity/ClientRib.php b/src/Module/Commercial/Domain/Entity/ClientRib.php index 63a9447..aa5eba5 100644 --- a/src/Module/Commercial/Domain/Entity/ClientRib.php +++ b/src/Module/Commercial/Domain/Entity/ClientRib.php @@ -79,10 +79,17 @@ class ClientRib implements TimestampableInterface, BlamableInterface { use TimestampableBlamableTrait; + // Double groupe de lecture : + // - `client_rib:read` : sous-ressource autonome GET /api/client_ribs/{id} + // (deja securisee par commercial.clients.accounting.view). + // - `client:read:accounting` : embed des RIB sous le detail Client, ajoute + // DYNAMIQUEMENT par ClientReadGroupContextBuilder uniquement si l'user a + // accounting.view. Ce double marquage gate les RIB embarques au meme titre + // que les scalaires comptables (RG : la Commerciale ne voit aucun RIB). #[ORM\Id] #[ORM\GeneratedValue] #[ORM\Column] - #[Groups(['client_rib:read'])] + #[Groups(['client_rib:read', 'client:read:accounting'])] private ?int $id = null; #[ORM\ManyToOne(targetEntity: Client::class, inversedBy: 'ribs')] @@ -92,23 +99,23 @@ class ClientRib implements TimestampableInterface, BlamableInterface #[ORM\Column(length: 120)] #[Assert\NotBlank] #[Assert\Length(max: 120, normalizer: 'trim')] - #[Groups(['client_rib:read', 'client_rib:write'])] + #[Groups(['client_rib:read', 'client:read:accounting', 'client_rib:write'])] private ?string $label = null; #[ORM\Column(length: 20)] #[Assert\NotBlank] #[Assert\Bic] - #[Groups(['client_rib:read', 'client_rib:write'])] + #[Groups(['client_rib:read', 'client:read:accounting', 'client_rib:write'])] private ?string $bic = null; #[ORM\Column(length: 34)] #[Assert\NotBlank] #[Assert\Iban] - #[Groups(['client_rib:read', 'client_rib:write'])] + #[Groups(['client_rib:read', 'client:read:accounting', 'client_rib:write'])] private ?string $iban = null; #[ORM\Column(options: ['default' => 0])] - #[Groups(['client_rib:read', 'client_rib:write'])] + #[Groups(['client_rib:read', 'client:read:accounting', 'client_rib:write'])] private int $position = 0; public function getId(): ?int diff --git a/tests/Module/Commercial/Api/ClientSerializationContractTest.php b/tests/Module/Commercial/Api/ClientSerializationContractTest.php new file mode 100644 index 0000000..680c9e3 --- /dev/null +++ b/tests/Module/Commercial/Api/ClientSerializationContractTest.php @@ -0,0 +1,281 @@ +skipIfSitesModuleDisabled(); + + $seed = $this->seedCompleteClient('Bool Addr Co'); + $id = $seed->getId(); + + $http = $this->createAdminClient(); + $data = $http->request('GET', '/api/clients/'.$id, ['headers' => ['Accept' => self::LD]])->toArray(); + + self::assertArrayHasKey('addresses', $data); + self::assertNotEmpty($data['addresses']); + $address = $data['addresses'][0]; + + // Le bug droppait TOTALEMENT ces cles. Apres correctif (Groups + + // SerializedName sur le getter), elles sont presentes ET typees bool. + self::assertArrayHasKey('isProspect', $address); + self::assertArrayHasKey('isDelivery', $address); + self::assertArrayHasKey('isBilling', $address); + + // L'adresse seedee est livraison + facturation (prospect exclusif, RG-1.06). + // Prouve qu'un booleen `true` est bien serialise (le bug masquait meme les true). + self::assertFalse($address['isProspect']); + self::assertTrue($address['isDelivery']); + self::assertTrue($address['isBilling']); + } + + // === #80 — Gating des RIB par accounting.view === + + public function testRibsPresentForAdminWithAccountingView(): void + { + $this->skipIfSitesModuleDisabled(); + + $seed = $this->seedCompleteClient('Rib Admin Co'); + $id = $seed->getId(); + + $http = $this->createAdminClient(); + $data = $http->request('GET', '/api/clients/'.$id, ['headers' => ['Accept' => self::LD]])->toArray(); + + // Admin bypass RBAC -> accounting.view -> RIB embarques (label/bic/iban). + self::assertArrayHasKey('ribs', $data); + self::assertNotEmpty($data['ribs']); + self::assertSame('Compte principal', $data['ribs'][0]['label']); + self::assertSame(self::VALID_IBAN, $data['ribs'][0]['iban']); + self::assertSame(self::VALID_BIC, $data['ribs'][0]['bic']); + } + + public function testRibsAbsentForUserWithoutAccountingView(): void + { + $this->skipIfSitesModuleDisabled(); + + $seed = $this->seedCompleteClient('Rib Commerciale Co'); + $id = $seed->getId(); + + // Commerciale : commercial.clients.view SANS accounting.view. + $creds = $this->createUserWithPermission('commercial.clients.view'); + $http = $this->authenticatedClient($creds['username'], $creds['password']); + + $data = $http->request('GET', '/api/clients/'.$id, ['headers' => ['Accept' => self::LD]])->toArray(); + + // La cle `ribs` est ABSENTE (pas null) : le groupe client:read:accounting + // n'est pas ajoute au contexte -> getRibs() jamais serialise. Fin de la + // fuite IBAN/BIC. + self::assertArrayNotHasKey('ribs', $data); + } + + // === #80.bis — Gating par OMISSION des scalaires comptables === + + public function testAccountingScalarsGatedByOmission(): void + { + $this->skipIfSitesModuleDisabled(); + + $seed = $this->seedCompleteClient('Compta Gating Co'); + $id = $seed->getId(); + + // Admin : scalaires comptables presents. + $admin = $this->createAdminClient(); + $adminData = $admin->request('GET', '/api/clients/'.$id, ['headers' => ['Accept' => self::LD]])->toArray(); + self::assertArrayHasKey('siren', $adminData); + self::assertSame('123456789', $adminData['siren']); + self::assertArrayHasKey('accountNumber', $adminData); + + // Commerciale : scalaires comptables ABSENTS (omission, pas null). + $creds = $this->createUserWithPermission('commercial.clients.view'); + $http = $this->authenticatedClient($creds['username'], $creds['password']); + $data = $http->request('GET', '/api/clients/'.$id, ['headers' => ['Accept' => self::LD]])->toArray(); + + self::assertArrayNotHasKey('siren', $data); + self::assertArrayNotHasKey('accountNumber', $data); + self::assertArrayNotHasKey('nTva', $data); + self::assertArrayNotHasKey('ribs', $data); + } + + // === #82 — Embed code/libelle des Category et Site === + + public function testCategoriesEmbedCodeAndLabel(): void + { + $this->skipIfSitesModuleDisabled(); + + $seed = $this->seedCompleteClient('Embed Cat Co'); + $id = $seed->getId(); + + $http = $this->createAdminClient(); + $data = $http->request('GET', '/api/clients/'.$id, ['headers' => ['Accept' => self::LD]])->toArray(); + + self::assertNotEmpty($data['categories']); + $category = $data['categories'][0]; + // Avant correctif : seuls @id/@type/createdAt/updatedAt (category:read + // absent du contexte). Apres : code + name (libelle) embarques. + self::assertArrayHasKey('code', $category); + self::assertArrayHasKey('name', $category); + self::assertNotSame('', $category['code']); + } + + public function testAddressSitesEmbedLabel(): void + { + $this->skipIfSitesModuleDisabled(); + + $seed = $this->seedCompleteClient('Embed Site Co'); + $id = $seed->getId(); + + $http = $this->createAdminClient(); + $data = $http->request('GET', '/api/clients/'.$id, ['headers' => ['Accept' => self::LD]])->toArray(); + + $address = $data['addresses'][0]; + self::assertArrayHasKey('sites', $address); + self::assertNotEmpty($address['sites']); + // Site embarque : libelle `name` present (avant : stub @id/@type nu). + // NB : Site n'a pas de champ `code` (cf. note de classe) -> on asserte name. + self::assertArrayHasKey('name', $address['sites'][0]); + self::assertNotSame('', $address['sites'][0]['name']); + + // L'adresse seedee est multi-sites : preuve que l'embed parcourt la collection. + self::assertGreaterThanOrEqual(2, count($address['sites'])); + + // Categories d'adresse : code embarque (category:read dans le contexte). + self::assertArrayHasKey('categories', $address); + self::assertNotEmpty($address['categories']); + self::assertArrayHasKey('code', $address['categories'][0]); + } + + // === Enveloppe AP4 (sans prefixe hydra:) + exclusion des archives === + + public function testCollectionEnvelopeShapeAndArchivedExcluded(): void + { + $http = $this->createAdminClient(); + $prefix = 'EnvCheck'.substr(bin2hex(random_bytes(3)), 0, 6); + + $this->seedClient($prefix.' Active'); + $this->seedClient($prefix.' Archived', true); + + // Liste par defaut filtree sur le prefixe : enveloppe member/totalItems + // sans prefixe hydra:, archive EXCLU du totalItems (RG-1.24). + $default = $http->request('GET', '/api/clients?search='.$prefix, ['headers' => ['Accept' => self::LD]])->toArray(); + + self::assertArrayHasKey('member', $default); + self::assertArrayHasKey('totalItems', $default); + self::assertArrayNotHasKey('hydra:member', $default); + self::assertArrayNotHasKey('hydra:totalItems', $default); + self::assertSame(1, $default['totalItems'], 'Archive exclu du totalItems par defaut.'); + + // includeArchived : l'archive reintegre le total. + $all = $http->request('GET', '/api/clients?search='.$prefix.'&includeArchived=true', ['headers' => ['Accept' => self::LD]])->toArray(); + self::assertSame(2, $all['totalItems']); + + // `view` (PartialCollectionView) sans prefixe hydra: : force le multi-page + // via itemsPerPage=1 sur les 2 resultats archives inclus. + $paged = $http->request('GET', '/api/clients?search='.$prefix.'&includeArchived=true&itemsPerPage=1', ['headers' => ['Accept' => self::LD]])->toArray(); + self::assertArrayHasKey('view', $paged); + self::assertArrayNotHasKey('hydra:view', $paged); + } + + // === Helper === + + /** + * Seede un client COMPLET (sans passer par l'API, validations applicatives + * non rejouees mais CHECK BDD respectes) : bloc comptable non nul, >= 1 RIB, + * >= 1 adresse multi-sites avec categories, >= 1 contact, >= 1 categorie. + * + * L'adresse est livraison + facturation (prospect exclusif, RG-1.06 ; email + * de facturation present, RG-1.11) afin de poser des booleens `true` + * serialisables tout en respectant les CHECK Postgres. + */ + private function seedCompleteClient(string $companyName): ClientEntity + { + $em = $this->getEm(); + + // Nom unique parmi les actifs (index partiel uq_client_company_name_active). + $suffix = substr(bin2hex(random_bytes(3)), 0, 6); + + $client = new ClientEntity(); + $client->setCompanyName(mb_strtoupper($companyName.' '.$suffix, 'UTF-8')); + $client->setLastName('Complet'); + $client->setPhonePrimary('0102030405'); + $client->setEmail('complet'.$suffix.'@seed.test'); + $client->addCategory($this->createCategory('SECTEUR')); + // Bloc comptable non nul (gating par omission cote Commerciale). + $client->setSiren('123456789'); + $client->setAccountNumber('C0001'); + $client->setNTva('FR00123456789'); + $em->persist($client); + + // >= 2 sites fixtures pour une adresse multi-sites (RG-1.10). + $sites = $em->getRepository(Site::class)->findBy([], null, 2); + self::assertGreaterThanOrEqual(2, count($sites), 'Au moins 2 sites fixtures requis (SitesFixtures).'); + + $address = new ClientAddress(); + $address->setClient($client); + $address->setIsProspect(false); + $address->setIsDelivery(true); + $address->setIsBilling(true); + $address->setBillingEmail('billing'.$suffix.'@seed.test'); + $address->setPostalCode('86000'); + $address->setCity('Poitiers'); + $address->setStreet('12 rue des Acacias'); + foreach ($sites as $site) { + $address->addSite($site); + } + $address->addCategory($this->createCategory('SECTEUR')); + $em->persist($address); + + $rib = new ClientRib(); + $rib->setClient($client); + $rib->setLabel('Compte principal'); + $rib->setBic(self::VALID_BIC); + $rib->setIban(self::VALID_IBAN); + $em->persist($rib); + + $contact = new ClientContact(); + $contact->setClient($client); + $contact->setFirstName('Marie'); + $contact->setLastName('Martin'); + $em->persist($contact); + + $em->flush(); + + return $client; + } +}