Harden sign-in and password reset

- OTP attempt limits, constant-time compare, random reference codes
- DB-backed rate limits (429) on sign-in, OTP, reset, register, onboarding
- one generic sign-in failure message; reset request no longer reveals accounts
- no password kept in the session; real status codes on failures
This commit is contained in:
Thanakorn
2026-09-24 14:53:40 +07:00
parent ae98dcdcdd
commit 73c680e844
16 changed files with 538 additions and 282 deletions
@@ -14,20 +14,25 @@
*
* The OTP is a 6-digit TOTP derived from the user's current password hash via HMAC-SHA1,
* scoped to a 3-minute time step. It cannot be replayed after the window expires.
* A human-readable reference number (6 uppercase letters) is also generated and emailed
* so the user can confirm they received the correct OTP request.
* A random reference number (6 uppercase letters) is also generated and emailed so the
* user can confirm they received the correct OTP request. It is not derived from the
* OTP: a derived reference let anyone who saw it recover the OTP offline.
*
* HTTP handler methods for thin AJAX endpoint wrappers:
* handleRequestOtp($user_id, $company_id)
* handleRequestOtp($user_id, $company_id) — signed-in profile page
* handleRequestOtpPublic($user_id, $company_id) — login page; same answer whether
* or not the account exists
* handleConfirmReset($user_id, $data)
*
* Session keys used (prefixed with 'reset_' to avoid collision with login OTP):
* reset_otp, reset_otp_time, reset_reference, reset_user_id
* reset_otp, reset_otp_time, reset_reference, reset_user_id, reset_attempts
*
* Security:
* - OTP is HMAC-derived from the current password hash — it changes when the password changes.
* - OTP is valid for OTP_EXPIRY_MINUTES (5) only; older OTPs are rejected with clearSession().
* - reset_user_id in session is verified against $user_id to prevent cross-user OTP reuse.
* - At most OTP_MAX_ATTEMPTS wrong entries per issued OTP, then it is discarded.
* - OTPs are compared with hash_equals().
* - Session is fully destroyed on successful reset, forcing re-authentication.
* - All DB queries use PDO prepared statements with bound parameters.
* - AJAX handler methods output JSON via json_encode (XSS-safe).
@@ -43,6 +48,12 @@ class PasswordResetManager {
/** OTP validity window in minutes — matches the login OTP window. */
const OTP_EXPIRY_MINUTES = 5;
/** Wrong OTP entries allowed per issued OTP before it is discarded. */
const OTP_MAX_ATTEMPTS = 5;
/** Answer shown on the login page whether or not the account exists. */
const PUBLIC_REQUEST_MESSAGE = "If an account matches, we've sent an OTP to its email.";
/**
* @param PDO $pdo1 PDO connection to the wms database (user table).
* @param PDO $pdo2 PDO connection to the company database (smtp_setting table).
@@ -94,10 +105,10 @@ class PasswordResetManager {
throw new \RuntimeException('No email address found for this account.');
}
// Generate 6-digit TOTP and a human-readable 6-letter reference number
// Generate 6-digit TOTP and a random 6-letter reference number
$otp_time = time();
$otp = $this->generateOTP($user['password'], $otp_time);
$reference_number = $this->numberToLetters((int) $this->generateOTP($otp, $otp_time));
$reference_number = $this->randomReference();
// Send via the mailer module (uses company SMTP or falls back to system default)
require_once $this->include_url . '/assets/utils/module/mailer.php';
@@ -124,6 +135,7 @@ class PasswordResetManager {
$_SESSION['reset_otp_time'] = $otp_time;
$_SESSION['reset_reference'] = $reference_number;
$_SESSION['reset_user_id'] = $user_id;
$_SESSION['reset_attempts'] = 0;
return [
'masked_email' => $this->maskEmail($user['email']),
@@ -131,6 +143,24 @@ class PasswordResetManager {
];
}
/**
* Start a reset that can never succeed, for a login-page request whose
* username/email matches no account. The session then looks exactly like a
* real request (random unguessable OTP, reset_user_id 0), so the confirm step
* answers "Incorrect OTP" instead of revealing that the account is missing.
*
* @return string Random 6-letter reference, same shape as a real one.
*/
public function startDecoy(): string {
$reference = $this->randomReference();
$_SESSION['reset_otp'] = bin2hex(random_bytes(16));
$_SESSION['reset_otp_time'] = time();
$_SESSION['reset_reference'] = $reference;
$_SESSION['reset_user_id'] = 0;
$_SESSION['reset_attempts'] = 0;
return $reference;
}
/**
* Verify the OTP and force-set a new password via PasswordManager.
*
@@ -170,8 +200,14 @@ class PasswordResetManager {
throw new \InvalidArgumentException('OTP has expired. Please request a new one.');
}
// Verify OTP value
if (trim($otp_input) !== $_SESSION['reset_otp']) {
// Verify OTP value — at most OTP_MAX_ATTEMPTS wrong entries per issued OTP,
// so the 6-digit code cannot be brute-forced inside its 5-minute window.
if (!hash_equals((string)$_SESSION['reset_otp'], trim($otp_input)) || $user_id <= 0) {
$_SESSION['reset_attempts'] = (int)($_SESSION['reset_attempts'] ?? 0) + 1;
if ($_SESSION['reset_attempts'] >= self::OTP_MAX_ATTEMPTS) {
$this->clearSession();
throw new \InvalidArgumentException('Too many incorrect OTP attempts. Please request a new OTP.');
}
throw new \InvalidArgumentException('Incorrect OTP. Please try again.');
}
@@ -234,6 +270,39 @@ class PasswordResetManager {
exit;
}
/**
* Handle the login-page request-OTP call. The answer is the same whether or
* not the username/email matches an account (no account enumeration): no
* masked email, a generic message and a reference number. Mail failures are
* logged, not reported, for the same reason.
*
* On success (always): { success: 1, message: PUBLIC_REQUEST_MESSAGE, reference: "ABCDEF" }
*
* @param int|null $user_id Resolved account, or null when nothing matched.
* @param int $company_id Company SMTP scope (0 = use system default).
*/
public function handleRequestOtpPublic(?int $user_id, int $company_id = 0): void {
$reference = null;
if ($user_id) {
try {
$reference = $this->requestOtp($user_id, $company_id)['reference'];
} catch (\Exception $e) {
error_log('[PasswordResetManager::handleRequestOtpPublic] ' . $e->getMessage());
}
}
if ($reference === null) {
$reference = $this->startDecoy();
}
echo json_encode([
'success' => 1,
'message' => self::PUBLIC_REQUEST_MESSAGE,
'reference' => $reference,
]);
exit;
}
/**
* Handle an AJAX confirm-reset call and echo a JSON response.
*
@@ -312,23 +381,17 @@ class PasswordResetManager {
}
/**
* Convert a positive integer into a base-26 uppercase letter string.
* Random 6-letter uppercase reference code (e.g. "BCDFHJ") for the reset email
* and the confirmation screen. Carries no information about the OTP.
*
* Used to turn the numeric reference OTP into a human-friendly 6-letter
* reference code (e.g. 123456 → "BCDFHJ") for inclusion in the reset email.
* The result is left-padded with 'A' to always return a 6-character string.
*
* @param int $num Positive integer to convert.
* @return string 6-character uppercase string (e.g. "AAAABC").
* @return string 6-character uppercase string.
*/
private function numberToLetters(int $num): string {
private function randomReference(): string {
$result = '';
while ($num > 0) {
$mod = ($num - 1) % 26;
$result = chr(65 + $mod) . $result;
$num = intval(($num - $mod) / 26);
for ($i = 0; $i < 6; $i++) {
$result .= chr(65 + random_int(0, 25));
}
return str_pad($result, 6, 'A', STR_PAD_LEFT);
return $result;
}
/**
@@ -356,14 +419,15 @@ class PasswordResetManager {
*
* Called on OTP expiry (to invalidate the request) and on successful
* reset (before session_destroy). Does not destroy the full session —
* only the 4 reset-specific keys are unset.
* only the reset-specific keys are unset.
*/
private function clearSession(): void {
unset(
$_SESSION['reset_otp'],
$_SESSION['reset_otp_time'],
$_SESSION['reset_reference'],
$_SESSION['reset_user_id']
$_SESSION['reset_user_id'],
$_SESSION['reset_attempts']
);
}
}