feat(commercial) : enforce RG-1.04 completeness for commerciale role

- RG-1.04 durcie : pour une Commerciale, la completude de l'onglet Information est exigee sur POST et sur tout PATCH, independamment des champs envoyes (suppression de la condition d'intersection dans validateInformationCompleteness). - Onglet Comptabilite editable par Compta : security du Patch /clients/{id} elargie a `manage` OU `accounting.manage` ; nouveau guardManage (ClientProcessor, mode strict RG-1.28) qui refuse a un porteur non-`manage` de modifier les onglets principal / Information -> 403. Compta reste donc cantonne a la Comptabilite. - Spec § 7 RG-1.04 amendee (+ consequence POST 422) + docblock du validator. - Tests unitaires ClientProcessor : guardManage (Compta accounting-only -> 200, champ metier -> 403) + RG-1.04 durcie hors onglet Information.
2026-06-01 22:26:49 +02:00
parent 275c6ff5b5
commit 09e1d7884a
5 changed files with 226 additions and 31 deletions
@@ -885,7 +885,8 @@ Cf. § 2.6. Pattern Shared standard.

 ### Onglet Information

- **RG-1.04** : Pour un utilisateur portant le rôle métier **Commerciale**, **tous** les champs de l'onglet Information (`description`, `competitors`, `foundedAt`, `employeesCount`, `revenueAmount`, `directorName`, `profitAmount`) deviennent obligatoires lors d'un PATCH sur le groupe `client:write:information`. Pour les autres rôles, ces champs restent optionnels. Implémenté via un validator custom `ClientInformationCompletenessValidator` invoqué par le `ClientProcessor` quand le user porte le rôle Commerciale.
+- **RG-1.04** _(durcie — ERP-74)_ : Pour un utilisateur portant le rôle métier **Commerciale**, **tous** les champs de l'onglet Information (`description`, `competitors`, `foundedAt`, `employeesCount`, `revenueAmount`, `directorName`, `profitAmount`) sont obligatoires sur **POST et sur tout PATCH**, **indépendamment des champs réellement envoyés** (la condition d'intersection avec `client:write:information` a été retirée). Pour les autres rôles, ces champs restent optionnels. Implémenté via un validator custom `ClientInformationCompletenessValidator` invoqué systématiquement par le `ClientProcessor` quand le user porte le rôle Commerciale.
+  - **Conséquence** : le POST n'exposant que le groupe `client:write:main`, l'onglet Information n'y est pas renseignable → une Commerciale obtient **422** sur tout POST (cf. § 8.1). La complétude se fait donc via les PATCH `client:write:information` ultérieurs. Un Admin (non gaté) crée normalement (201).

 ### Onglet Contact

@@ -10,17 +10,15 @@ use Symfony\Component\Validator\ConstraintViolation;
 use Symfony\Component\Validator\ConstraintViolationList;

 /**
- * Validator metier RG-1.04 : pour un utilisateur portant le role metier
- * Commerciale, TOUS les champs de l'onglet Information deviennent obligatoires
- * lors d'un PATCH touchant le groupe `client:write:information`.
+ * Validator metier RG-1.04 (durcie ERP-74) : pour un utilisateur portant le
+ * role metier Commerciale, TOUS les champs de l'onglet Information sont
+ * obligatoires sur POST comme sur tout PATCH, independamment des champs
+ * reellement envoyes.
 *
- * Invoque par le ClientProcessor UNIQUEMENT quand les deux conditions sont
- * reunies (role Commerciale + payload touchant l'onglet Information). Pour les
- * autres roles, ces champs restent optionnels — le validator n'est pas appele.
- *
- * Tant qu'aucun user ne porte le role `commerciale` (seede par ERP-74,
- * cf. App\Shared\Domain\Security\BusinessRoles::COMMERCIALE), cette regle reste
- * DORMANTE : aucun appelant ne la declenche.
+ * Invoque par le ClientProcessor des que l'utilisateur courant porte le role
+ * Commerciale (plus de condition d'intersection avec l'onglet Information).
+ * Pour les autres roles, ces champs restent optionnels — le validator n'est
+ * pas appele.
 *
 * Leve une ValidationException (HTTP 422) listant chaque champ manquant, par
 * coherence avec les violations Symfony rendues par API Platform.
@@ -83,11 +83,19 @@ use Symfony\Component\Validator\Constraints as Assert;
            processor: ClientProcessor::class,
        ),
        new Patch(
-            security: "is_granted('commercial.clients.manage')",
+            // Security elargie (ERP-74) : `manage` OU `accounting.manage`. Le
+            // role Compta n'a pas `manage` mais doit pouvoir editer l'onglet
+            // Comptabilite d'un client existant (§ 2.7). Le ClientProcessor
+            // re-gate ensuite onglet par onglet :
+            //  - champs accounting -> accounting.manage (guardAccounting, RG-1.28) ;
+            //  - champs main/information -> manage (guardManage : empeche Compta
+            //    d'editer les autres onglets) ;
+            //  - isArchived -> archive (guardArchive, RG-1.22).
+            security: "is_granted('commercial.clients.manage') or is_granted('commercial.clients.accounting.manage')",
            // Le ClientProcessor inspecte les champs reellement envoyes pour
            // autoriser/refuser onglet par onglet (RG-1.22 / RG-1.28) : les
            // champs accounting exigent accounting.manage, isArchived exige
-            // archive.
+            // archive, le reste (main/information) exige manage.
            normalizationContext: ['groups' => ['client:read', 'default:read']],
            denormalizationContext: ['groups' => [
                'client:write:main',
@@ -31,16 +31,19 @@ use Symfony\Component\Validator\ConstraintViolationList;
 * § 2.9 / § 4.3 / § 4.4 + RG-1.01 a RG-1.28.
 *
 * Sequence (POST / PATCH) :
- *  1. Autorisation additionnelle par groupe d'onglet (le `security` de
- *     l'operation a deja exige commercial.clients.manage) :
- *     - champ comptable dans le payload -> exige accounting.manage (RG-1.28, 403) ;
+ *  1. Autorisation additionnelle par groupe d'onglet. La security d'operation
+ *     du PATCH a ete elargie (ERP-74) a `manage` OU `accounting.manage` pour
+ *     laisser entrer le role Compta ; ce processor re-gate alors finement :
+ *     - champ comptable modifie dans le payload -> exige accounting.manage (RG-1.28, 403) ;
+ *     - champ main/information modifie -> exige manage (guardManage, 403) : empeche
+ *       Compta d'editer un autre onglet que la Comptabilite (§ 2.7) ;
 *     - champ isArchived dans le payload -> exige archive (RG-1.22, 403) et
 *       interdit toute autre modification dans la meme requete (RG-1.22, 422).
 *  2. Normalisation serveur (RG-1.18 a 1.21) via ClientFieldNormalizer.
 *  3. Regles metier : RG-1.01 (prenom/nom), RG-1.03 (distributor/broker
 *     exclusifs + type de categorie), RG-1.12 (Virement -> banque),
- *     RG-1.13 (LCR -> >= 1 RIB), RG-1.04 (completude Information pour le role
- *     Commerciale).
+ *     RG-1.13 (LCR -> >= 1 RIB), RG-1.04 (completude Information exigee sur POST
+ *     et tout PATCH pour le role Commerciale).
 *  4. Pose / retrait de archivedAt (RG-1.22 true=now, RG-1.23 false=null).
 *  5. Persistance via le persist_processor Doctrine, avec traduction des
 *     collisions d'unicite en 409 (RG-1.16 doublon de nom ; RG-1.23 conflit de
@@ -75,6 +78,7 @@ final class ClientProcessor implements ProcessorInterface
    /** Champ d'archivage (groupe client:write:archive). */
    private const string ARCHIVE_FIELD = 'isArchived';

+    private const string PERM_MANAGE            = 'commercial.clients.manage';
    private const string PERM_ACCOUNTING_MANAGE = 'commercial.clients.accounting.manage';
    private const string PERM_ARCHIVE           = 'commercial.clients.archive';

@@ -101,10 +105,15 @@ final class ClientProcessor implements ProcessorInterface

        $this->normalize($data);

+        // guardManage apres normalize : la comparaison « change vs etat
+        // persiste » des champs texte (companyName, email...) se fait sur des
+        // valeurs normalisees des deux cotes (l'etat persiste l'a deja ete).
+        $this->guardManage($data);
+
        $this->validateMainContact($data);
        $this->validateDistributorBroker($data);
        $this->validateAccountingConsistency($data);
-        $this->validateInformationCompleteness($data, $writableKeys);
+        $this->validateInformationCompleteness($data);

        try {
            return $this->persistProcessor->process($data, $operation, $uriVariables, $context);
@@ -199,6 +208,87 @@ final class ClientProcessor implements ProcessorInterface
        }
    }

+    /**
+     * § 2.7 / RG-1.28 (ERP-74) : la modification effective d'un champ « metier »
+     * (onglets principal ou Information) exige `commercial.clients.manage`. Sans
+     * cette permission -> 403 sur l'ensemble du payload (mode strict, miroir de
+     * guardAccounting). C'est ce qui empeche le role Compta — qui entre dans le
+     * PATCH via `accounting.manage` (security d'operation elargie) — d'editer
+     * autre chose que l'onglet Comptabilite.
+     *
+     * Ne s'applique qu'aux mises a jour (entite geree) : la creation (POST) est
+     * deja gardee par la security d'operation `manage`, donc inutile de la
+     * re-gater ici (et un POST par un porteur de `manage` passerait de toute
+     * facon).
+     */
+    private function guardManage(Client $data): void
+    {
+        if (!$this->em->contains($data)) {
+            return;
+        }
+
+        $changed = $this->changedBusinessFields($data);
+
+        if ([] === $changed) {
+            return;
+        }
+
+        if (!$this->security->isGranted(self::PERM_MANAGE)) {
+            throw new AccessDeniedHttpException(sprintf(
+                'Le champ "%s" requiert la permission "%s".',
+                $changed[0],
+                self::PERM_MANAGE,
+            ));
+        }
+    }
+
+    /**
+     * Champs « metier » (onglets principal + Information, hors comptabilite et
+     * archivage) dont la valeur courante differe de l'etat persiste. Memes
+     * regles de comparaison que changedAccountingFields (scalaires par valeur,
+     * relations ManyToOne distributor/broker par identite via l'identity map).
+     *
+     * Cas particulier `categories` (M2M) : non trace par getOriginalEntityData,
+     * on se rabat sur sa presence explicite dans le payload (modifier les
+     * categories = action metier exigeant manage).
+     *
+     * @return list<string>
+     */
+    private function changedBusinessFields(Client $data): array
+    {
+        $newValues = [
+            'companyName'    => $data->getCompanyName(),
+            'firstName'      => $data->getFirstName(),
+            'lastName'       => $data->getLastName(),
+            'phonePrimary'   => $data->getPhonePrimary(),
+            'phoneSecondary' => $data->getPhoneSecondary(),
+            'email'          => $data->getEmail(),
+            'distributor'    => $data->getDistributor(),
+            'broker'         => $data->getBroker(),
+            'triageService'  => $data->isTriageService(),
+            'description'    => $data->getDescription(),
+            'competitors'    => $data->getCompetitors(),
+            'foundedAt'      => $data->getFoundedAt(),
+            'employeesCount' => $data->getEmployeesCount(),
+            'revenueAmount'  => $data->getRevenueAmount(),
+            'directorName'   => $data->getDirectorName(),
+            'profitAmount'   => $data->getProfitAmount(),
+        ];
+
+        $changed = [];
+        foreach ($newValues as $field => $newValue) {
+            if ($this->fieldChanged($data, $field, $newValue)) {
+                $changed[] = $field;
+            }
+        }
+
+        if (in_array('categories', $this->writablePayloadKeys(), true)) {
+            $changed[] = 'categories';
+        }
+
+        return $changed;
+    }
+
    /**
     * Champs comptables dont la valeur courante differe de l'etat persiste. Les
     * relations (tvaMode, paymentDelay, paymentType, bank) sont comparees par
@@ -353,17 +443,16 @@ final class ClientProcessor implements ProcessorInterface
    }

    /**
-     * RG-1.04 : si l'utilisateur porte le role metier Commerciale ET que le
-     * payload touche l'onglet Information, tous les champs Information sont
-     * obligatoires. Dormant tant qu'aucun user ne porte le role `commerciale`.
-     *
-     * @param list<string> $payloadKeys
+     * RG-1.04 (durcie ERP-74) : si l'utilisateur porte le role metier
+     * Commerciale, TOUS les champs de l'onglet Information sont obligatoires sur
+     * POST comme sur TOUT PATCH — independamment des champs reellement envoyes
+     * (plus de condition d'intersection avec INFORMATION_FIELDS). Garantit qu'un
+     * client cree/edite par une Commerciale ne reste jamais avec un onglet
+     * Information incomplet.
     */
-    private function validateInformationCompleteness(Client $data, array $payloadKeys): void
+    private function validateInformationCompleteness(Client $data): void
    {
-        $touchesInformation = [] !== array_intersect($payloadKeys, self::INFORMATION_FIELDS);
-
-        if ($touchesInformation && $this->currentUserIsCommerciale()) {
+        if ($this->currentUserIsCommerciale()) {
            $this->informationValidator->validate($data);
        }
    }
@@ -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.