Prevent brute force attacks on password reset by lisafast · Pull Request #1190 · cds-snc/ai-answers

lisafast · 2026-03-20T13:18:10Z

Summary of fixes

┌──────────────┬────────────────────────────────┐
│ Issue │ Fix │
├──────────────┼────────────────────────────────┤
│ In-memory │ auth-rate-limiter.js now uses │
│ limiter │ Mongo-backed persistence (same │
│ doesn't │ as global limiter), with │
│ scale │ memory fallback │
├──────────────┼────────────────────────────────┤
│ Race │ Failed attempts use atomic │
│ condition on │ $inc via findOneAndUpdate; │
│ attempt │ lockout uses updateOne with │
│ counter │ $set │
├──────────────┼────────────────────────────────┤
│ Test │ Rewrote tests to await the │
│ flakiness │ middleware directly — no │
│ (setTimeout) │ timing dependencies │
├──────────────┼────────────────────────────────┤
│ Stale │ Updated auth-send-reset.js │
│ comment │ line 19 comment │
├──────────────┼────────────────────────────────┤
│ Limiter not │ Added │
│ initialized │ initializeAuthRateLimiter() │
│ at startup │ call in server.js startup │
│ │ sequence, after DB connect │
└──────────────┴────────────────────────────────┘

Summary | Résumé

1-3 sentence description of the changed you're proposing, including a link to
a GitHub Issue # or Trello card if applicable.

Description en 1 à 3 phrases de la modification proposée, avec un lien vers le
problème (« issue ») GitHub ou la fiche Trello, le cas échéant.

Test instructions | Instructions pour tester la modification

Sequential steps (1., 2., 3., ...) that describe how to test this change. This
will help a developer test things out without too much detective work. Also,
include any environmental setup steps that aren't in the normal README steps
and/or any time-based elements that this requires.

Étapes consécutives (1., 2., 3., …) qui décrivent la façon de tester la
modification. Elles aideront les développeurs à faire des tests sans avoir à
jouer au détective. Veuillez aussi inclure toutes les étapes de configuration
de l’environnement qui ne font pas partie des étapes normales dans le fichier
README et tout élément temporel requis.

Summary of fixes ┌──────────────┬────────────────────────────────┐ │ Issue │ Fix │ ├──────────────┼────────────────────────────────┤ │ In-memory │ auth-rate-limiter.js now uses │ │ limiter │ Mongo-backed persistence (same │ │ doesn't │ as global limiter), with │ │ scale │ memory fallback │ ├──────────────┼────────────────────────────────┤ │ Race │ Failed attempts use atomic │ │ condition on │ $inc via findOneAndUpdate; │ │ attempt │ lockout uses updateOne with │ │ counter │ $set │ ├──────────────┼────────────────────────────────┤ │ Test │ Rewrote tests to await the │ │ flakiness │ middleware directly — no │ │ (setTimeout) │ timing dependencies │ ├──────────────┼────────────────────────────────┤ │ Stale │ Updated auth-send-reset.js │ │ comment │ line 19 comment │ ├──────────────┼────────────────────────────────┤ │ Limiter not │ Added │ │ initialized │ initializeAuthRateLimiter() │ │ at startup │ call in server.js startup │ │ │ sequence, after DB connect │ └──────────────┴────────────────────────────────┘

api/auth/auth-reset-password.js

sylviamclaughlin · 2026-03-20T14:49:27Z

api/auth/auth-reset-password.js

+    // Check if too many failed attempts — invalidate secret atomically
+    if ((user.resetPasswordAttempts || 0) >= MAX_RESET_ATTEMPTS) {
+      console.warn(`[auth-reset-password][${os.hostname()}] Too many failed attempts, secret invalidated for: ${email}`);
+      await User.updateOne(


After 5 attempts do you invalidate the users password?

No won't invalidate. Instead will block for 30 minutes with an error message displayed. Am working now on adding OL translations for those error messages.

sylviamclaughlin · 2026-03-20T14:51:24Z

api/auth/auth-reset-password.js

+      console.warn(`[auth-reset-password][${os.hostname()}] Invalid or expired TOTP code (attempt ${attempts}/${MAX_RESET_ATTEMPTS})`);
+
+      // If this increment just hit the limit, invalidate the secret now
+      if (updated && updated.resetPasswordAttempts >= MAX_RESET_ATTEMPTS) {


I guess it is quite secure that you invalidate the users password, but do you notify the user about it? There are password guesses all the time so this will make some users passwords invalid. You should at least send a message to the user letting them know

We have very few admin users on sandbox - just our dev team. Will just display a message that they're locked out of password reset for 30 minutes. Working on that message now. Will summarize in new commit.

Here's what changed: - auth-reset-password.js — API responses now include a code field (RESET_LOCKED_OUT, RESET_INVALID_CODE). Lockout message is now generic: "too many failed attempts, please try again later" (no mention of reset links or timing). - AuthService.js — resetPassword attaches err.code from the API response to the thrown Error. - ResetCompletePage.js — Maps error codes to translation keys, falls back to t('reset.complete.error'). - en.json — Added reset.complete.lockedOut and reset.complete.invalidCode. - fr.json — Added matching French translations. No emails, codes, reset links, attempt counts, or lockout timestamps in logs now. Every log message is purely operational — describes what happened without exposing any sensitive data.

lisafast · 2026-03-20T16:32:14Z

Rate limiting (middleware/auth-rate-limiter.js —
new)

Dedicated Mongo-backed rate limiter for auth
endpoints
auth-reset-password: 5 attempts per 15 min per
IP+email
auth-send-reset: 3 attempts per 15 min per
IP+email

Timed lockout (api/auth/auth-reset-password.js)

After 5 failed code attempts, account is locked
for 30 minutes
User sees a translated error message ("Too many
failed attempts. Please try again later.")
Lockout persists even if a new reset email is
requested
Expired lockouts are cleared automatically on next
attempt
Failed attempts use atomic $inc to prevent race
conditions

User enumeration fix
(api/auth/auth-reset-password.js)

Returns generic 401 "invalid or expired code" for
both user-not-found and wrong-code scenarios
(previously returned 404 "user not found")

Fresh secret per request
(api/auth/auth-send-reset.js)

Each reset request generates a new TOTP secret,
invalidating any prior codes

Bilingual error messages
(src/pages/ResetCompletePage.js,
src/locales/en.json, src/locales/fr.json)

API returns error codes (RESET_LOCKED_OUT,
RESET_INVALID_CODE)
Frontend maps codes to translated strings via t()

Sensitive data scrubbed from logs
(auth-reset-password.js, auth-send-reset.js)

Removed email addresses, TOTP codes, reset links,
attempt counts, and lockout timestamps from all
console output

Schema (models/user.js)

Added resetPasswordAttempts (Number) and
resetPasswordLockedUntil (Date) fields

sylviamclaughlin · 2026-03-20T18:45:59Z

api/auth/auth-reset-password.js

+        { _id: user._id },
+        { $set: { resetPasswordSecret: null, resetPasswordAttempts: 0 } }
+      );
+      return res.status(401).json({ success: false, message: 'invalid or expired code' });


Instead of code, I would use password here. Unless users are logging in with a code, which I assume not? I think the "invalid or expired code" would be a confusing message to users.

sylviamclaughlin · 2026-03-20T18:48:46Z

api/auth/auth-reset-password.js

    // Reject literal string "undefined" or "null" from client
    if (code === 'undefined' || code === 'null') {
-      console.warn(`[auth-reset-password][${os.hostname()}] Received invalid code string: ${code}`);
+      console.warn(`[auth-reset-password][${os.hostname()}] Received invalid code string`);


I would replace code with password

Actually this is for the TOTP code

User enters their email on the reset request page

auth-send-reset generates a 6-digit TOTP code and
emails a reset link containing it (e.g.
?email=...&code=123456)

User clicks the link, lands on ResetCompletePage
with code and email pre-filled from the URL

User enters their new password and submits

auth-reset-password receives { email, code,
newPassword } and verifies the TOTP code

This guard catches the
case where the URL was malformed (missing ?code=
param), which would cause the frontend to send the
literal string "undefined".

sylviamclaughlin · 2026-03-20T18:50:19Z

api/auth/auth-reset-password.js

    }

-    console.debug(`[auth-reset-password][${os.hostname()}] Validating TOTP code for: ${email}`);
+    console.debug(`[auth-reset-password][${os.hostname()}] Validating TOTP code`);


Ae you using a TOTP code for sign in? So you are enforcing MFA?

MFA is built, and ready to switch over to enforce it via a settings toggle, which is defaulting to false right now. We just weren't ready to have it enabled.

lisafast · 2026-03-21T19:25:26Z

Created docs/coding-agent-docs/authentication.md
covering:

Roles and activation — admin vs partner,
auto-activation of first user
Sign up — full flow including inactive-by-default
behavior
Sign in — Passport local strategy, noting 2FA is
opt-in (not enforced)
2FA — verification and resend flows
Password reset — both steps end-to-end with all
security controls listed
Sign out — session destruction
Session management — TTL, deserialize active check
Key files — reference table

github-actions · 2026-03-21T19:39:24Z

🧪 AI Answers Review Environment

https://ehckgnbqcxc4nq6szotbdlmcmm0cshus.lambda-url.ca-central-1.on.aws/

lisafast requested a review from HamzaAburaneh March 20, 2026 13:18

lisafast self-assigned this Mar 20, 2026

lisafast added the Security | Sécurité label Mar 20, 2026

github-advanced-security bot found potential problems Mar 20, 2026

View reviewed changes

api/auth/auth-reset-password.js Fixed Show fixed Hide fixed

lisafast requested review from CalvinRodo and sylviamclaughlin March 20, 2026 13:59

sylviamclaughlin reviewed Mar 20, 2026

View reviewed changes

api/auth/auth-reset-password.js Show resolved Hide resolved

sylviamclaughlin reviewed Mar 20, 2026

View reviewed changes

fix: add docs and comments

a2f1a08

fix: error in doc

9a59d2d

lisafast mentioned this pull request Mar 21, 2026

Rate limiting for password reset #1193

Open

Conversation

lisafast commented Mar 20, 2026

Summary | Résumé

Test instructions | Instructions pour tester la modification

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lisafast commented Mar 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sylviamclaughlin Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lisafast commented Mar 21, 2026

Uh oh!

github-actions bot commented Mar 21, 2026

🧪 AI Answers Review Environment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sylviamclaughlin Mar 20, 2026 •

edited

Loading