feat: implement forgot password and reset password functionality#144
feat: implement forgot password and reset password functionality#144
Conversation
|
🚅 Deployed to the reqcore-pr-144 environment in applirank
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a two-step password recovery flow (forgot-password and reset-password pages), a "Forgot password?" link on sign-in, SSRF protections for OIDC issuer/endpoint prefetched hosts, an empty-buffer guard in PDF parsing, and removal of the X-XSS-Protection header from global routeRules. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant ForgotPage as Forgot Password Page
participant ResetPage as Reset Password Page
participant AuthClient as Auth Client
participant Backend as Backend API
participant Email as Email Service
User->>Browser: Click "Forgot password?" on Sign-in
Browser->>ForgotPage: Navigate to /auth/forgot-password
ForgotPage->>AuthClient: requestPasswordReset(email, redirectTo)
AuthClient->>Backend: POST /auth/forgot-password { email, redirectTo }
Backend->>Email: Queue/send reset email with token link
Backend-->>AuthClient: 200 OK
AuthClient-->>ForgotPage: resolve
ForgotPage-->>Browser: show generic success UI
Email->>User: User clicks reset link (with token)
User->>Browser: Navigate to /auth/reset-password?token=...
Browser->>ResetPage: load page and track view
User->>ResetPage: Submit new password
ResetPage->>AuthClient: resetPassword({ newPassword, token })
AuthClient->>Backend: POST /auth/reset-password { token, newPassword }
Backend->>Backend: validate token, update password
Backend-->>AuthClient: 200 OK
AuthClient-->>ResetPage: resolve
ResetPage-->>Browser: show success UI and sign-in link
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/pages/auth/reset-password.vue (1)
27-74: Consider clearing password fields after successful reset.The password values remain in the reactive refs after a successful reset. For defense-in-depth, consider clearing them to minimize the time sensitive data stays in memory.
🛡️ Optional: Clear passwords after success
track("reset_password_completed"); success.value = true; isLoading.value = false; + newPassword.value = ""; + confirmPassword.value = ""; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/pages/auth/reset-password.vue` around lines 27 - 74, The password refs are left populated after a successful reset in handleResetPassword; after setting success.value = true (and before finishing), clear the sensitive refs (newPassword.value and confirmPassword.value) and any related token if desired (token.value) to minimize sensitive data in memory, ensuring you update the same function (handleResetPassword) and keep isLoading.value handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/pages/auth/reset-password.vue`:
- Around line 27-74: The password refs are left populated after a successful
reset in handleResetPassword; after setting success.value = true (and before
finishing), clear the sensitive refs (newPassword.value and
confirmPassword.value) and any related token if desired (token.value) to
minimize sensitive data in memory, ensuring you update the same function
(handleResetPassword) and keep isLoading.value handling unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c729041-002b-475d-8464-d75b5fe0ae21
📒 Files selected for processing (3)
app/pages/auth/forgot-password.vueapp/pages/auth/reset-password.vueapp/pages/auth/sign-in.vue
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
server/api/sso/providers.post.ts (1)
6-36: Code duplication withisBlockedHostinauth.ts— consider reusing.This function duplicates
isBlockedHostfromserver/utils/auth.tswith a subtle difference: line 34 only checkshostname.startsWith('fe80:')but the auth.ts version (line 52) also handles bracketed IPv6 notation[fe80:. This inconsistency could allow a bypass in the Zod validation layer.Since
prefetchOidcEndpointOrigins()already validates internally viaisBlockedHost(), consider either:
- Exporting
isBlockedHostfromauth.tsand reusing it here- Removing this duplicate validation and relying on the guard inside
prefetchOidcEndpointOrigins()If you keep both layers for defense-in-depth, at minimum add the missing bracket check:
🛡️ Quick fix for the missing bracket check
if (hostname === '::1') return true - if (hostname.startsWith('fe80:')) return true + if (hostname.startsWith('fe80:') || hostname.startsWith('[fe80:')) return true return false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/api/sso/providers.post.ts` around lines 6 - 36, This duplicates the SSRF hostname check in isBlockedHost (auth.ts); fix by importing and reusing isBlockedHost inside isBlockedIssuerUrl (or remove this duplicate and rely on prefetchOidcEndpointOrigins), otherwise make the duplicate consistent by adding the missing IPv6 bracket check (e.g., also treat hostnames starting with '[fe80:' as blocked) and ensure the same blocked set and IPv4 ranges logic as isBlockedHost so validation behavior is identical to prefetchOidcEndpointOrigins.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pages/auth/forgot-password.vue`:
- Around line 90-95: The error message container currently rendered when the
reactive property "error" is set should be made screen-reader announceable:
update the div that displays {{ error }} to include accessibility attributes
(e.g., role="alert" and aria-live="assertive", and optionally
aria-atomic="true") so assistive technologies immediately announce failures;
keep the conditional v-if="error" and the existing styling/classes intact while
adding these attributes to the error-rendering div.
- Around line 38-47: The current flow exposes whether an account exists because
it shows different UI for errors vs. success; update the forgot-password
submission handler so that regardless of result.error, thrown exceptions, or
success you always clear isLoading.value and set a generic success state/message
(do not assign to error.value or return early on result.error), logging/internal
errors can be recorded internally but the user-facing UI must always show the
same success message; apply the identical change to the reset-password handler
(the code that sets error.value and checks result.error/throws) so both handlers
always present a uniform success response to the client.
---
Nitpick comments:
In `@server/api/sso/providers.post.ts`:
- Around line 6-36: This duplicates the SSRF hostname check in isBlockedHost
(auth.ts); fix by importing and reusing isBlockedHost inside isBlockedIssuerUrl
(or remove this duplicate and rely on prefetchOidcEndpointOrigins), otherwise
make the duplicate consistent by adding the missing IPv6 bracket check (e.g.,
also treat hostnames starting with '[fe80:' as blocked) and ensure the same
blocked set and IPv4 ranges logic as isBlockedHost so validation behavior is
identical to prefetchOidcEndpointOrigins.
🪄 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: e6108bf9-b7e4-485f-acd8-d498dac2764c
📒 Files selected for processing (5)
app/pages/auth/forgot-password.vuenuxt.config.tsserver/api/sso/providers.post.tsserver/utils/auth.tsserver/utils/resume-parser.ts
💤 Files with no reviewable changes (1)
- nuxt.config.ts
✅ Files skipped from review due to trivial changes (1)
- server/utils/resume-parser.ts
| if (result.error) { | ||
| error.value = | ||
| result.error.message ?? "Failed to send reset email. Please try again."; | ||
| isLoading.value = false; | ||
| return; | ||
| } | ||
| } catch (e: unknown) { | ||
| error.value = | ||
| e instanceof Error ? e.message : "Failed to send reset email. Please try again."; | ||
| isLoading.value = false; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether forgot/reset flows emit user-specific messages that could enable enumeration.
# Expected: no "user not found / account does not exist" style messaging in this flow.
rg -n -C3 --iglob '*.{ts,js,vue}' \
'requestPasswordReset|forgot.?password|reset.?password|user not found|account.*exist|email.*exist|invalid email'Repository: reqcore-inc/reqcore
Length of output: 6254
🏁 Script executed:
cat -n app/pages/auth/forgot-password.vue | sed -n '1,100p'Repository: reqcore-inc/reqcore
Length of output: 3887
Behavioral enumeration risk: error vs. success states reveal account existence.
The code's control flow enables account enumeration regardless of message content. Errors (lines 38–42, 44–48) display an error panel and exit early, while success (lines 53–54) displays a success message. An attacker can infer whether an account exists by observing which UI state appears—this contradicts the anti-enumeration intent stated at line 53.
Treat all request outcomes (error, exception, success) as success to prevent behavioral leakage:
🔧 Proposed fix
async function handleRequestReset() {
error.value = "";
if (!email.value) {
error.value = "Email is required.";
return;
}
isLoading.value = true;
try {
const result = await authClient.requestPasswordReset({
email: email.value,
redirectTo: `${window.location.origin}${localePath("/auth/reset-password")}`,
});
if (result.error) {
- error.value =
- result.error.message ?? "Failed to send reset email. Please try again.";
- isLoading.value = false;
- return;
+ // Treat all failures as success to prevent account enumeration.
}
} catch (e: unknown) {
- error.value =
- e instanceof Error ? e.message : "Failed to send reset email. Please try again.";
- isLoading.value = false;
- return;
+ // Treat all failures as success to prevent account enumeration.
}
track("forgot_password_submitted");
- // Always show success to prevent email enumeration
success.value = true;
isLoading.value = false;
}Also check app/pages/auth/reset-password.vue (lines 58–69) for the same pattern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (result.error) { | |
| error.value = | |
| result.error.message ?? "Failed to send reset email. Please try again."; | |
| isLoading.value = false; | |
| return; | |
| } | |
| } catch (e: unknown) { | |
| error.value = | |
| e instanceof Error ? e.message : "Failed to send reset email. Please try again."; | |
| isLoading.value = false; | |
| async function handleRequestReset() { | |
| error.value = ""; | |
| if (!email.value) { | |
| error.value = "Email is required."; | |
| return; | |
| } | |
| isLoading.value = true; | |
| try { | |
| const result = await authClient.requestPasswordReset({ | |
| email: email.value, | |
| redirectTo: `${window.location.origin}${localePath("/auth/reset-password")}`, | |
| }); | |
| if (result.error) { | |
| // Treat all failures as success to prevent account enumeration. | |
| } | |
| } catch (e: unknown) { | |
| // Treat all failures as success to prevent account enumeration. | |
| } | |
| track("forgot_password_submitted"); | |
| success.value = true; | |
| isLoading.value = false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/auth/forgot-password.vue` around lines 38 - 47, The current flow
exposes whether an account exists because it shows different UI for errors vs.
success; update the forgot-password submission handler so that regardless of
result.error, thrown exceptions, or success you always clear isLoading.value and
set a generic success state/message (do not assign to error.value or return
early on result.error), logging/internal errors can be recorded internally but
the user-facing UI must always show the same success message; apply the
identical change to the reset-password handler (the code that sets error.value
and checks result.error/throws) so both handlers always present a uniform
success response to the client.
| <div | ||
| v-if="error" | ||
| class="rounded-md border border-danger-200 dark:border-danger-800 bg-danger-50 dark:bg-danger-950 p-3 text-sm text-danger-700 dark:text-danger-400" | ||
| > | ||
| {{ error }} | ||
| </div> |
There was a problem hiding this comment.
Make error feedback screen-reader announceable.
The error block should be a live alert so failures are announced immediately.
♿ Proposed fix
<div
v-if="error"
+ role="alert"
+ aria-live="assertive"
class="rounded-md border border-danger-200 dark:border-danger-800 bg-danger-50 dark:bg-danger-950 p-3 text-sm text-danger-700 dark:text-danger-400"
>
{{ error }}
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div | |
| v-if="error" | |
| class="rounded-md border border-danger-200 dark:border-danger-800 bg-danger-50 dark:bg-danger-950 p-3 text-sm text-danger-700 dark:text-danger-400" | |
| > | |
| {{ error }} | |
| </div> | |
| <div | |
| v-if="error" | |
| role="alert" | |
| aria-live="assertive" | |
| class="rounded-md border border-danger-200 dark:border-danger-800 bg-danger-50 dark:bg-danger-950 p-3 text-sm text-danger-700 dark:text-danger-400" | |
| > | |
| {{ error }} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/auth/forgot-password.vue` around lines 90 - 95, The error message
container currently rendered when the reactive property "error" is set should be
made screen-reader announceable: update the div that displays {{ error }} to
include accessibility attributes (e.g., role="alert" and aria-live="assertive",
and optionally aria-atomic="true") so assistive technologies immediately
announce failures; keep the conditional v-if="error" and the existing
styling/classes intact while adding these attributes to the error-rendering div.
1 participant
Summary
This pull request introduces a complete password reset flow for users, including both "forgot password" and "reset password" pages, and adds a link to the "forgot password" page on the sign-in screen. The new pages provide user-friendly forms, error handling, and tracking, while following security best practices such as preventing email enumeration.
Password reset flow implementation:
forgot-password.vuepage that allows users to request a password reset link by entering their email. The page includes form validation, error handling, success messaging, and tracking of user actions.reset-password.vuepage that lets users set a new password using a reset token. The page handles token validation, password validation, error and success messaging, and tracks relevant events.Sign-in page update:
sign-in.vue), directing users to the new password reset flow.Type of change
Validation
DCO
Signed-off-by) viagit commit -sSummary by CodeRabbit
New Features
Bug Fixes
Security