AcadeNice/corentin_wakdo
7
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(admin): chemin d'echec PIN atomique (pin.failed + throttle dans 1 transaction)
All checks were successful
CI / php-lint (pull_request) Successful in 25s
Details
CI / static-tests (push) Successful in 37s
Details
CI / auto-merge (pull_request) Successful in 7s
Details
CI / secret-scan (pull_request) Successful in 11s
Details
CI / auto-merge (push) Has been skipped
Details
CI / static-tests (pull_request) Successful in 41s
Details
CI / secret-scan (push) Successful in 8s
Details
CI / php-lint (push) Successful in 24s
Details

Browse source 
Sur un PIN invalide, ProductController ecrivait la trace audit pin.failed
(autocommit) PUIS appelait PinThrottle::recordFailure (qui ouvrait SA propre
transaction) : deux ecritures non atomiques, un crash entre les deux laissait un
etat partiel, en tension avec le claim RG-T08 (audit dans la meme transaction que
l'effet).

- PinThrottle : extraction de recordFailureWithin(db, ...) (memes effets, SANS
  transaction propre) ; recordFailure() reste l'API autonome (l'enveloppe).
- ProductController (update + destroy) : les chemins d'echec PIN enveloppent
  logFailedPin(, ...) + recordFailureWithin(, ...) dans UNE transaction.
- logFailedPin prend desormais le  de la transaction.

Aucun changement de test necessaire (les assertions audit/throttle tiennent dans
la transaction). 188 verts, PinThrottleDbTest 2/2 contre la vraie DB, PHPStan L6.
This commit is contained in:
Imugiii 2026-06-16 12:15:21 +00:00
parent ad5203d3fc
commit a3fa72cf86
2 changed files with 70 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

86
src/app/Auth/PinThrottle.php
View file

@ -75,47 +75,67 @@ final class PinThrottle
return; return;
} }
// Variante autonome : ouvre sa propre transaction. Le controleur, lui,
// prefere recordFailureWithin() pour ecrire la trace pin.failed et cet
// increment dans UNE SEULE transaction (RG-T08).
$this->db->transaction(function (DatabaseInterface $db) use ($actorUserId, $now): void {
$this->recordFailureWithin($db, $actorUserId, $now);
});
}
/**
* Variante SANS transaction propre : suppose que l'appelant a deja ouvert une
* transaction (le controleur enveloppe la trace audit pin.failed (RG-T14) et
* cet increment dans la meme, RG-T08 : pas d'etat partiel si crash entre les
* deux). Memes effets que recordFailure : upsert atomique sous verrou de ligne,
* fenetre glissante reinitialisee en SQL, backoff degressif. Ne touche jamais
* user ni login_throttle (RG-T22).
*/
public function recordFailureWithin(DatabaseInterface $db, int $actorUserId, ?int $now = null): void
{
if ($actorUserId <= 0) {
return;
}
$now ??= time(); $now ??= time();
$nowDt = date('Y-m-d H:i:s', $now); $nowDt = date('Y-m-d H:i:s', $now);
$windowSeconds = $this->config->int('PIN_THROTTLE_WINDOW_SECONDS', 900); $windowSeconds = $this->config->int('PIN_THROTTLE_WINDOW_SECONDS', 900);
$windowCutoff = date('Y-m-d H:i:s', $now - $windowSeconds); $windowCutoff = date('Y-m-d H:i:s', $now - $windowSeconds);
$policy = ThrottlePolicy::fromConfig($this->config, 'pin'); $policy = ThrottlePolicy::fromConfig($this->config, 'pin');
$this->db->transaction(function (DatabaseInterface $db) use ($actorUserId, $nowDt, $windowCutoff, $policy, $now): void { // Increment ATOMIQUE cote SQL sous le verrou de ligne pris par l'upsert
// Increment ATOMIQUE cote SQL sous le verrou de ligne pris par l'upsert // (anti lost-update sous POSTs concurrents). Placeholders distincts : en
// (anti lost-update sous POSTs concurrents). Placeholders distincts : en // prepare reelle (EMULATE_PREPARES = false) un meme nom ne peut etre lie
// prepare reelle (EMULATE_PREPARES = false) un meme nom ne peut etre lie // qu'une fois. Meme forme que AuthService (dimension IP).
// qu'une fois. Meme forme que AuthService (dimension IP). $db->execute(
$db->execute( 'INSERT INTO pin_throttle (actor_user_id, failed_attempts, window_started_at, last_attempt_at) '
'INSERT INTO pin_throttle (actor_user_id, failed_attempts, window_started_at, last_attempt_at) ' . 'VALUES (:uid, 1, :now_i, :now_li) '
. 'VALUES (:uid, 1, :now_i, :now_li) ' . 'ON DUPLICATE KEY UPDATE '
. 'ON DUPLICATE KEY UPDATE ' . 'failed_attempts = IF(window_started_at < :cutoff, 1, failed_attempts + 1), '
. 'failed_attempts = IF(window_started_at < :cutoff, 1, failed_attempts + 1), ' . 'window_started_at = IF(window_started_at < :cutoff2, :now_w, window_started_at), '
. 'window_started_at = IF(window_started_at < :cutoff2, :now_w, window_started_at), ' . 'last_attempt_at = :now_lu',
. 'last_attempt_at = :now_lu', [
[ 'uid' => $actorUserId,
'uid' => $actorUserId, 'now_i' => $nowDt,
'now_i' => $nowDt, 'now_li' => $nowDt,
'now_li' => $nowDt, 'cutoff' => $windowCutoff,
'cutoff' => $windowCutoff, 'cutoff2' => $windowCutoff,
'cutoff2' => $windowCutoff, 'now_w' => $nowDt,
'now_w' => $nowDt, 'now_lu' => $nowDt,
'now_lu' => $nowDt, ],
], );
);
// Relit le compteur autoritaire (ligne deja verrouillee par cette tx) // Relit le compteur autoritaire (ligne deja verrouillee par cette tx)
// pour calculer le backoff en PHP, puis pose le verrou. // pour calculer le backoff en PHP, puis pose le verrou.
$row = $db->fetch('SELECT failed_attempts FROM pin_throttle WHERE actor_user_id = :uid', ['uid' => $actorUserId]); $row = $db->fetch('SELECT failed_attempts FROM pin_throttle WHERE actor_user_id = :uid', ['uid' => $actorUserId]);
$attempts = (int) ($row['failed_attempts'] ?? 1); $attempts = (int) ($row['failed_attempts'] ?? 1);
$lockSeconds = $policy->lockoutSeconds($attempts); $lockSeconds = $policy->lockoutSeconds($attempts);
$lockUntil = $lockSeconds > 0 ? date('Y-m-d H:i:s', $now + $lockSeconds) : null; $lockUntil = $lockSeconds > 0 ? date('Y-m-d H:i:s', $now + $lockSeconds) : null;
$db->execute( $db->execute(
'UPDATE pin_throttle SET lockout_until = :lock WHERE actor_user_id = :uid', 'UPDATE pin_throttle SET lockout_until = :lock WHERE actor_user_id = :uid',
['lock' => $lockUntil, 'uid' => $actorUserId], ['lock' => $lockUntil, 'uid' => $actorUserId],
); );
});
} }
/** /**

23
src/app/Controllers/ProductController.php
View file

@ -157,8 +157,14 @@ class ProductController extends AdminController
$actor = $this->pinVerifier()->resolveActingUser(trim($form['pin_email'] ?? ''), $form['pin'] ?? ''); $actor = $this->pinVerifier()->resolveActingUser(trim($form['pin_email'] ?? ''), $form['pin'] ?? '');
if ($actor === null) { if ($actor === null) {
$this->logFailedPin(trim($form['pin_email'] ?? ''), $id); // RG-T08 : la trace pin.failed (RG-T14) et l'increment du throttle
$this->pinThrottle()->recordFailure($actorId); // (RG-T22) sont ecrits dans UNE meme transaction (pas d'etat partiel
// si crash entre les deux ecritures).
$email = trim($form['pin_email'] ?? '');
$this->db()->transaction(function (DatabaseInterface $db) use ($email, $id, $actorId): void {
$this->logFailedPin($db, $email, $id);
$this->pinThrottle()->recordFailureWithin($db, $actorId);
});
return $this->renderForm($guard, $id, $form, ['pin' => 'Email ou PIN invalide (requis pour modifier prix/TVA).'], 422); return $this->renderForm($guard, $id, $form, ['pin' => 'Email ou PIN invalide (requis pour modifier prix/TVA).'], 422);
} }
@ -232,8 +238,13 @@ class ProductController extends AdminController
$actor = $this->pinVerifier()->resolveActingUser(trim($form['pin_email'] ?? ''), $form['pin'] ?? ''); $actor = $this->pinVerifier()->resolveActingUser(trim($form['pin_email'] ?? ''), $form['pin'] ?? '');
if ($actor === null) { if ($actor === null) {
$this->logFailedPin(trim($form['pin_email'] ?? ''), $id); // RG-T08 : trace pin.failed (RG-T14) + increment throttle (RG-T22) dans
$this->pinThrottle()->recordFailure($actorId); // UNE meme transaction (pas d'etat partiel si crash entre les deux).
$email = trim($form['pin_email'] ?? '');
$this->db()->transaction(function (DatabaseInterface $db) use ($email, $id, $actorId): void {
$this->logFailedPin($db, $email, $id);
$this->pinThrottle()->recordFailureWithin($db, $actorId);
});
return $this->renderDelete($guard, $id, $product, 'Email ou PIN invalide (requis pour supprimer).'); return $this->renderDelete($guard, $id, $product, 'Email ou PIN invalide (requis pour supprimer).');
} }
@ -376,9 +387,9 @@ class ProductController extends AdminController
* echecs ayant arme le verrou sont deja audites), ce qui borne l'amplification * echecs ayant arme le verrou sont deja audites), ce qui borne l'amplification
* de l'audit append-only (RG-T14). * de l'audit append-only (RG-T14).
*/ */
private function logFailedPin(string $email, int $productId): void private function logFailedPin(DatabaseInterface $db, string $email, int $productId): void
{ {
$this->db()->execute( $db->execute(
'INSERT INTO audit_log (actor_user_id, actor_role_id, action_code, entity_type, entity_id, summary) ' 'INSERT INTO audit_log (actor_user_id, actor_role_id, action_code, entity_type, entity_id, summary) '
. 'VALUES (:uid, :rid, :code, :etype, :eid, :summary)', . 'VALUES (:uid, :rid, :code, :etype, :eid, :summary)',
[ [