feat: add forgot password & reset password flow - #68
Conversation
- Add password_reset_tokens table to schema - POST /api/auth/forgot-password — rate-limited, no email enumeration - POST /api/auth/reset-password — token hash validation, bcrypt, atomic tx - /forgot-password page — email input with success/error feedback - /reset-password page — new password + confirm with auto-redirect - Add 'Forgot Password?' link to login page (existing-user mode) Security: SHA-256 token hashing, 1hr expiry, one-time use, rate limiting
📝 WalkthroughWalkthroughAdded a password reset feature: new database table for token storage, two backend API endpoints to request and consume reset tokens (with rate limiting, hashing, expiry, and transactional updates), and frontend pages/UI to request a reset and set a new password. Login page gains a "Forgot Password?" link. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ForgotPage as Forgot Password Page
participant API_Forgot as /api/auth/forgot-password
participant DB as Database
participant Email as Email Service
participant ResetPage as Reset Password Page
participant API_Reset as /api/auth/reset-password
User->>ForgotPage: Submit email
ForgotPage->>API_Forgot: POST { email }
API_Forgot->>API_Forgot: Rate limit check
API_Forgot->>DB: Query user by email
alt user exists
API_Forgot->>API_Forgot: Generate token, hash (SHA-256)
API_Forgot->>DB: Mark prior unused tokens used
API_Forgot->>DB: Insert token_hash with expires_at
API_Forgot->>Email: Send reset link (includes raw token)
end
API_Forgot-->>ForgotPage: 200 (generic success)
User->>ResetPage: Open link with token
ResetPage->>ResetPage: Extract token from URL
User->>ResetPage: Submit new password
ResetPage->>API_Reset: POST { token, password }
API_Reset->>API_Reset: Rate limit check
API_Reset->>API_Reset: Hash token (SHA-256)
API_RESET->>DB: Query token + user
alt token valid & not expired & not used
API_Reset->>API_Reset: Hash password (bcrypt)
API_Reset->>DB: BEGIN TRANSACTION
API_Reset->>DB: Update users.password_hash
API_Reset->>DB: Mark token used and other tokens used
API_Reset->>DB: COMMIT
API_Reset-->>ResetPage: 200 success
else invalid/expired/used
API_Reset-->>ResetPage: 400 error (specific)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a complete “forgot password / reset password” flow across UI, API, and DB to support password recovery for credential-based accounts.
Changes:
- Added
/forgot-passwordand/reset-passwordpages with form validation + status feedback. - Added API routes for forgot/reset password with token hashing + expiry + rate limiting.
- Added
password_reset_tokenstable to the schema and linked the flow from the login page.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/pages/forgot-password/index.js |
New UI page to request a reset link via email. |
src/pages/reset-password/index.js |
New UI page to set a new password using a token from the reset link. |
src/pages/login/index.js |
Adds “Forgot Password?” link in existing-user login mode. |
src/pages/api/auth/forgot-password.js |
Generates/stores reset token hash and sends reset email (rate-limited). |
src/pages/api/auth/reset-password.js |
Validates token + expiry and updates password, marking token(s) as used. |
schema.sql |
Adds password_reset_tokens table + indexes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| -- Password reset tokens | ||
| CREATE TABLE password_reset_tokens ( | ||
| id BIGSERIAL PRIMARY KEY, | ||
| user_id BIGINT NOT NULL, | ||
| token_hash VARCHAR(128) NOT NULL, | ||
| expires_at TIMESTAMP WITH TIME ZONE NOT NULL, | ||
| used BOOLEAN DEFAULT FALSE, | ||
| created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, | ||
| FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE | ||
| ); | ||
|
|
||
| CREATE INDEX idx_password_reset_tokens_hash ON password_reset_tokens(token_hash); | ||
| CREATE INDEX idx_password_reset_tokens_user_id ON password_reset_tokens(user_id); | ||
|
|
There was a problem hiding this comment.
This change updates the root schema.sql but doesn’t add a numbered migration in src/services/database/migrations/ (used by the migration runner) and also leaves src/services/database/schema.sql without this table, so schema sources will diverge. Add the next migration (e.g. 009_add_password_reset_tokens.sql) and keep both schema files in sync per the DB docs/README.
| -- Password reset tokens | |
| CREATE TABLE password_reset_tokens ( | |
| id BIGSERIAL PRIMARY KEY, | |
| user_id BIGINT NOT NULL, | |
| token_hash VARCHAR(128) NOT NULL, | |
| expires_at TIMESTAMP WITH TIME ZONE NOT NULL, | |
| used BOOLEAN DEFAULT FALSE, | |
| created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, | |
| FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE | |
| ); | |
| CREATE INDEX idx_password_reset_tokens_hash ON password_reset_tokens(token_hash); | |
| CREATE INDEX idx_password_reset_tokens_user_id ON password_reset_tokens(user_id); |
| used BOOLEAN DEFAULT FALSE, | ||
| created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, |
There was a problem hiding this comment.
used and created_at are nullable as defined here; that can lead to ambiguous semantics (NULL vs FALSE) and makes queries more error-prone. Consider used BOOLEAN NOT NULL DEFAULT FALSE and created_at TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP (and optionally a UNIQUE constraint/index on token_hash since the API assumes one row per hash).
| used BOOLEAN DEFAULT FALSE, | |
| created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, | |
| used BOOLEAN NOT NULL DEFAULT FALSE, | |
| created_at TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT CURRENT_TIMESTAMP, |
| <InputAdornment position='end'> | ||
| <IconButton size='small' onClick={() => setShowPassword(!showPassword)} edge='end'> | ||
| {showPassword ? <Icon icon='tabler:eye-off' fontSize={17} /> : <Icon icon='tabler:eye' fontSize={17} />} | ||
| </IconButton> |
There was a problem hiding this comment.
The password visibility IconButtons don’t have accessible labels. Add aria-label (and ideally aria-pressed) so screen readers can announce the control purpose/state.
| all: 'unset', display: 'inline', cursor: 'pointer', | ||
| color: 'primary.main', fontWeight: 700, fontSize: 'inherit', fontFamily: 'inherit', | ||
| '&:hover, &:focus-visible': { textDecoration: 'underline' } |
There was a problem hiding this comment.
This button-styled Box removes all default button styles and only adds underline on focus; unlike similar buttons elsewhere (e.g., login/forgot pages), it lacks a visible focus outline, which hurts keyboard accessibility. Add a :focus-visible outline (or use MUI Link/Button) for consistent focus treatment.
| all: 'unset', display: 'inline', cursor: 'pointer', | |
| color: 'primary.main', fontWeight: 700, fontSize: 'inherit', fontFamily: 'inherit', | |
| '&:hover, &:focus-visible': { textDecoration: 'underline' } | |
| all: 'unset', | |
| display: 'inline', | |
| cursor: 'pointer', | |
| color: 'primary.main', | |
| fontWeight: 700, | |
| fontSize: 'inherit', | |
| fontFamily: 'inherit', | |
| '&:hover': { textDecoration: 'underline' }, | |
| '&:focus-visible': { | |
| textDecoration: 'underline', | |
| outline: '2px solid', | |
| outlineColor: 'primary.main', | |
| outlineOffset: 2, | |
| borderRadius: 1 | |
| } |
| } catch (err) { | ||
| console.error('[ForgotPassword] Error:', err) | ||
|
|
||
| return res.status(500).json({ error: 'Something went wrong. Please try again later.' }) | ||
| } |
There was a problem hiding this comment.
The catch path returns a 500, which breaks the “always returns 200 to avoid email enumeration” guarantee. If SMTP/sendMail fails only when a user exists (because unknown emails return early), an attacker can infer account existence via 200 vs 500; consider always returning the generic 200 response after validation and logging/queuing the email send failure instead.
| <h2 style="margin:0 0 12px;font-size:18px;color:#1a1a2e;">Password Reset</h2> | ||
| <p style="margin:0 0 20px;font-size:14px;line-height:1.6;color:#555;"> | ||
| Hi <strong>${name}</strong>, we received a request to reset your password. | ||
| Click the button below to choose a new one. This link expires in <strong>1 hour</strong>. | ||
| </p> |
There was a problem hiding this comment.
The HTML email template interpolates name directly into HTML. Since users.name can contain arbitrary characters, this can break markup or enable HTML injection in email clients; escape the name (and any other user-controlled fields) before embedding in HTML (see existing HTML escaping patterns in the repo).
| const resetRecord = await dbOneOrNone( | ||
| `SELECT prt.id, prt.user_id, prt.expires_at, prt.used, u.email | ||
| FROM password_reset_tokens prt | ||
| JOIN users u ON u.id = prt.user_id | ||
| WHERE prt.token_hash = $1`, | ||
| [tokenHash] | ||
| ) | ||
|
|
||
| if (!resetRecord) { | ||
| return res.status(400).json({ error: 'Invalid or expired reset link. Please request a new one.' }) | ||
| } | ||
|
|
||
| if (resetRecord.used) { | ||
| return res.status(400).json({ error: 'This reset link has already been used. Please request a new one.' }) | ||
| } | ||
|
|
||
| if (new Date(resetRecord.expires_at) < new Date()) { | ||
| return res.status(400).json({ error: 'This reset link has expired. Please request a new one.' }) | ||
| } | ||
|
|
||
| // Hash the new password | ||
| const passwordHash = await bcrypt.hash(password, 12) | ||
|
|
||
| // Update password & mark token used in a transaction | ||
| await dbTx(async t => { | ||
| await t.none( | ||
| 'UPDATE users SET password_hash = $1, updated_at = NOW() WHERE id = $2', | ||
| [passwordHash, resetRecord.user_id] | ||
| ) | ||
| await t.none( | ||
| 'UPDATE password_reset_tokens SET used = TRUE WHERE id = $1', | ||
| [resetRecord.id] | ||
| ) | ||
| // Invalidate all other unused tokens for this user | ||
| await t.none( | ||
| 'UPDATE password_reset_tokens SET used = TRUE WHERE user_id = $1 AND id != $2 AND used = FALSE', | ||
| [resetRecord.user_id, resetRecord.id] | ||
| ) |
There was a problem hiding this comment.
Reset token validation isn’t atomic: the code reads used/expires_at outside the transaction and then unconditionally updates the token row, so two concurrent requests can both pass validation and both reset the password before either marks the token used. Make the update conditional (WHERE used = FALSE AND expires_at > NOW()) and verify rowCount/RETURNING, or SELECT … FOR UPDATE inside the tx.
| // Redirect to login after 3 seconds | ||
| setTimeout(() => router.push('/login'), 3000) | ||
| } else { |
There was a problem hiding this comment.
setTimeout is scheduled on success without cleanup, so navigating away before it fires can trigger a state update/navigation after unmount. Store the timeout id and clear it in a useEffect cleanup (or redirect immediately once serverMsg is set).
| // Invalidate any existing unused tokens for this user | ||
| await dbNone( | ||
| 'UPDATE password_reset_tokens SET used = TRUE WHERE user_id = $1 AND used = FALSE', | ||
| [user.id] | ||
| ) | ||
|
|
||
| // Generate a cryptographically secure token | ||
| const rawToken = crypto.randomBytes(32).toString('hex') | ||
|
|
||
| // Store only the SHA-256 hash in DB | ||
| const tokenHash = crypto.createHash('sha256').update(rawToken).digest('hex') | ||
|
|
||
| // Token expires in 1 hour | ||
| const expiresAt = new Date(Date.now() + 60 * 60 * 1000) | ||
|
|
||
| await dbNone( | ||
| 'INSERT INTO password_reset_tokens (user_id, token_hash, expires_at) VALUES ($1, $2, $3)', | ||
| [user.id, tokenHash, expiresAt] | ||
| ) |
There was a problem hiding this comment.
Token invalidation + insert isn’t atomic: concurrent forgot-password requests for the same user can both mark old tokens used and then each insert a new unused token, leaving multiple valid reset links. Wrap the invalidate+insert in a single dbTx with row locking, or enforce a partial unique constraint (e.g., one unused token per user) and handle conflicts deterministically.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/pages/reset-password/index.js (1)
72-73: Consider clearing the redirect timeout on unmount.If the user navigates away before the 3-second timeout completes, the
router.push('/login')will still fire, potentially causing unexpected navigation. This is a minor edge case.Proposed fix using useEffect for cleanup
Add a ref to track the timeout and clear it on unmount:
// At the top of the component const redirectTimerRef = useRef(null) // In handleSubmit success block redirectTimerRef.current = setTimeout(() => router.push('/login'), 3000) // Add cleanup effect useEffect(() => { return () => { if (redirectTimerRef.current) clearTimeout(redirectTimerRef.current) } }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/reset-password/index.js` around lines 72 - 73, The timeout started with setTimeout(() => router.push('/login'), 3000) can fire after the component unmounts causing unexpected navigation; fix by using a ref (e.g., redirectTimerRef via useRef) to store the timeout id when setting it inside handleSubmit's success branch and add a cleanup effect (useEffect with empty deps) that clears the timeout (clearTimeout(redirectTimerRef.current)) on unmount so router.push('/login') won't run after the component has been torn down.schema.sql (1)
187-187: Consider adding a UNIQUE constraint ontoken_hash.While SHA-256 collisions are cryptographically improbable, adding a UNIQUE constraint on
token_hashprovides defense-in-depth and ensures data integrity at the database level. It also makes the lookup query more efficient.Proposed change
- token_hash VARCHAR(128) NOT NULL, + token_hash VARCHAR(128) NOT NULL UNIQUE,If using UNIQUE on the column, the separate index on
token_hash(line 194) becomes redundant and can be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema.sql` at line 187, Add a UNIQUE constraint to the token_hash column (e.g., change token_hash VARCHAR(128) NOT NULL to include UNIQUE or add a table-level UNIQUE (token_hash)) to enforce uniqueness at the DB level and improve lookup performance; if you add UNIQUE, remove the separate token_hash index defined later (the redundant index on token_hash) to avoid duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/api/auth/forgot-password.js`:
- Line 108: Escape the user-provided name before inserting it into the HTML
email template to prevent HTML injection: add or reuse a small HTML-escaping
helper (e.g., escapeHtml) and call it when building the template instead of
interpolating raw name; update the email creation/site-mailing code in
src/pages/api/auth/forgot-password.js (the template that contains Hi
<strong>${name}</strong>) to use escapeHtml(name) and ensure the helper properly
replaces &, <, >, ", and ' characters.
- Around line 35-38: The emailRegex validation in forgot-password.js (emailRegex
and trimmedEmail) can be exploited for ReDoS; update the handler to first
enforce a strict length constraint on trimmedEmail (e.g., max 254 chars and a
reasonable min) before running any regex, or replace the custom regex with a
well-tested linear-time validator (e.g., validator.isEmail) and use that
instead; ensure you update the validation branch that currently returns
res.status(400).json({ error: 'Invalid email format' }) to handle the new length
check and/or validator call and import the validator module if used.
---
Nitpick comments:
In `@schema.sql`:
- Line 187: Add a UNIQUE constraint to the token_hash column (e.g., change
token_hash VARCHAR(128) NOT NULL to include UNIQUE or add a table-level UNIQUE
(token_hash)) to enforce uniqueness at the DB level and improve lookup
performance; if you add UNIQUE, remove the separate token_hash index defined
later (the redundant index on token_hash) to avoid duplication.
In `@src/pages/reset-password/index.js`:
- Around line 72-73: The timeout started with setTimeout(() =>
router.push('/login'), 3000) can fire after the component unmounts causing
unexpected navigation; fix by using a ref (e.g., redirectTimerRef via useRef) to
store the timeout id when setting it inside handleSubmit's success branch and
add a cleanup effect (useEffect with empty deps) that clears the timeout
(clearTimeout(redirectTimerRef.current)) on unmount so router.push('/login')
won't run after the component has been torn down.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a49f1f5-570d-475a-b230-285fe64c3adf
📒 Files selected for processing (6)
schema.sqlsrc/pages/api/auth/forgot-password.jssrc/pages/api/auth/reset-password.jssrc/pages/forgot-password/index.jssrc/pages/login/index.jssrc/pages/reset-password/index.js
| <tr><td style="padding:32px;"> | ||
| <h2 style="margin:0 0 12px;font-size:18px;color:#1a1a2e;">Password Reset</h2> | ||
| <p style="margin:0 0 20px;font-size:14px;line-height:1.6;color:#555;"> | ||
| Hi <strong>${name}</strong>, we received a request to reset your password. |
There was a problem hiding this comment.
Escape user-provided name in HTML email template.
The name value is interpolated directly into HTML without escaping. If a user registered with a name containing HTML characters (e.g., <script> or &), it could cause rendering issues or, in edge cases, be exploited in email clients that don't fully sanitize HTML.
Proposed fix: Add HTML escaping helper
+function escapeHtml(str) {
+ return str
+ .replace(/&/g, '&')
+ .replace(/</g, '<')
+ .replace(/>/g, '>')
+ .replace(/"/g, '"')
+}
+
function buildResetEmailHtml(name, resetUrl) {
+ const safeName = escapeHtml(name)
return `
...
- Hi <strong>${name}</strong>, we received a request to reset your password.
+ Hi <strong>${safeName}</strong>, we received a request to reset your password.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/api/auth/forgot-password.js` at line 108, Escape the user-provided
name before inserting it into the HTML email template to prevent HTML injection:
add or reuse a small HTML-escaping helper (e.g., escapeHtml) and call it when
building the template instead of interpolating raw name; update the email
creation/site-mailing code in src/pages/api/auth/forgot-password.js (the
template that contains Hi <strong>${name}</strong>) to use escapeHtml(name) and
ensure the helper properly replaces &, <, >, ", and ' characters.
- Add password_reset_tokens table to schema - POST /api/auth/forgot-password — rate-limited, no email enumeration - POST /api/auth/reset-password — token hash validation, bcrypt, atomic tx - /forgot-password page — email input with success/error feedback - /reset-password page — new password + confirm with auto-redirect - Add 'Forgot Password?' link to login page (existing-user mode) Security: SHA-256 token hashing, 1hr expiry, one-time use, rate limiting
559ed7f to
9d30bc3
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/pages/api/auth/reset-password.js (2)
32-34: Consider stronger password validation.The current minimum of 6 characters is relatively weak. Additionally, bcrypt silently truncates passwords beyond 72 bytes. Consider:
- Increasing minimum to 8+ characters (NIST SP 800-63B recommendation)
- Adding a maximum length check (e.g., 128 chars) to prevent unexpected bcrypt truncation
- Optionally checking against common password lists
Example enhancement
- if (!password || password.length < 6) { - return res.status(400).json({ error: 'Password must be at least 6 characters' }) + if (!password || typeof password !== 'string') { + return res.status(400).json({ error: 'Password is required' }) + } + if (password.length < 8 || password.length > 128) { + return res.status(400).json({ error: 'Password must be between 8 and 128 characters' }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/api/auth/reset-password.js` around lines 32 - 34, Update the password validation in the reset-password request handler to enforce stronger limits: change the minimum length from 6 to 8 and add a maximum length check (e.g., 128) to guard against bcrypt’s 72-byte truncation; in the same block around the existing if (!password || password.length < 6) check, validate password.length >= 8 && password.length <= 128 and return distinct 400 JSON errors for too-short and too-long cases; optionally add a call to a common-passwords check (e.g., isCommonPassword(password)) and return a 400 if it matches, ensuring the error messages reference the specific validation that failed.
63-77: Good use of transaction for atomicity.The transaction correctly ensures the password update and token invalidation happen atomically.
Consider: Existing sessions remain valid after password reset.
Per the context in
src/lib/nextAuthConfig.js, JWT-based sessions issued before the password reset will remain valid until they naturally expire. For security-sensitive applications, consider invalidating existing sessions when a password is reset (e.g., by storing apassword_changed_attimestamp and checking it during token validation, or using a session revocation list).This is a design decision and may be acceptable for your threat model.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/api/auth/reset-password.js` around lines 63 - 77, The reset-password transaction updates the password and marks tokens used but doesn't invalidate existing JWT sessions; add an update inside the dbTx block to set a password_changed_at (e.g., 'UPDATE users SET password_hash = $1, password_changed_at = NOW(), updated_at = NOW() WHERE id = $2') so the database records when the password changed, and then modify the session/jwt validation logic in src/lib/nextAuthConfig.js to reject tokens issued before users.password_changed_at (compare token.iat or token.issueTime against password_changed_at) or implement a revocation check against that timestamp during token validation.src/pages/api/auth/forgot-password.js (1)
68-83: Consider wrapping token insert and email send in a transaction or reordering.The token is persisted (lines 68-71) before
sendMailis called (lines 78-83). Persrc/lib/mailer.js:64-80,sendMailcan throw on SMTP errors. If the email fails, an unused token remains in the database until expiry.While not critical (orphan tokens expire and are harmless), this creates unnecessary database clutter and could confuse debugging. Consider either:
- Wrapping both operations in a transaction and rolling back on email failure, or
- Inserting the token only after successful email send (though this risks the reverse scenario).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/api/auth/forgot-password.js` around lines 68 - 83, Wrap the token INSERT and sendMail call in a database transaction so that if sendMail (from src/lib/mailer.js) throws, you rollback and avoid an orphan token; specifically, begin a transaction before calling dbNone to INSERT into password_reset_tokens (using the same DB client/transaction context), call sendMail(buildResetEmailHtml(user.name, resetUrl), ...) inside that transaction, and COMMIT only after sendMail succeeds, otherwise ROLLBACK on error and rethrow; ensure you still generate rawToken/tokenHash as before and use the same transaction context for the INSERT/rollback logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/pages/api/auth/forgot-password.js`:
- Around line 68-83: Wrap the token INSERT and sendMail call in a database
transaction so that if sendMail (from src/lib/mailer.js) throws, you rollback
and avoid an orphan token; specifically, begin a transaction before calling
dbNone to INSERT into password_reset_tokens (using the same DB
client/transaction context), call sendMail(buildResetEmailHtml(user.name,
resetUrl), ...) inside that transaction, and COMMIT only after sendMail
succeeds, otherwise ROLLBACK on error and rethrow; ensure you still generate
rawToken/tokenHash as before and use the same transaction context for the
INSERT/rollback logic.
In `@src/pages/api/auth/reset-password.js`:
- Around line 32-34: Update the password validation in the reset-password
request handler to enforce stronger limits: change the minimum length from 6 to
8 and add a maximum length check (e.g., 128) to guard against bcrypt’s 72-byte
truncation; in the same block around the existing if (!password ||
password.length < 6) check, validate password.length >= 8 && password.length <=
128 and return distinct 400 JSON errors for too-short and too-long cases;
optionally add a call to a common-passwords check (e.g.,
isCommonPassword(password)) and return a 400 if it matches, ensuring the error
messages reference the specific validation that failed.
- Around line 63-77: The reset-password transaction updates the password and
marks tokens used but doesn't invalidate existing JWT sessions; add an update
inside the dbTx block to set a password_changed_at (e.g., 'UPDATE users SET
password_hash = $1, password_changed_at = NOW(), updated_at = NOW() WHERE id =
$2') so the database records when the password changed, and then modify the
session/jwt validation logic in src/lib/nextAuthConfig.js to reject tokens
issued before users.password_changed_at (compare token.iat or token.issueTime
against password_changed_at) or implement a revocation check against that
timestamp during token validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 357df2c8-771c-4d42-8325-e03e583e70d5
📒 Files selected for processing (7)
schema.sqlsrc/pages/api/auth/forgot-password.jssrc/pages/api/auth/register.jssrc/pages/api/auth/reset-password.jssrc/pages/forgot-password/index.jssrc/pages/login/index.jssrc/pages/reset-password/index.js
✅ Files skipped from review due to trivial changes (3)
- src/pages/api/auth/register.js
- src/pages/login/index.js
- schema.sql
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/reset-password/index.js
- src/pages/forgot-password/index.js
Security: SHA-256 token hashing, 1hr expiry, one-time use, rate limiting
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.
Checklist:
Summary by CodeRabbit
New Features
Security & Reliability
Bug Fixes