From 36c02fa3e959e212fabbc38363ccf2e1041b7f0e Mon Sep 17 00:00:00 2001 From: Pauljd Date: Fri, 11 Sep 2026 20:47:15 +0100 Subject: [PATCH] Davidson CI testing & AI refinement --- lib/ticket.php | 67 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/lib/ticket.php b/lib/ticket.php index 3ab7dfd..7d249cc 100644 --- a/lib/ticket.php +++ b/lib/ticket.php @@ -22,7 +22,13 @@ function getTicketSecret(): string function createTicket(int $userId, string $purpose = 'default', ?int $lifetimeSeconds = null): string { $expiresAt = time() + ($lifetimeSeconds ?? TICKET_LIFETIME_SECONDS); - $payload = $userId . '|' . $expiresAt . '|' . $purpose; + // The random nonce is what makes the payload (and therefore the resulting token and + // its hash) unique even when two tickets are minted for the same user/purpose within + // the same second — otherwise identical inputs produce an identical HMAC, and a second + // password-reset ticket in the same second collides with the first's UNIQUE token_hash + // row and throws instead of just minting a second, independently usable link. + $nonce = bin2hex(random_bytes(16)); + $payload = $userId . '|' . $expiresAt . '|' . $purpose . '|' . $nonce; $signature = hash_hmac('sha256', $payload, getTicketSecret()); return base64_encode($payload . '.' . $signature); @@ -48,8 +54,8 @@ function decodeTicket(string $ticket, string $purpose = 'default'): ?int return null; } - $parts = explode('|', $payload, 3); - if (count($parts) !== 3 || !ctype_digit($parts[0]) || !ctype_digit($parts[1])) { + $parts = explode('|', $payload, 4); + if (count($parts) !== 4 || !ctype_digit($parts[0]) || !ctype_digit($parts[1])) { return null; } @@ -82,22 +88,51 @@ function createPasswordResetTicket(PDO $pdo, int $userId): string { ensurePasswordResetTicketsTable($pdo); - // Opportunistic cleanup: every new ticket also sweeps out rows that are long past - // useful (expired for over a day), so this table doesn't grow unbounded without - // needing a separate cron job. - $pdo->prepare('DELETE FROM password_reset_tickets WHERE expires_at < :cutoff') - ->execute(['cutoff' => time() - 86400]); - $token = createTicket($userId, 'password_reset'); + $tokenHash = hash('sha256', $token); + $expiresAt = time() + TICKET_LIFETIME_SECONDS; + $cutoff = time() - 86400; - $stmt = $pdo->prepare('INSERT INTO password_reset_tickets (token_hash, user_id, expires_at) VALUES (:token_hash, :user_id, :expires_at)'); - $stmt->execute([ - 'token_hash' => hash('sha256', $token), - 'user_id' => $userId, - 'expires_at' => time() + TICKET_LIFETIME_SECONDS, - ]); + // The cleanup delete and the insert run as one transaction so they can't be split by + // a concurrent writer. Retried on a transient SQLITE_BUSY: two password-reset requests + // landing at nearly the same instant can still surface "database is locked" here even + // with the connection's busy_timeout, so back off briefly and retry rather than let it + // bubble up as an uncaught 500. + $maxAttempts = 3; + for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { + try { + $pdo->exec('BEGIN IMMEDIATE'); - return $token; + // Opportunistic cleanup: every new ticket also sweeps out rows that are long + // past useful (expired for over a day), so this table doesn't grow unbounded + // without needing a separate cron job. + $pdo->prepare('DELETE FROM password_reset_tickets WHERE expires_at < :cutoff') + ->execute(['cutoff' => $cutoff]); + + $stmt = $pdo->prepare('INSERT INTO password_reset_tickets (token_hash, user_id, expires_at) VALUES (:token_hash, :user_id, :expires_at)'); + $stmt->execute([ + 'token_hash' => $tokenHash, + 'user_id' => $userId, + 'expires_at' => $expiresAt, + ]); + + $pdo->exec('COMMIT'); + + return $token; + } catch (PDOException $e) { + if ($pdo->inTransaction()) { + $pdo->exec('ROLLBACK'); + } + + if ($attempt === $maxAttempts || !str_contains($e->getMessage(), 'database is locked')) { + throw $e; + } + + usleep(50000 * $attempt); + } + } + + throw new RuntimeException('Unreachable'); } // Validates signature/purpose/expiry only. Safe to call on GET to decide page