Skip to content

feat: implement accessible password reset flow - #22

Open
vishu-bh wants to merge 2 commits into
mainfrom
feat/password-reset-flow
Open

feat: implement accessible password reset flow#22
vishu-bh wants to merge 2 commits into
mainfrom
feat/password-reset-flow

Conversation

@vishu-bh

Copy link
Copy Markdown
Contributor

What changed

  • implement forgot-password email request flow using public backend APIs
  • validate reset tokens and allow users to set and confirm a new password
  • show persistent success, expired-link, disabled, rate-limit, and failure states
  • align the password-changed confirmation card with the Refiring Figma design
  • add localized copy for English, Spanish, and Portuguese
  • add focused API and component coverage

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:check
  • npm run lint
  • npm test — 2,832 passed, 1 skipped
  • npm run build
  • git diff --check

Note: this frontend repository has no make pre-commit target, so project-standard npm checks were run instead.

Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
@vishu-bh
vishu-bh marked this pull request as ready for review August 13, 2026 10:29

@marekdano marekdano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/AuthGuard public-path wiring untouched and correct
  • InlineNotification role semantics (status vs alert) are appropriate
  • api.get() signature change is backward-compatible with existing callers

@gcgoncalves

Copy link
Copy Markdown
Contributor

Depends on IBM/mcp-context-forge#6209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants