From 75dd98668c00d32ace018581d3498ceeb41d4b99 Mon Sep 17 00:00:00 2001 From: Imugiii Date: Mon, 15 Jun 2026 18:57:01 +0000 Subject: [PATCH] feat(pin): primitif de verification du PIN d'action sensible (RG-T13) PinVerifier verifie un PIN soumis contre user.pin_hash (argon2id, default-deny, filtre is_active = 1) et porte la politique de longueur (chiffres ASCII, bornes min/max STAFF_PIN_*, RG-T18). Primitif reutilise par chaque operation sensible en P3 (annulation, prix/TVA, suppressions, inventaire, gestion user/RBAC, effacement PII) ; le flux PIN + audit_log dans la meme transaction est specifie dans docs/uml/security-sequence.md. Un decoy argon2id sur le chemin sans PIN egalise le timing (anti-enumeration). Tests unit + integration (auto-skippee), dont la garde du filtre is_active contre le vrai schema. --- .env.example | 4 +- docker-compose.yml | 3 +- src/app/Auth/PinVerifier.php | 79 +++++++++++++++++ tests/Integration/PinVerifierDbTest.php | 102 +++++++++++++++++++++ tests/Support/FakeDatabase.php | 13 +++ tests/Unit/Auth/PinVerifierTest.php | 112 ++++++++++++++++++++++++ 6 files changed, 311 insertions(+), 2 deletions(-) create mode 100644 src/app/Auth/PinVerifier.php create mode 100644 tests/Integration/PinVerifierDbTest.php create mode 100644 tests/Unit/Auth/PinVerifierTest.php diff --git a/.env.example b/.env.example index 10d3c84..27853b2 100644 --- a/.env.example +++ b/.env.example @@ -93,8 +93,10 @@ ACCOUNT_LOCKOUT_MAX_SECONDS=900 # plafond du backoff (15 min) IP_THROTTLE_WINDOW_SECONDS=900 # 15 min IP_THROTTLE_MAX_ATTEMPTS=20 # par IP sur la fenetre -# PIN equipier pour actions sensibles (annulation, override). Longueur minimale. +# PIN equipier pour actions sensibles (annulation, override). Chiffres uniquement, +# bornes min ET max (RG-T18 : validation serveur + longueur bornee). STAFF_PIN_MIN_LENGTH=4 +STAFF_PIN_MAX_LENGTH=12 # Expiration du token de reinitialisation de mot de passe (secondes). PASSWORD_RESET_TTL=3600 # 1h diff --git a/docker-compose.yml b/docker-compose.yml index 589abe9..303f000 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -147,8 +147,9 @@ services: ACCOUNT_LOCKOUT_MAX_SECONDS: ${ACCOUNT_LOCKOUT_MAX_SECONDS} IP_THROTTLE_WINDOW_SECONDS: ${IP_THROTTLE_WINDOW_SECONDS} IP_THROTTLE_MAX_ATTEMPTS: ${IP_THROTTLE_MAX_ATTEMPTS} - # Longueur minimale du PIN equipier (actions sensibles, P3). + # Bornes du PIN equipier (actions sensibles, P3) : longueur min ET max. STAFF_PIN_MIN_LENGTH: ${STAFF_PIN_MIN_LENGTH} + STAFF_PIN_MAX_LENGTH: ${STAFF_PIN_MAX_LENGTH} # Expiration du token de reinitialisation de mot de passe (mlt.md 12.3). PASSWORD_RESET_TTL: ${PASSWORD_RESET_TTL} UPLOAD_MAX_SIZE_MB: ${UPLOAD_MAX_SIZE_MB} diff --git a/src/app/Auth/PinVerifier.php b/src/app/Auth/PinVerifier.php new file mode 100644 index 0000000..85c2b9a --- /dev/null +++ b/src/app/Auth/PinVerifier.php @@ -0,0 +1,79 @@ +db->fetch( + 'SELECT pin_hash FROM user WHERE id = :id AND is_active = 1', + ['id' => $userId], + ); + + $hash = is_string($row['pin_hash'] ?? null) ? (string) $row['pin_hash'] : ''; + + if ($hash === '') { + // Egalise le timing avec le chemin mauvais-PIN (verify argon2id) : sans + // ce leurre, un compte sans PIN (ou inactif/absent) repondrait plus vite, + // revelant par la latence quels comptes ont un PIN defini (anti-enumeration, + // meme posture que AuthService RG-2). Le leurre est mis en cache process. + $this->hasher->verifyDecoy($pin); + + return false; + } + + return $this->hasher->verify($pin, $hash); + } + + /** + * Politique de PIN a verifier cote serveur avant de hacher un nouveau PIN + * (P3, definition du PIN) : chiffres ASCII uniquement, bornes min ET max + * (RG-T18). ctype_digit garantit le charset numerique, ce qui rend strlen + * fiable comme nombre de caracteres. + */ + public function meetsLengthPolicy(string $pin): bool + { + $min = $this->config->int('STAFF_PIN_MIN_LENGTH', 4); + $max = $this->config->int('STAFF_PIN_MAX_LENGTH', 12); + + return $pin !== '' && ctype_digit($pin) && strlen($pin) >= $min && strlen($pin) <= $max; + } +} diff --git a/tests/Integration/PinVerifierDbTest.php b/tests/Integration/PinVerifierDbTest.php new file mode 100644 index 0000000..c52a9af --- /dev/null +++ b/tests/Integration/PinVerifierDbTest.php @@ -0,0 +1,102 @@ +config = new Config(); + $this->db = new Database($this->config); + + try { + $this->db->fetch('SELECT 1'); + } catch (Throwable $exception) { + self::markTestSkipped('Base injoignable: ' . $exception->getMessage()); + } + + $roleRow = $this->db->fetch('SELECT id FROM role ORDER BY id LIMIT 1'); + $roleId = (int) ($roleRow['id'] ?? 0); + self::assertGreaterThan(0, $roleId, 'role seede attendu'); + + $hasher = new PasswordHasher($this->config); + $this->db->execute( + 'INSERT INTO user (email, password_hash, pin_hash, first_name, last_name, role_id, is_active) ' + . 'VALUES (:email, :pwd, :pin, :fn, :ln, :role, 1)', + [ + 'email' => 'it-pin-' . bin2hex(random_bytes(6)) . '@wakdo.invalid', + 'pwd' => $hasher->hash('IntegrationPass1'), + 'pin' => $hasher->hash(self::PIN), + 'fn' => 'Integration', + 'ln' => 'Pin', + 'role' => $roleId, + ], + ); + $this->userId = (int) ($this->db->fetch('SELECT LAST_INSERT_ID() AS id')['id'] ?? 0); + } + + protected function tearDown(): void + { + if ($this->userId === 0) { + return; + } + + $this->db->execute('DELETE FROM user WHERE id = :id', ['id' => $this->userId]); + $this->userId = 0; + } + + private function verifier(): PinVerifier + { + return new PinVerifier($this->db, $this->config, new PasswordHasher($this->config)); + } + + public function testVerifyAgainstRealPinHash(): void + { + $verifier = $this->verifier(); + + self::assertTrue($verifier->verify($this->userId, self::PIN)); + self::assertFalse($verifier->verify($this->userId, '0000')); + } + + public function testVerifyFalseWhenPinHashNull(): void + { + $this->db->execute('UPDATE user SET pin_hash = NULL WHERE id = :id', ['id' => $this->userId]); + + self::assertFalse($this->verifier()->verify($this->userId, self::PIN)); + } + + public function testVerifyFalseWhenUserInactive(): void + { + // Compte desactive mais pin_hash encore valide : le filtre is_active = 1 + // doit refuser (un equipier desactive ne re-autorise plus d'action sensible). + $this->db->execute('UPDATE user SET is_active = 0 WHERE id = :id', ['id' => $this->userId]); + + self::assertFalse($this->verifier()->verify($this->userId, self::PIN)); + } +} diff --git a/tests/Support/FakeDatabase.php b/tests/Support/FakeDatabase.php index 3c27e54..f6ad7d2 100644 --- a/tests/Support/FakeDatabase.php +++ b/tests/Support/FakeDatabase.php @@ -83,6 +83,13 @@ final class FakeDatabase implements DatabaseInterface */ public ?array $roleRow = null; + /** + * Ligne user renvoyee pour la verification du PIN (RG-T13) ; null = absent/inactif. + * + * @var array|null + */ + public ?array $pinUserRow = null; + /** Si non nul, execute() leve cette exception (simulation panne DB -> fail-closed). */ public ?RuntimeException $failOnExecute = null; @@ -120,6 +127,12 @@ final class FakeDatabase implements DatabaseInterface return $this->roleRow; } + // Exige le predicat is_active = 1 : si la production le retirait, le double + // renverrait null et le test verify-true virerait au rouge (garde RG-T13). + if (str_contains($sql, 'SELECT pin_hash FROM user WHERE id') && str_contains($sql, 'is_active = 1')) { + return $this->pinUserRow; + } + if (str_contains($sql, 'SELECT lockout_until FROM login_throttle')) { return ['lockout_until' => $this->ipLockoutUntil]; } diff --git a/tests/Unit/Auth/PinVerifierTest.php b/tests/Unit/Auth/PinVerifierTest.php new file mode 100644 index 0000000..969fd76 --- /dev/null +++ b/tests/Unit/Auth/PinVerifierTest.php @@ -0,0 +1,112 @@ + */ + private array $touchedKeys = []; + + private FakeDatabase $db; + private PasswordHasher $hasher; + + protected function setUp(): void + { + $this->setEnv('STAFF_PIN_MIN_LENGTH', '4'); + $this->setEnv('STAFF_PIN_MAX_LENGTH', '12'); + $this->setEnv('ARGON2_MEMORY_COST', '1024'); + $this->setEnv('ARGON2_TIME_COST', '1'); + $this->setEnv('ARGON2_THREADS', '1'); + + $this->db = new FakeDatabase(); + $this->hasher = new PasswordHasher(new Config()); + } + + protected function tearDown(): void + { + foreach ($this->touchedKeys as $key) { + putenv($key); + } + $this->touchedKeys = []; + } + + private function setEnv(string $key, string $value): void + { + $this->touchedKeys[] = $key; + putenv($key . '=' . $value); + } + + private function verifier(): PinVerifier + { + return new PinVerifier($this->db, new Config(), $this->hasher); + } + + public function testVerifyTrueWhenPinMatches(): void + { + $this->db->pinUserRow = ['pin_hash' => $this->hasher->hash('4729')]; + + self::assertTrue($this->verifier()->verify(7, '4729')); + // Garde RG-T13 : la lecture filtre bien is_active = 1 (retirer le predicat + // ferait echouer ce cas via le routage durci du FakeDatabase). + self::assertStringContainsString('is_active = 1', $this->db->reads[0]['sql']); + } + + public function testVerifyFalseWhenPinWrong(): void + { + $this->db->pinUserRow = ['pin_hash' => $this->hasher->hash('4729')]; + + self::assertFalse($this->verifier()->verify(7, '0000')); + } + + public function testVerifyFalseWhenPinHashNull(): void + { + // PIN non defini sur le compte. + $this->db->pinUserRow = ['pin_hash' => null]; + + self::assertFalse($this->verifier()->verify(7, '4729')); + } + + public function testVerifyFalseWhenUserAbsentOrInactive(): void + { + // La requete filtre is_active = 1 : un compte inactif/absent ne renvoie rien. + $this->db->pinUserRow = null; + + self::assertFalse($this->verifier()->verify(7, '4729')); + } + + public function testVerifyFalseWhenPinEmpty(): void + { + $this->db->pinUserRow = ['pin_hash' => $this->hasher->hash('4729')]; + + self::assertFalse($this->verifier()->verify(7, '')); + } + + public function testMeetsLengthPolicy(): void + { + $verifier = $this->verifier(); + + // Sous le minimum / au minimum / dans les bornes. + self::assertFalse($verifier->meetsLengthPolicy('123')); + self::assertTrue($verifier->meetsLengthPolicy('1234')); + self::assertTrue($verifier->meetsLengthPolicy('123456')); + // Au max (12) accepte, au-dela refuse (RG-T18 borne haute). + self::assertTrue($verifier->meetsLengthPolicy('123456789012')); + self::assertFalse($verifier->meetsLengthPolicy('1234567890123')); + // Charset : chiffres uniquement ; vide refuse. + self::assertFalse($verifier->meetsLengthPolicy('abcd')); + self::assertFalse($verifier->meetsLengthPolicy('12ab')); + self::assertFalse($verifier->meetsLengthPolicy('')); + } +}