feat: implement accessible password reset flow - #22
Conversation
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
marekdano
left a comment
There was a problem hiding this comment.
Findings
1. Misleading error message hides password-policy failures (High)
File: src/pages/ResetPassword.tsx:621
The backend returns HTTP 400 for two different reasons: an invalid/reused token, and a password that fails policy validation (PasswordValidationError). The frontend maps every 400 to a single message: "This reset link is invalid or has already been used."
Failure scenario: User submits a password that fails the backend's complexity rules (e.g., missing an uppercase letter). Backend raises 400 with detail like "Password must contain at least 3 of the following: ...". Frontend shows the generic invalid-link message, so the user thinks their one-time link is broken and goes to request a new one — when they just needed a different password.
2. Client password validation doesn't match backend policy (High)
File: src/pages/ResetPassword.tsx:606
Client checks password.length < 8 and shows the hint "Use at least 8 characters." The actual backend (PasswordPolicyService) requires:
- 12 characters minimum for regular users, 22 for privileged accounts
- 3-of-4 complexity classes (upper/lower/digit/special)
- Not a common password
- Must not contain the username
Failure scenario: A user enters an 8–11 character password that passes client validation and the visible hint, submits, and gets rejected by the backend — then hits bug #1's wrong error message on top of it, with no way to tell what actually went wrong.
3. Hardcoded magic number instead of shared constant (Low / cleanup)
File: src/pages/ResetPassword.tsx:606
src/lib/constants.ts already defines VALIDATION.MIN_PASSWORD_LENGTH (used in src/hooks/useUserForm.ts), but ResetPassword.tsx hardcodes 8 independently instead of importing it.
4. Duplicated error-mapping logic (Low / cleanup)
File: src/pages/ForgotPassword.tsx:349
ForgotPassword.tsx and ResetPassword.tsx each implement their own ad hoc ApiError status→message chain instead of a shared helper. Not a bug, but a maintenance cost — future status codes need to be added in two places by hand.
What checked out clean
- Token URL-encoding/decoding round-trips correctly through the router (
encodeURIComponent/safeDecodeParam) - i18n keys are in full parity across all 3 locales
- Routing/
AuthGuardpublic-path wiring untouched and correct InlineNotificationrole semantics (statusvsalert) are appropriateapi.get()signature change is backward-compatible with existing callers
|
Depends on IBM/mcp-context-forge#6209 |
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
marekdano
left a comment
There was a problem hiding this comment.
1. Blocking bug
Stale "8 characters" copy after MIN_PASSWORD_LENGTH bump
File: src/lib/constants.ts:16
VALIDATION.MIN_PASSWORD_LENGTH was changed from 8 to 12 to support the new reset-password complexity rules. This constant is shared with the admin Create/Edit User form (src/hooks/useUserForm.ts), and that surface wasn't updated to match:
- src/i18n/locales/*/users.json still says "Password must be at least 8 characters" and the placeholder still says "min 8 characters"
- e2e/users.spec.ts:215,333 still assert on that stale "8 characters" text
Impact: admins creating/editing users see a wrong minimum-length message, and the e2e suite passes despite the UI being incorrect.
2. Should fix
- src/pages/ResetPassword.tsx:38-43 — Token-validation effect doesn't handle a rate-limited (429) response — falls through to "This reset link is invalid" plus a "Request New Link" CTA, which is misleading and could encourage more requests while already throttled.
- src/api/passwordResetErrors.ts:192 — Invalid-link vs. password-policy 400s are distinguished by checking whether the backend's English detail string contains "reset link" or "token" — brittle against wording changes or localization.
- src/pages/ResetPassword.tsx:190 — Editing the password field after a mismatch error doesn't clear the stale "Passwords do not match" message shown under the confirm field.
- src/pages/ResetPassword.tsx:129 — "Request New Link" is also shown when tokenState === "disabled" (feature turned off entirely) — leads to the same dead end on the forgot-password page.
None of these are security-critical, but 1 is a shipped inconsistency that will confuse admins and gives false confidence in the e2e suite — worth flagging before approval. The rest are reasonable follow-ups or inline review comments.
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
marekdano
left a comment
There was a problem hiding this comment.
The PR looks good now!
LGTM 🚀
|
Pls wait before merge, I'm reviewing. 🙏 |
|
Thanks waiting for backend PR to merge first |
What changed
Why
The existing forgot-password and reset-password routes were placeholders. Backend email delivery and frontend-compatible reset links now allow the React client to complete the full password-reset journey.
User and developer impact
Users can request a reset link, follow the emailed token, change their password, and explicitly return to login. The flow avoids account enumeration, URL-encodes tokens, does not expose token details in errors, and includes accessible labels, live status/error announcements, password controls, and focus management.
Validation
npm run format:checknpm run lintnpm test— 2,832 passed, 1 skippednpm run buildgit diff --checkNote: this frontend repository has no
make pre-committarget, so project-standard npm checks were run instead.