From aec1347e5d5e275d2c11b1c34f764858e2667988 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Thu, 13 Aug 2026 10:40:15 +0100 Subject: [PATCH 1/5] feat: implement password reset flow Signed-off-by: Vishu Bhatnagar --- src/api/client.ts | 9 +- src/api/passwordReset.test.ts | 43 +++++++ src/api/passwordReset.ts | 43 +++++++ src/i18n/locales/en-US/auth.json | 18 +++ src/i18n/locales/es-ES/auth.json | 18 +++ src/i18n/locales/pt-BR/auth.json | 18 +++ src/pages/ForgotPassword.test.tsx | 80 +++++++++++++ src/pages/ForgotPassword.tsx | 126 ++++++++++++++++++-- src/pages/ResetPassword.test.tsx | 91 ++++++++++++++ src/pages/ResetPassword.tsx | 190 ++++++++++++++++++++++++++++-- src/pages/SimplePages.test.tsx | 12 +- 11 files changed, 623 insertions(+), 25 deletions(-) create mode 100644 src/api/passwordReset.test.ts create mode 100644 src/api/passwordReset.ts create mode 100644 src/pages/ForgotPassword.test.tsx create mode 100644 src/pages/ResetPassword.test.tsx diff --git a/src/api/client.ts b/src/api/client.ts index 7b5bd45..b469e2e 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -200,8 +200,13 @@ async function request(path: string, options: RequestOptions = {}): Promise(path: string, headers?: Record, signal?: AbortSignal): Promise { - return request(path, { method: "GET", headers, signal }); + get( + path: string, + headers?: Record, + signal?: AbortSignal, + opts?: Pick, + ): Promise { + return request(path, { method: "GET", headers, signal, ...opts }); }, post( diff --git a/src/api/passwordReset.test.ts b/src/api/passwordReset.test.ts new file mode 100644 index 0000000..31ceb5b --- /dev/null +++ b/src/api/passwordReset.test.ts @@ -0,0 +1,43 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { api } from "./client"; +import { requestPasswordReset, resetPassword, validatePasswordResetToken } from "./passwordReset"; + +vi.mock("./client", () => ({ + api: { get: vi.fn(), post: vi.fn() }, +})); + +describe("passwordReset API", () => { + beforeEach(() => vi.clearAllMocks()); + + it("uses public forgot-password endpoint", async () => { + vi.mocked(api.post).mockResolvedValue({ success: true, message: "ok" }); + await requestPasswordReset("person@example.com"); + expect(api.post).toHaveBeenCalledWith( + "/auth/email/forgot-password", + { email: "person@example.com" }, + { authenticated: false }, + ); + }); + + it("URL-encodes token and validates without authentication", async () => { + vi.mocked(api.get).mockResolvedValue({ valid: true, message: "ok", expires_at: null }); + const controller = new AbortController(); + await validatePasswordResetToken("token/with space", controller.signal); + expect(api.get).toHaveBeenCalledWith( + "/auth/email/reset-password/token%2Fwith%20space", + undefined, + controller.signal, + { authenticated: false }, + ); + }); + + it("submits matching password fields to public reset endpoint", async () => { + vi.mocked(api.post).mockResolvedValue({ success: true, message: "ok" }); + await resetPassword("token/one", "new-password", "new-password"); + expect(api.post).toHaveBeenCalledWith( + "/auth/email/reset-password/token%2Fone", + { new_password: "new-password", confirm_password: "new-password" }, + { authenticated: false }, + ); + }); +}); diff --git a/src/api/passwordReset.ts b/src/api/passwordReset.ts new file mode 100644 index 0000000..50c089b --- /dev/null +++ b/src/api/passwordReset.ts @@ -0,0 +1,43 @@ +import { api } from "./client"; + +interface SuccessResponse { + success: boolean; + message: string; +} + +export interface PasswordResetTokenValidationResponse { + valid: boolean; + message: string; + expires_at: string | null; +} + +const resetPath = (token: string) => `/auth/email/reset-password/${encodeURIComponent(token)}`; + +export function requestPasswordReset(email: string): Promise { + return api.post( + "/auth/email/forgot-password", + { email }, + { authenticated: false }, + ); +} + +export function validatePasswordResetToken( + token: string, + signal?: AbortSignal, +): Promise { + return api.get(resetPath(token), undefined, signal, { + authenticated: false, + }); +} + +export function resetPassword( + token: string, + newPassword: string, + confirmPassword: string, +): Promise { + return api.post( + resetPath(token), + { new_password: newPassword, confirm_password: confirmPassword }, + { authenticated: false }, + ); +} diff --git a/src/i18n/locales/en-US/auth.json b/src/i18n/locales/en-US/auth.json index 964d80d..664ffdd 100644 --- a/src/i18n/locales/en-US/auth.json +++ b/src/i18n/locales/en-US/auth.json @@ -9,11 +9,29 @@ "auth.forgotPassword.description": "Enter your email to receive reset instructions", "auth.forgotPassword.email": "Email Address", "auth.forgotPassword.submit": "Send Reset Link", + "auth.forgotPassword.submitting": "Sending reset link…", "auth.forgotPassword.backToLogin": "Back to Sign In", + "auth.forgotPassword.success": "If this email is registered, you will receive a reset link.", + "auth.forgotPassword.error.rateLimited": "Too many requests. Please try again later.", + "auth.forgotPassword.error.disabled": "Password reset is currently unavailable.", + "auth.forgotPassword.error.failed": "We could not send a reset link. Please try again.", + "auth.forgotPassword.error.invalidEmail": "Enter a valid email address.", "auth.resetPassword.title": "Create New Password", + "auth.resetPassword.description": "Enter and confirm your new password.", "auth.resetPassword.password": "New Password", "auth.resetPassword.confirmPassword": "Confirm Password", "auth.resetPassword.submit": "Reset Password", + "auth.resetPassword.submitting": "Resetting password…", + "auth.resetPassword.passwordHint": "Use at least 8 characters.", + "auth.resetPassword.successTitle": "Password Reset", + "auth.resetPassword.success": "Your password was reset successfully. You can now sign in with your new password.", + "auth.resetPassword.requestNewLink": "Request New Link", + "auth.resetPassword.error.tooShort": "Password must be at least 8 characters.", + "auth.resetPassword.error.mismatch": "Passwords do not match.", + "auth.resetPassword.error.invalid": "This reset link is invalid or has already been used.", + "auth.resetPassword.error.expired": "This reset link has expired.", + "auth.resetPassword.error.disabled": "Password reset is currently unavailable.", + "auth.resetPassword.error.failed": "We could not reset your password. Please try again.", "auth.changePassword.title": "Change Password", "auth.changePassword.currentPassword": "Current Password", "auth.changePassword.newPassword": "New Password", diff --git a/src/i18n/locales/es-ES/auth.json b/src/i18n/locales/es-ES/auth.json index 3b7230f..b618ff5 100644 --- a/src/i18n/locales/es-ES/auth.json +++ b/src/i18n/locales/es-ES/auth.json @@ -9,11 +9,29 @@ "auth.forgotPassword.description": "Ingresa tu correo electrónico para recibir instrucciones de restablecimiento", "auth.forgotPassword.email": "Dirección de Correo Electrónico", "auth.forgotPassword.submit": "Enviar Enlace de Restablecimiento", + "auth.forgotPassword.submitting": "Enviando enlace de restablecimiento…", "auth.forgotPassword.backToLogin": "Volver a Iniciar Sesión", + "auth.forgotPassword.success": "Si este correo está registrado, recibirás un enlace de restablecimiento.", + "auth.forgotPassword.error.rateLimited": "Demasiadas solicitudes. Inténtalo de nuevo más tarde.", + "auth.forgotPassword.error.disabled": "El restablecimiento de contraseña no está disponible actualmente.", + "auth.forgotPassword.error.failed": "No pudimos enviar el enlace de restablecimiento. Inténtalo de nuevo.", + "auth.forgotPassword.error.invalidEmail": "Ingresa una dirección de correo electrónico válida.", "auth.resetPassword.title": "Crear Nueva Contraseña", + "auth.resetPassword.description": "Ingresa y confirma tu nueva contraseña.", "auth.resetPassword.password": "Nueva Contraseña", "auth.resetPassword.confirmPassword": "Confirmar Contraseña", "auth.resetPassword.submit": "Restablecer Contraseña", + "auth.resetPassword.submitting": "Restableciendo contraseña…", + "auth.resetPassword.passwordHint": "Usa al menos 8 caracteres.", + "auth.resetPassword.successTitle": "Contraseña Restablecida", + "auth.resetPassword.success": "Tu contraseña se restableció correctamente. Ya puedes iniciar sesión con tu nueva contraseña.", + "auth.resetPassword.requestNewLink": "Solicitar Nuevo Enlace", + "auth.resetPassword.error.tooShort": "La contraseña debe tener al menos 8 caracteres.", + "auth.resetPassword.error.mismatch": "Las contraseñas no coinciden.", + "auth.resetPassword.error.invalid": "Este enlace de restablecimiento no es válido o ya fue utilizado.", + "auth.resetPassword.error.expired": "Este enlace de restablecimiento ha caducado.", + "auth.resetPassword.error.disabled": "El restablecimiento de contraseña no está disponible actualmente.", + "auth.resetPassword.error.failed": "No pudimos restablecer tu contraseña. Inténtalo de nuevo.", "auth.changePassword.title": "Cambiar Contraseña", "auth.changePassword.currentPassword": "Contraseña Actual", "auth.changePassword.newPassword": "Nueva Contraseña", diff --git a/src/i18n/locales/pt-BR/auth.json b/src/i18n/locales/pt-BR/auth.json index 21bc3ef..18cb9db 100644 --- a/src/i18n/locales/pt-BR/auth.json +++ b/src/i18n/locales/pt-BR/auth.json @@ -9,11 +9,29 @@ "auth.forgotPassword.description": "Digite seu e-mail para receber instruções de redefinição", "auth.forgotPassword.email": "Endereço de E-mail", "auth.forgotPassword.submit": "Enviar Link de Redefinição", + "auth.forgotPassword.submitting": "Enviando link de redefinição…", "auth.forgotPassword.backToLogin": "Voltar para Entrar", + "auth.forgotPassword.success": "Se este e-mail estiver registrado, você receberá um link de redefinição.", + "auth.forgotPassword.error.rateLimited": "Muitas solicitações. Tente novamente mais tarde.", + "auth.forgotPassword.error.disabled": "A redefinição de senha não está disponível no momento.", + "auth.forgotPassword.error.failed": "Não foi possível enviar o link de redefinição. Tente novamente.", + "auth.forgotPassword.error.invalidEmail": "Digite um endereço de e-mail válido.", "auth.resetPassword.title": "Criar Nova Senha", + "auth.resetPassword.description": "Digite e confirme sua nova senha.", "auth.resetPassword.password": "Nova Senha", "auth.resetPassword.confirmPassword": "Confirmar Senha", "auth.resetPassword.submit": "Redefinir Senha", + "auth.resetPassword.submitting": "Redefinindo senha…", + "auth.resetPassword.passwordHint": "Use pelo menos 8 caracteres.", + "auth.resetPassword.successTitle": "Senha Redefinida", + "auth.resetPassword.success": "Sua senha foi redefinida com sucesso. Agora você pode entrar com sua nova senha.", + "auth.resetPassword.requestNewLink": "Solicitar Novo Link", + "auth.resetPassword.error.tooShort": "A senha deve ter pelo menos 8 caracteres.", + "auth.resetPassword.error.mismatch": "As senhas não coincidem.", + "auth.resetPassword.error.invalid": "Este link de redefinição é inválido ou já foi usado.", + "auth.resetPassword.error.expired": "Este link de redefinição expirou.", + "auth.resetPassword.error.disabled": "A redefinição de senha não está disponível no momento.", + "auth.resetPassword.error.failed": "Não foi possível redefinir sua senha. Tente novamente.", "auth.changePassword.title": "Alterar Senha", "auth.changePassword.currentPassword": "Senha Atual", "auth.changePassword.newPassword": "Nova Senha", diff --git a/src/pages/ForgotPassword.test.tsx b/src/pages/ForgotPassword.test.tsx new file mode 100644 index 0000000..3a36cd8 --- /dev/null +++ b/src/pages/ForgotPassword.test.tsx @@ -0,0 +1,80 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; +import { requestPasswordReset } from "@/api/passwordReset"; +import { I18nProvider } from "@/i18n"; +import { useRouter } from "@/router"; +import { ForgotPassword } from "./ForgotPassword"; + +vi.mock("@/api/passwordReset", () => ({ requestPasswordReset: vi.fn() })); +vi.mock("@/router", () => ({ useRouter: vi.fn() })); + +describe("ForgotPassword", () => { + const navigate = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(useRouter).mockReturnValue({ navigate } as unknown as ReturnType); + }); + + function renderPage() { + return render( + + + , + ); + } + + it("submits email and shows persistent generic success state", async () => { + vi.mocked(requestPasswordReset).mockResolvedValue({ success: true, message: "ok" }); + renderPage(); + + fireEvent.change(screen.getByLabelText("Email Address"), { + target: { value: "person@example.com" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Send Reset Link" })); + + await waitFor(() => expect(requestPasswordReset).toHaveBeenCalledWith("person@example.com")); + expect(screen.getByRole("status")).toHaveTextContent( + "If this email is registered, you will receive a reset link.", + ); + expect(navigate).not.toHaveBeenCalled(); + }); + + it("navigates to login only after user clicks success action", async () => { + vi.mocked(requestPasswordReset).mockResolvedValue({ success: true, message: "ok" }); + renderPage(); + fireEvent.change(screen.getByLabelText("Email Address"), { + target: { value: "person@example.com" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Send Reset Link" })); + + await screen.findByRole("status"); + fireEvent.click(screen.getByRole("button", { name: "Back to Sign In" })); + expect(navigate).toHaveBeenCalledWith("/app/login"); + }); + + it("shows accessible rate-limit error", async () => { + vi.mocked(requestPasswordReset).mockRejectedValue( + new ApiError(429, { detail: "Too many requests" }, "HTTP 429"), + ); + renderPage(); + const input = screen.getByLabelText("Email Address"); + fireEvent.change(input, { target: { value: "person@example.com" } }); + fireEvent.click(screen.getByRole("button", { name: "Send Reset Link" })); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "Too many requests. Please try again later.", + ); + expect(input).toHaveAttribute("aria-invalid", "false"); + }); + + it("validates email before calling API", async () => { + renderPage(); + fireEvent.change(screen.getByLabelText("Email Address"), { target: { value: "invalid" } }); + fireEvent.click(screen.getByRole("button", { name: "Send Reset Link" })); + + expect(await screen.findByRole("alert")).toHaveTextContent("Enter a valid email address."); + expect(requestPasswordReset).not.toHaveBeenCalled(); + }); +}); diff --git a/src/pages/ForgotPassword.tsx b/src/pages/ForgotPassword.tsx index 190171d..7af4bf9 100644 --- a/src/pages/ForgotPassword.tsx +++ b/src/pages/ForgotPassword.tsx @@ -1,12 +1,124 @@ +import { useState } from "react"; +import { useIntl } from "react-intl"; +import { ApiError } from "@/api/client"; +import { requestPasswordReset } from "@/api/passwordReset"; +import { Button } from "@/components/ui/button"; +import { InlineNotification } from "@/components/ui/inline-notification"; +import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; +import { useRouter } from "@/router"; + export function ForgotPassword() { + const intl = useIntl(); + const { navigate } = useRouter(); + const [email, setEmail] = useState(""); + const [loading, setLoading] = useState(false); + const [submitted, setSubmitted] = useState(false); + const [emailError, setEmailError] = useState(null); + const [error, setError] = useState(null); + + async function handleSubmit(event: React.FormEvent) { + event.preventDefault(); + setError(null); + setEmailError(null); + + if (!event.currentTarget.checkValidity()) { + setEmailError(intl.formatMessage({ id: "auth.forgotPassword.error.invalidEmail" })); + return; + } + + setLoading(true); + + try { + await requestPasswordReset(email.trim()); + setSubmitted(true); + } catch (err) { + if (err instanceof ApiError && err.status === 429) { + setError(intl.formatMessage({ id: "auth.forgotPassword.error.rateLimited" })); + } else if (err instanceof ApiError && err.status === 403) { + setError(intl.formatMessage({ id: "auth.forgotPassword.error.disabled" })); + } else { + setError(intl.formatMessage({ id: "auth.forgotPassword.error.failed" })); + } + } finally { + setLoading(false); + } + } + return ( -
-
-

- Forgot password +
+
+

+ {intl.formatMessage({ id: "auth.forgotPassword.title" })}

-

Not yet implemented.

-

-
+

+ {intl.formatMessage({ id: "auth.forgotPassword.description" })} +

+ + {submitted ? ( +
+ + +
+ ) : ( +
+
+ + { + setEmail(event.target.value); + setEmailError(null); + }} + aria-invalid={!!emailError} + aria-describedby={emailError ? "forgot-password-email-error" : undefined} + /> + {emailError && ( + + )} +
+ {error && } + + + + )} + + ); } diff --git a/src/pages/ResetPassword.test.tsx b/src/pages/ResetPassword.test.tsx new file mode 100644 index 0000000..0be1130 --- /dev/null +++ b/src/pages/ResetPassword.test.tsx @@ -0,0 +1,91 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; +import { resetPassword, validatePasswordResetToken } from "@/api/passwordReset"; +import { I18nProvider } from "@/i18n"; +import { useRouter } from "@/router"; +import { ResetPassword } from "./ResetPassword"; + +vi.mock("@/api/passwordReset", () => ({ + resetPassword: vi.fn(), + validatePasswordResetToken: vi.fn(), +})); +vi.mock("@/router", () => ({ useRouter: vi.fn() })); + +describe("ResetPassword", () => { + const navigate = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(useRouter).mockReturnValue({ navigate } as unknown as ReturnType); + vi.mocked(validatePasswordResetToken).mockResolvedValue({ + valid: true, + message: "valid", + expires_at: null, + }); + }); + + function renderPage(token = "reset/token") { + return render( + + + , + ); + } + + async function fillPasswords(password: string, confirmation = password) { + await waitFor(() => expect(document.getElementById("new-password")).toBeInTheDocument()); + fireEvent.change(document.getElementById("new-password")!, { target: { value: password } }); + fireEvent.change(document.getElementById("confirm-password")!, { + target: { value: confirmation }, + }); + } + + it("validates route token before showing form", async () => { + renderPage("safe-token"); + await waitFor(() => + expect(validatePasswordResetToken).toHaveBeenCalledWith( + "safe-token", + expect.any(AbortSignal), + ), + ); + await waitFor(() => expect(document.getElementById("new-password")).toBeInTheDocument()); + }); + + it("rejects mismatched passwords without API request", async () => { + renderPage(); + await fillPasswords("password-one", "password-two"); + fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); + + expect(await screen.findAllByText("Passwords do not match.")).not.toHaveLength(0); + expect(resetPassword).not.toHaveBeenCalled(); + }); + + it("shows persistent success and waits for explicit login navigation", async () => { + vi.mocked(resetPassword).mockResolvedValue({ success: true, message: "ok" }); + renderPage("reset/token"); + await fillPasswords("new-password"); + fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); + + expect(await screen.findByRole("status")).toHaveTextContent( + "Your password was reset successfully", + ); + expect(resetPassword).toHaveBeenCalledWith("reset/token", "new-password", "new-password"); + expect(navigate).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByRole("button", { name: "Back to Sign In" })); + expect(navigate).toHaveBeenCalledWith("/app/login"); + }); + + it("offers new link for expired token without exposing token", async () => { + vi.mocked(validatePasswordResetToken).mockRejectedValue( + new ApiError(410, { detail: "expired reset/token" }, "HTTP 410"), + ); + renderPage("secret-reset-token"); + + expect(await screen.findByRole("alert")).toHaveTextContent("This reset link has expired."); + expect(screen.queryByText(/secret-reset-token/)).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Request New Link" })); + expect(navigate).toHaveBeenCalledWith("/app/forgot-password"); + }); +}); diff --git a/src/pages/ResetPassword.tsx b/src/pages/ResetPassword.tsx index 3ab402a..6d595bd 100644 --- a/src/pages/ResetPassword.tsx +++ b/src/pages/ResetPassword.tsx @@ -1,12 +1,184 @@ -export function ResetPassword() { +import { useEffect, useRef, useState } from "react"; +import { useIntl } from "react-intl"; +import { ApiError } from "@/api/client"; +import { resetPassword, validatePasswordResetToken } from "@/api/passwordReset"; +import { PasswordInput } from "@/components/users/PasswordInput"; +import { Button } from "@/components/ui/button"; +import { InlineNotification } from "@/components/ui/inline-notification"; +import { Loading } from "@/components/ui/loading"; +import { useRouter } from "@/router"; + +type TokenState = "validating" | "valid" | "invalid" | "expired" | "disabled"; + +export function ResetPassword({ token = "" }: { token?: string }) { + const intl = useIntl(); + const { navigate } = useRouter(); + const [tokenState, setTokenState] = useState("validating"); + const [password, setPassword] = useState(""); + const [confirmPassword, setConfirmPassword] = useState(""); + const [passwordError, setPasswordError] = useState(null); + const [confirmPasswordError, setConfirmPasswordError] = useState(null); + const [submitError, setSubmitError] = useState(null); + const [submitting, setSubmitting] = useState(false); + const [succeeded, setSucceeded] = useState(false); + const successHeadingRef = useRef(null); + + useEffect(() => { + const controller = new AbortController(); + + if (!token) { + setTokenState("invalid"); + return () => controller.abort(); + } + + validatePasswordResetToken(token, controller.signal) + .then((result) => setTokenState(result.valid ? "valid" : "invalid")) + .catch((err) => { + if (controller.signal.aborted) return; + if (err instanceof ApiError && err.status === 410) setTokenState("expired"); + else if (err instanceof ApiError && err.status === 403) setTokenState("disabled"); + else setTokenState("invalid"); + }); + + return () => controller.abort(); + }, [token]); + + useEffect(() => { + if (succeeded) successHeadingRef.current?.focus(); + }, [succeeded]); + + async function handleSubmit(event: React.FormEvent) { + event.preventDefault(); + setPasswordError(null); + setConfirmPasswordError(null); + setSubmitError(null); + + if (password.length < 8) { + setPasswordError(intl.formatMessage({ id: "auth.resetPassword.error.tooShort" })); + return; + } + if (password !== confirmPassword) { + setConfirmPasswordError(intl.formatMessage({ id: "auth.resetPassword.error.mismatch" })); + return; + } + + setSubmitting(true); + try { + await resetPassword(token, password, confirmPassword); + setPassword(""); + setConfirmPassword(""); + setSucceeded(true); + } catch (err) { + if (err instanceof ApiError && err.status === 410) setTokenState("expired"); + else if (err instanceof ApiError && err.status === 403) setTokenState("disabled"); + else if (err instanceof ApiError && err.status === 400) + setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.invalid" })); + else setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.failed" })); + } finally { + setSubmitting(false); + } + } + + const tokenErrorMessage = + tokenState === "expired" + ? intl.formatMessage({ id: "auth.resetPassword.error.expired" }) + : tokenState === "disabled" + ? intl.formatMessage({ id: "auth.resetPassword.error.disabled" }) + : intl.formatMessage({ id: "auth.resetPassword.error.invalid" }); + return ( -
-
-

- Reset password -

-

Not yet implemented.

-
-
+
+
+ {succeeded ? ( +
+

+ {intl.formatMessage({ id: "auth.resetPassword.successTitle" })} +

+ + +
+ ) : ( + <> +

+ {intl.formatMessage({ id: "auth.resetPassword.title" })} +

+

+ {intl.formatMessage({ id: "auth.resetPassword.description" })} +

+ + {tokenState === "validating" ? ( +
+ +
+ ) : tokenState !== "valid" ? ( +
+ + +
+ ) : ( +
+ { + setPassword(value); + setPasswordError(null); + }} + label={intl.formatMessage({ id: "auth.resetPassword.password" })} + placeholder={intl.formatMessage({ id: "auth.resetPassword.password" })} + required + hint={intl.formatMessage({ id: "auth.resetPassword.passwordHint" })} + error={passwordError ?? undefined} + /> + { + setConfirmPassword(value); + setConfirmPasswordError(null); + }} + label={intl.formatMessage({ id: "auth.resetPassword.confirmPassword" })} + placeholder={intl.formatMessage({ id: "auth.resetPassword.confirmPassword" })} + required + error={confirmPasswordError ?? undefined} + /> + {submitError && } + + + )} + + )} +
+
); } diff --git a/src/pages/SimplePages.test.tsx b/src/pages/SimplePages.test.tsx index 785fc43..fdee715 100644 --- a/src/pages/SimplePages.test.tsx +++ b/src/pages/SimplePages.test.tsx @@ -34,7 +34,6 @@ import { Observability } from "./Observability"; import { Performance } from "./Performance"; import { Plugins } from "./Plugins"; import { Prompts } from "./Prompts"; -import { ResetPassword } from "./ResetPassword"; import { Resources } from "./Resources"; import { RestApi } from "./RestApi"; import { ServerCatalog } from "./ServerCatalog"; @@ -61,7 +60,11 @@ describe("Simple Page Components", () => { }); it("renders ForgotPassword page", () => { - renderWithProviders(); + renderWithProviders( + + + , + ); expect(document.body).toBeTruthy(); }); @@ -110,11 +113,6 @@ describe("Simple Page Components", () => { expect(document.body).toBeTruthy(); }); - it("renders ResetPassword page", () => { - renderWithProviders(); - expect(document.body).toBeTruthy(); - }); - it("renders Resources page", () => { renderWithProviders( From 9e7ca90c3595d8b0ec579e8bbb7524910c826089 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Thu, 13 Aug 2026 11:28:41 +0100 Subject: [PATCH 2/5] style: align password success state with design Signed-off-by: Vishu Bhatnagar --- src/i18n/locales/en-US/auth.json | 5 ++-- src/i18n/locales/es-ES/auth.json | 5 ++-- src/i18n/locales/pt-BR/auth.json | 5 ++-- src/pages/ResetPassword.test.tsx | 7 +++--- src/pages/ResetPassword.tsx | 43 +++++++++++++++++++++----------- 5 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/i18n/locales/en-US/auth.json b/src/i18n/locales/en-US/auth.json index 664ffdd..75010dc 100644 --- a/src/i18n/locales/en-US/auth.json +++ b/src/i18n/locales/en-US/auth.json @@ -23,8 +23,9 @@ "auth.resetPassword.submit": "Reset Password", "auth.resetPassword.submitting": "Resetting password…", "auth.resetPassword.passwordHint": "Use at least 8 characters.", - "auth.resetPassword.successTitle": "Password Reset", - "auth.resetPassword.success": "Your password was reset successfully. You can now sign in with your new password.", + "auth.resetPassword.successTitle": "Password changed", + "auth.resetPassword.success": "Your password was changed for ContextForge.", + "auth.resetPassword.returnToLogin": "Return to login", "auth.resetPassword.requestNewLink": "Request New Link", "auth.resetPassword.error.tooShort": "Password must be at least 8 characters.", "auth.resetPassword.error.mismatch": "Passwords do not match.", diff --git a/src/i18n/locales/es-ES/auth.json b/src/i18n/locales/es-ES/auth.json index b618ff5..d208cac 100644 --- a/src/i18n/locales/es-ES/auth.json +++ b/src/i18n/locales/es-ES/auth.json @@ -23,8 +23,9 @@ "auth.resetPassword.submit": "Restablecer Contraseña", "auth.resetPassword.submitting": "Restableciendo contraseña…", "auth.resetPassword.passwordHint": "Usa al menos 8 caracteres.", - "auth.resetPassword.successTitle": "Contraseña Restablecida", - "auth.resetPassword.success": "Tu contraseña se restableció correctamente. Ya puedes iniciar sesión con tu nueva contraseña.", + "auth.resetPassword.successTitle": "Contraseña cambiada", + "auth.resetPassword.success": "Tu contraseña de ContextForge fue cambiada.", + "auth.resetPassword.returnToLogin": "Volver al inicio de sesión", "auth.resetPassword.requestNewLink": "Solicitar Nuevo Enlace", "auth.resetPassword.error.tooShort": "La contraseña debe tener al menos 8 caracteres.", "auth.resetPassword.error.mismatch": "Las contraseñas no coinciden.", diff --git a/src/i18n/locales/pt-BR/auth.json b/src/i18n/locales/pt-BR/auth.json index 18cb9db..2e75b63 100644 --- a/src/i18n/locales/pt-BR/auth.json +++ b/src/i18n/locales/pt-BR/auth.json @@ -23,8 +23,9 @@ "auth.resetPassword.submit": "Redefinir Senha", "auth.resetPassword.submitting": "Redefinindo senha…", "auth.resetPassword.passwordHint": "Use pelo menos 8 caracteres.", - "auth.resetPassword.successTitle": "Senha Redefinida", - "auth.resetPassword.success": "Sua senha foi redefinida com sucesso. Agora você pode entrar com sua nova senha.", + "auth.resetPassword.successTitle": "Senha alterada", + "auth.resetPassword.success": "Sua senha do ContextForge foi alterada.", + "auth.resetPassword.returnToLogin": "Voltar ao login", "auth.resetPassword.requestNewLink": "Solicitar Novo Link", "auth.resetPassword.error.tooShort": "A senha deve ter pelo menos 8 caracteres.", "auth.resetPassword.error.mismatch": "As senhas não coincidem.", diff --git a/src/pages/ResetPassword.test.tsx b/src/pages/ResetPassword.test.tsx index 0be1130..63145b3 100644 --- a/src/pages/ResetPassword.test.tsx +++ b/src/pages/ResetPassword.test.tsx @@ -67,13 +67,14 @@ describe("ResetPassword", () => { await fillPasswords("new-password"); fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); - expect(await screen.findByRole("status")).toHaveTextContent( - "Your password was reset successfully", + expect(await screen.findByRole("status")).toHaveTextContent("Password changed"); + expect(screen.getByRole("status")).toHaveTextContent( + "Your password was changed for ContextForge.", ); expect(resetPassword).toHaveBeenCalledWith("reset/token", "new-password", "new-password"); expect(navigate).not.toHaveBeenCalled(); - fireEvent.click(screen.getByRole("button", { name: "Back to Sign In" })); + fireEvent.click(screen.getByRole("button", { name: "Return to login" })); expect(navigate).toHaveBeenCalledWith("/app/login"); }); diff --git a/src/pages/ResetPassword.tsx b/src/pages/ResetPassword.tsx index 6d595bd..1b435ed 100644 --- a/src/pages/ResetPassword.tsx +++ b/src/pages/ResetPassword.tsx @@ -1,4 +1,5 @@ import { useEffect, useRef, useState } from "react"; +import { CircleCheck } from "lucide-react"; import { useIntl } from "react-intl"; import { ApiError } from "@/api/client"; import { resetPassword, validatePasswordResetToken } from "@/api/passwordReset"; @@ -89,25 +90,37 @@ export function ResetPassword({ token = "" }: { token?: string }) { return (
{succeeded ? ( -
-

+
+

+

+

+ {intl.formatMessage({ id: "auth.resetPassword.success" })} +

+
+

- -
) : ( From 4efc8515f1f57776d042e026a182886354c1278d Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Fri, 14 Aug 2026 11:36:45 +0100 Subject: [PATCH 3/5] fix: address password reset review feedback Signed-off-by: Vishu Bhatnagar --- src/api/passwordResetErrors.test.ts | 38 +++++++++++++++++++++++++++++ src/api/passwordResetErrors.ts | 33 +++++++++++++++++++++++++ src/i18n/locales/en-US/auth.json | 5 ++-- src/i18n/locales/es-ES/auth.json | 5 ++-- src/i18n/locales/pt-BR/auth.json | 5 ++-- src/lib/constants.ts | 2 +- src/lib/lib.test.ts | 4 +-- src/pages/ForgotPassword.tsx | 7 +++--- src/pages/ResetPassword.test.tsx | 36 ++++++++++++++++++++++++--- src/pages/ResetPassword.tsx | 25 +++++++++++++------ 10 files changed, 138 insertions(+), 22 deletions(-) create mode 100644 src/api/passwordResetErrors.test.ts create mode 100644 src/api/passwordResetErrors.ts diff --git a/src/api/passwordResetErrors.test.ts b/src/api/passwordResetErrors.test.ts new file mode 100644 index 0000000..eb28b66 --- /dev/null +++ b/src/api/passwordResetErrors.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from "vitest"; +import { ApiError } from "./client"; +import { classifyPasswordResetError } from "./passwordResetErrors"; + +describe("classifyPasswordResetError", () => { + it.each([ + [403, "disabled"], + [410, "expired"], + [429, "rateLimited"], + [500, "failed"], + ])("maps HTTP %s to %s", (status, kind) => { + expect(classifyPasswordResetError(new ApiError(status, null, `HTTP ${status}`))).toEqual({ + kind, + }); + }); + + it("preserves actionable backend password-policy detail", () => { + const detail = "Password must contain at least 3 character types"; + expect(classifyPasswordResetError(new ApiError(400, { detail }, "HTTP 400"))).toEqual({ + kind: "validation", + message: detail, + }); + }); + + it("uses invalid fallback when a 400 has no safe detail", () => { + expect(classifyPasswordResetError(new ApiError(400, null, "HTTP 400"))).toEqual({ + kind: "invalid", + }); + }); + + it("classifies token detail as an invalid link rather than password validation", () => { + expect( + classifyPasswordResetError( + new ApiError(400, { detail: "This reset link is invalid" }, "HTTP 400"), + ), + ).toEqual({ kind: "invalid" }); + }); +}); diff --git a/src/api/passwordResetErrors.ts b/src/api/passwordResetErrors.ts new file mode 100644 index 0000000..8037ff6 --- /dev/null +++ b/src/api/passwordResetErrors.ts @@ -0,0 +1,33 @@ +import { ApiError } from "./client"; +import { extractApiErrorDetail } from "@/utils/errors"; + +export type PasswordResetError = + | { kind: "disabled" } + | { kind: "expired" } + | { kind: "invalid" } + | { kind: "rateLimited" } + | { kind: "validation"; message: string } + | { kind: "failed" }; + +/** Classify password-reset API failures in one place for both public auth screens. */ +export function classifyPasswordResetError(error: unknown): PasswordResetError { + if (!(error instanceof ApiError)) return { kind: "failed" }; + + if (error.status === 403) return { kind: "disabled" }; + if (error.status === 410) return { kind: "expired" }; + if (error.status === 429) return { kind: "rateLimited" }; + + if (error.status === 400) { + const detail = extractApiErrorDetail(error.body); + if (!detail) return { kind: "invalid" }; + + const normalizedDetail = detail.toLowerCase(); + if (normalizedDetail.includes("reset link") || normalizedDetail.includes("token")) { + return { kind: "invalid" }; + } + + return { kind: "validation", message: detail }; + } + + return { kind: "failed" }; +} diff --git a/src/i18n/locales/en-US/auth.json b/src/i18n/locales/en-US/auth.json index 75010dc..52a3871 100644 --- a/src/i18n/locales/en-US/auth.json +++ b/src/i18n/locales/en-US/auth.json @@ -22,12 +22,13 @@ "auth.resetPassword.confirmPassword": "Confirm Password", "auth.resetPassword.submit": "Reset Password", "auth.resetPassword.submitting": "Resetting password…", - "auth.resetPassword.passwordHint": "Use at least 8 characters.", + "auth.resetPassword.passwordHint": "Use at least 12 characters and 3 of: uppercase, lowercase, number, or special character. Privileged accounts may require more.", "auth.resetPassword.successTitle": "Password changed", "auth.resetPassword.success": "Your password was changed for ContextForge.", "auth.resetPassword.returnToLogin": "Return to login", "auth.resetPassword.requestNewLink": "Request New Link", - "auth.resetPassword.error.tooShort": "Password must be at least 8 characters.", + "auth.resetPassword.error.tooShort": "Password must be at least 12 characters.", + "auth.resetPassword.error.complexity": "Password must contain at least 3 of: uppercase, lowercase, number, or special character.", "auth.resetPassword.error.mismatch": "Passwords do not match.", "auth.resetPassword.error.invalid": "This reset link is invalid or has already been used.", "auth.resetPassword.error.expired": "This reset link has expired.", diff --git a/src/i18n/locales/es-ES/auth.json b/src/i18n/locales/es-ES/auth.json index d208cac..e9e6cf0 100644 --- a/src/i18n/locales/es-ES/auth.json +++ b/src/i18n/locales/es-ES/auth.json @@ -22,12 +22,13 @@ "auth.resetPassword.confirmPassword": "Confirmar Contraseña", "auth.resetPassword.submit": "Restablecer Contraseña", "auth.resetPassword.submitting": "Restableciendo contraseña…", - "auth.resetPassword.passwordHint": "Usa al menos 8 caracteres.", + "auth.resetPassword.passwordHint": "Usa al menos 12 caracteres y 3 de estos tipos: mayúscula, minúscula, número o carácter especial. Las cuentas privilegiadas pueden requerir más.", "auth.resetPassword.successTitle": "Contraseña cambiada", "auth.resetPassword.success": "Tu contraseña de ContextForge fue cambiada.", "auth.resetPassword.returnToLogin": "Volver al inicio de sesión", "auth.resetPassword.requestNewLink": "Solicitar Nuevo Enlace", - "auth.resetPassword.error.tooShort": "La contraseña debe tener al menos 8 caracteres.", + "auth.resetPassword.error.tooShort": "La contraseña debe tener al menos 12 caracteres.", + "auth.resetPassword.error.complexity": "La contraseña debe contener al menos 3 de estos tipos: mayúscula, minúscula, número o carácter especial.", "auth.resetPassword.error.mismatch": "Las contraseñas no coinciden.", "auth.resetPassword.error.invalid": "Este enlace de restablecimiento no es válido o ya fue utilizado.", "auth.resetPassword.error.expired": "Este enlace de restablecimiento ha caducado.", diff --git a/src/i18n/locales/pt-BR/auth.json b/src/i18n/locales/pt-BR/auth.json index 2e75b63..4d22d93 100644 --- a/src/i18n/locales/pt-BR/auth.json +++ b/src/i18n/locales/pt-BR/auth.json @@ -22,12 +22,13 @@ "auth.resetPassword.confirmPassword": "Confirmar Senha", "auth.resetPassword.submit": "Redefinir Senha", "auth.resetPassword.submitting": "Redefinindo senha…", - "auth.resetPassword.passwordHint": "Use pelo menos 8 caracteres.", + "auth.resetPassword.passwordHint": "Use pelo menos 12 caracteres e 3 destes tipos: maiúscula, minúscula, número ou caractere especial. Contas privilegiadas podem exigir mais.", "auth.resetPassword.successTitle": "Senha alterada", "auth.resetPassword.success": "Sua senha do ContextForge foi alterada.", "auth.resetPassword.returnToLogin": "Voltar ao login", "auth.resetPassword.requestNewLink": "Solicitar Novo Link", - "auth.resetPassword.error.tooShort": "A senha deve ter pelo menos 8 caracteres.", + "auth.resetPassword.error.tooShort": "A senha deve ter pelo menos 12 caracteres.", + "auth.resetPassword.error.complexity": "A senha deve conter pelo menos 3 destes tipos: maiúscula, minúscula, número ou caractere especial.", "auth.resetPassword.error.mismatch": "As senhas não coincidem.", "auth.resetPassword.error.invalid": "Este link de redefinição é inválido ou já foi usado.", "auth.resetPassword.error.expired": "Este link de redefinição expirou.", diff --git a/src/lib/constants.ts b/src/lib/constants.ts index e3d5c3d..6ca5d84 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -13,7 +13,7 @@ export const VALIDATION = { MAX_PASSWORD_LENGTH: 1000, /** Minimum password length requirement */ - MIN_PASSWORD_LENGTH: 8, + MIN_PASSWORD_LENGTH: 12, /** Maximum length for name fields */ MAX_NAME_LENGTH: 255, diff --git a/src/lib/lib.test.ts b/src/lib/lib.test.ts index 3bf2317..5d7f964 100644 --- a/src/lib/lib.test.ts +++ b/src/lib/lib.test.ts @@ -61,8 +61,8 @@ describe("VALIDATION constants", () => { expect(VALIDATION.MAX_PASSWORD_LENGTH).toBe(1000); }); - it("MIN_PASSWORD_LENGTH is 8", () => { - expect(VALIDATION.MIN_PASSWORD_LENGTH).toBe(8); + it("MIN_PASSWORD_LENGTH is 12", () => { + expect(VALIDATION.MIN_PASSWORD_LENGTH).toBe(12); }); it("MAX_NAME_LENGTH is 255", () => { diff --git a/src/pages/ForgotPassword.tsx b/src/pages/ForgotPassword.tsx index 7af4bf9..c7553e3 100644 --- a/src/pages/ForgotPassword.tsx +++ b/src/pages/ForgotPassword.tsx @@ -1,7 +1,7 @@ import { useState } from "react"; import { useIntl } from "react-intl"; -import { ApiError } from "@/api/client"; import { requestPasswordReset } from "@/api/passwordReset"; +import { classifyPasswordResetError } from "@/api/passwordResetErrors"; import { Button } from "@/components/ui/button"; import { InlineNotification } from "@/components/ui/inline-notification"; import { Input } from "@/components/ui/input"; @@ -33,9 +33,10 @@ export function ForgotPassword() { await requestPasswordReset(email.trim()); setSubmitted(true); } catch (err) { - if (err instanceof ApiError && err.status === 429) { + const resetError = classifyPasswordResetError(err); + if (resetError.kind === "rateLimited") { setError(intl.formatMessage({ id: "auth.forgotPassword.error.rateLimited" })); - } else if (err instanceof ApiError && err.status === 403) { + } else if (resetError.kind === "disabled") { setError(intl.formatMessage({ id: "auth.forgotPassword.error.disabled" })); } else { setError(intl.formatMessage({ id: "auth.forgotPassword.error.failed" })); diff --git a/src/pages/ResetPassword.test.tsx b/src/pages/ResetPassword.test.tsx index 63145b3..62a3831 100644 --- a/src/pages/ResetPassword.test.tsx +++ b/src/pages/ResetPassword.test.tsx @@ -54,7 +54,7 @@ describe("ResetPassword", () => { it("rejects mismatched passwords without API request", async () => { renderPage(); - await fillPasswords("password-one", "password-two"); + await fillPasswords("Password-one1", "Password-two2"); fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); expect(await screen.findAllByText("Passwords do not match.")).not.toHaveLength(0); @@ -64,20 +64,50 @@ describe("ResetPassword", () => { it("shows persistent success and waits for explicit login navigation", async () => { vi.mocked(resetPassword).mockResolvedValue({ success: true, message: "ok" }); renderPage("reset/token"); - await fillPasswords("new-password"); + await fillPasswords("New-password1"); fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); expect(await screen.findByRole("status")).toHaveTextContent("Password changed"); expect(screen.getByRole("status")).toHaveTextContent( "Your password was changed for ContextForge.", ); - expect(resetPassword).toHaveBeenCalledWith("reset/token", "new-password", "new-password"); + expect(resetPassword).toHaveBeenCalledWith("reset/token", "New-password1", "New-password1"); expect(navigate).not.toHaveBeenCalled(); fireEvent.click(screen.getByRole("button", { name: "Return to login" })); expect(navigate).toHaveBeenCalledWith("/app/login"); }); + it("enforces minimum length and complexity before submission", async () => { + renderPage(); + await fillPasswords("short-A1!"); + fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); + expect(await screen.findByText("Password must be at least 12 characters.")).toBeInTheDocument(); + + await fillPasswords("alllowercasepassword"); + fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); + expect( + await screen.findByText( + "Password must contain at least 3 of: uppercase, lowercase, number, or special character.", + ), + ).toBeInTheDocument(); + expect(resetPassword).not.toHaveBeenCalled(); + }); + + it("shows backend password-policy detail without treating link as invalid", async () => { + vi.mocked(resetPassword).mockRejectedValue( + new ApiError(400, { detail: "Password must not be a commonly used password" }, "HTTP 400"), + ); + renderPage(); + await fillPasswords("New-password1"); + fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); + + expect( + await screen.findByText("Password must not be a commonly used password"), + ).toBeInTheDocument(); + expect(screen.queryByText(/reset link is invalid/i)).not.toBeInTheDocument(); + }); + it("offers new link for expired token without exposing token", async () => { vi.mocked(validatePasswordResetToken).mockRejectedValue( new ApiError(410, { detail: "expired reset/token" }, "HTTP 410"), diff --git a/src/pages/ResetPassword.tsx b/src/pages/ResetPassword.tsx index 1b435ed..0a14c94 100644 --- a/src/pages/ResetPassword.tsx +++ b/src/pages/ResetPassword.tsx @@ -1,12 +1,13 @@ import { useEffect, useRef, useState } from "react"; import { CircleCheck } from "lucide-react"; import { useIntl } from "react-intl"; -import { ApiError } from "@/api/client"; import { resetPassword, validatePasswordResetToken } from "@/api/passwordReset"; +import { classifyPasswordResetError } from "@/api/passwordResetErrors"; import { PasswordInput } from "@/components/users/PasswordInput"; import { Button } from "@/components/ui/button"; import { InlineNotification } from "@/components/ui/inline-notification"; import { Loading } from "@/components/ui/loading"; +import { VALIDATION } from "@/lib/constants"; import { useRouter } from "@/router"; type TokenState = "validating" | "valid" | "invalid" | "expired" | "disabled"; @@ -36,8 +37,9 @@ export function ResetPassword({ token = "" }: { token?: string }) { .then((result) => setTokenState(result.valid ? "valid" : "invalid")) .catch((err) => { if (controller.signal.aborted) return; - if (err instanceof ApiError && err.status === 410) setTokenState("expired"); - else if (err instanceof ApiError && err.status === 403) setTokenState("disabled"); + const resetError = classifyPasswordResetError(err); + if (resetError.kind === "expired") setTokenState("expired"); + else if (resetError.kind === "disabled") setTokenState("disabled"); else setTokenState("invalid"); }); @@ -54,10 +56,17 @@ export function ResetPassword({ token = "" }: { token?: string }) { setConfirmPasswordError(null); setSubmitError(null); - if (password.length < 8) { + if (password.length < VALIDATION.MIN_PASSWORD_LENGTH) { setPasswordError(intl.formatMessage({ id: "auth.resetPassword.error.tooShort" })); return; } + const characterTypes = [/[a-z]/, /[A-Z]/, /\d/, /[^A-Za-z0-9]/].filter((pattern) => + pattern.test(password), + ).length; + if (characterTypes < 3) { + setPasswordError(intl.formatMessage({ id: "auth.resetPassword.error.complexity" })); + return; + } if (password !== confirmPassword) { setConfirmPasswordError(intl.formatMessage({ id: "auth.resetPassword.error.mismatch" })); return; @@ -70,9 +79,11 @@ export function ResetPassword({ token = "" }: { token?: string }) { setConfirmPassword(""); setSucceeded(true); } catch (err) { - if (err instanceof ApiError && err.status === 410) setTokenState("expired"); - else if (err instanceof ApiError && err.status === 403) setTokenState("disabled"); - else if (err instanceof ApiError && err.status === 400) + const resetError = classifyPasswordResetError(err); + if (resetError.kind === "expired") setTokenState("expired"); + else if (resetError.kind === "disabled") setTokenState("disabled"); + else if (resetError.kind === "validation") setPasswordError(resetError.message); + else if (resetError.kind === "invalid") setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.invalid" })); else setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.failed" })); } finally { From baadbb2aef8e2928958fc5e756162fa2fbe63b06 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Fri, 14 Aug 2026 13:29:18 +0100 Subject: [PATCH 4/5] fix: handle password reset review findings Signed-off-by: Vishu Bhatnagar --- e2e/users.spec.ts | 4 +- src/api/passwordResetErrors.test.ts | 19 +++----- src/api/passwordResetErrors.ts | 13 +----- src/hooks/useUserForm.test.tsx | 6 +-- src/i18n/locales/en-US/auth.json | 1 + src/i18n/locales/en-US/users.json | 4 +- src/i18n/locales/es-ES/auth.json | 1 + src/i18n/locales/es-ES/users.json | 4 +- src/i18n/locales/pt-BR/auth.json | 1 + src/i18n/locales/pt-BR/users.json | 4 +- src/pages/ResetPassword.test.tsx | 53 ++++++++++++++++++++++ src/pages/ResetPassword.tsx | 70 +++++++++++++++++++++-------- 12 files changed, 127 insertions(+), 53 deletions(-) diff --git a/e2e/users.spec.ts b/e2e/users.spec.ts index 56aa522..6454035 100644 --- a/e2e/users.spec.ts +++ b/e2e/users.spec.ts @@ -212,7 +212,7 @@ test.describe("Users page", () => { await page.getByRole("button", { name: "Create User" }).click(); - await expect(page.getByText("Password must be at least 8 characters")).toBeVisible(); + await expect(page.getByText("Password must be at least 12 characters")).toBeVisible(); }); test("passwords must match", async ({ page }) => { @@ -330,7 +330,7 @@ test.describe("Users page", () => { await page.getByRole("button", { name: "Save Changes" }).click(); await expect.poll(() => patchCalled).toBe(true); - await expect(page.getByText("Password must be at least 8 characters")).not.toBeVisible(); + await expect(page.getByText("Password must be at least 12 characters")).not.toBeVisible(); }); test("passwords must match if password is filled", async ({ page }) => { diff --git a/src/api/passwordResetErrors.test.ts b/src/api/passwordResetErrors.test.ts index eb28b66..20873b7 100644 --- a/src/api/passwordResetErrors.test.ts +++ b/src/api/passwordResetErrors.test.ts @@ -14,25 +14,18 @@ describe("classifyPasswordResetError", () => { }); }); - it("preserves actionable backend password-policy detail", () => { - const detail = "Password must contain at least 3 character types"; + it("preserves 400 detail without guessing its meaning from wording", () => { + const detail = "El enlace ya no es válido"; expect(classifyPasswordResetError(new ApiError(400, { detail }, "HTTP 400"))).toEqual({ - kind: "validation", + kind: "badRequest", message: detail, }); }); - it("uses invalid fallback when a 400 has no safe detail", () => { + it("handles a 400 without detail", () => { expect(classifyPasswordResetError(new ApiError(400, null, "HTTP 400"))).toEqual({ - kind: "invalid", + kind: "badRequest", + message: null, }); }); - - it("classifies token detail as an invalid link rather than password validation", () => { - expect( - classifyPasswordResetError( - new ApiError(400, { detail: "This reset link is invalid" }, "HTTP 400"), - ), - ).toEqual({ kind: "invalid" }); - }); }); diff --git a/src/api/passwordResetErrors.ts b/src/api/passwordResetErrors.ts index 8037ff6..e1cf282 100644 --- a/src/api/passwordResetErrors.ts +++ b/src/api/passwordResetErrors.ts @@ -4,9 +4,8 @@ import { extractApiErrorDetail } from "@/utils/errors"; export type PasswordResetError = | { kind: "disabled" } | { kind: "expired" } - | { kind: "invalid" } + | { kind: "badRequest"; message: string | null } | { kind: "rateLimited" } - | { kind: "validation"; message: string } | { kind: "failed" }; /** Classify password-reset API failures in one place for both public auth screens. */ @@ -18,15 +17,7 @@ export function classifyPasswordResetError(error: unknown): PasswordResetError { if (error.status === 429) return { kind: "rateLimited" }; if (error.status === 400) { - const detail = extractApiErrorDetail(error.body); - if (!detail) return { kind: "invalid" }; - - const normalizedDetail = detail.toLowerCase(); - if (normalizedDetail.includes("reset link") || normalizedDetail.includes("token")) { - return { kind: "invalid" }; - } - - return { kind: "validation", message: detail }; + return { kind: "badRequest", message: extractApiErrorDetail(error.body) }; } return { kind: "failed" }; diff --git a/src/hooks/useUserForm.test.tsx b/src/hooks/useUserForm.test.tsx index 09f9480..a406e40 100644 --- a/src/hooks/useUserForm.test.tsx +++ b/src/hooks/useUserForm.test.tsx @@ -13,7 +13,7 @@ vi.mock("@/hooks/useQuery", () => ({ const messages = { "users.form.error.emailInvalid": "Invalid email address", - "users.form.error.passwordMinLength": "Password must be at least 8 characters", + "users.form.error.passwordMinLength": "Password must be at least 12 characters", "users.form.error.passwordsDoNotMatch": "Passwords do not match", "users.form.error.createFailed": "Failed to create user", "users.form.error.updateFailed": "Failed to update user", @@ -293,7 +293,7 @@ describe("useUserForm", () => { vi.advanceTimersByTime(300); }); - expect(result.current.errors.password).toBe("Password must be at least 8 characters"); + expect(result.current.errors.password).toBe("Password must be at least 12 characters"); act(() => { result.current.validateField("password", "LongEnoughPassword123!"); @@ -419,7 +419,7 @@ describe("useUserForm", () => { }); expect(isValid).toBe(false); - expect(result.current.errors.password).toBe("Password must be at least 8 characters"); + expect(result.current.errors.password).toBe("Password must be at least 12 characters"); }); it("should return false for mismatched passwords", () => { diff --git a/src/i18n/locales/en-US/auth.json b/src/i18n/locales/en-US/auth.json index 52a3871..df243a1 100644 --- a/src/i18n/locales/en-US/auth.json +++ b/src/i18n/locales/en-US/auth.json @@ -33,6 +33,7 @@ "auth.resetPassword.error.invalid": "This reset link is invalid or has already been used.", "auth.resetPassword.error.expired": "This reset link has expired.", "auth.resetPassword.error.disabled": "Password reset is currently unavailable.", + "auth.resetPassword.error.rateLimited": "Too many requests. Please try again later.", "auth.resetPassword.error.failed": "We could not reset your password. Please try again.", "auth.changePassword.title": "Change Password", "auth.changePassword.currentPassword": "Current Password", diff --git a/src/i18n/locales/en-US/users.json b/src/i18n/locales/en-US/users.json index 07419a3..b74310b 100644 --- a/src/i18n/locales/en-US/users.json +++ b/src/i18n/locales/en-US/users.json @@ -18,14 +18,14 @@ "users.form.error.createFailed": "Failed to create user. Please try again.", "users.form.error.emailInvalid": "Invalid email address", "users.form.error.emailRequired": "Email is required", - "users.form.error.passwordMinLength": "Password must be at least 8 characters", + "users.form.error.passwordMinLength": "Password must be at least 12 characters", "users.form.error.passwordRequired": "Password is required", "users.form.error.passwordsDoNotMatch": "Passwords do not match", "users.form.fullName.placeholder": "John Doe", "users.form.fullName": "Full Name", "users.form.isActive": "Account is active", "users.form.isAdmin": "Grant admin privileges", - "users.form.password.placeholder": "Enter password (min 8 characters)", + "users.form.password.placeholder": "Enter password (min 12 characters)", "users.form.password": "Password", "users.form.passwordChangeRequired": "Require password change on first login", "users.form.title": "Create User", diff --git a/src/i18n/locales/es-ES/auth.json b/src/i18n/locales/es-ES/auth.json index e9e6cf0..b63afe5 100644 --- a/src/i18n/locales/es-ES/auth.json +++ b/src/i18n/locales/es-ES/auth.json @@ -33,6 +33,7 @@ "auth.resetPassword.error.invalid": "Este enlace de restablecimiento no es válido o ya fue utilizado.", "auth.resetPassword.error.expired": "Este enlace de restablecimiento ha caducado.", "auth.resetPassword.error.disabled": "El restablecimiento de contraseña no está disponible actualmente.", + "auth.resetPassword.error.rateLimited": "Demasiadas solicitudes. Inténtalo de nuevo más tarde.", "auth.resetPassword.error.failed": "No pudimos restablecer tu contraseña. Inténtalo de nuevo.", "auth.changePassword.title": "Cambiar Contraseña", "auth.changePassword.currentPassword": "Contraseña Actual", diff --git a/src/i18n/locales/es-ES/users.json b/src/i18n/locales/es-ES/users.json index 699e546..ecd8390 100644 --- a/src/i18n/locales/es-ES/users.json +++ b/src/i18n/locales/es-ES/users.json @@ -18,14 +18,14 @@ "users.form.error.createFailed": "Error al crear usuario. Por favor, inténtelo de nuevo.", "users.form.error.emailInvalid": "Correo electrónico inválido", "users.form.error.emailRequired": "El correo electrónico es obligatorio", - "users.form.error.passwordMinLength": "La contraseña debe tener al menos 8 caracteres", + "users.form.error.passwordMinLength": "La contraseña debe tener al menos 12 caracteres", "users.form.error.passwordRequired": "La contraseña es obligatoria", "users.form.error.passwordsDoNotMatch": "Las contraseñas no coinciden", "users.form.fullName.placeholder": "Juan Pérez", "users.form.fullName": "Nombre Completo", "users.form.isActive": "La cuenta está activa", "users.form.isAdmin": "Otorgar privilegios de administrador", - "users.form.password.placeholder": "Ingrese contraseña (mín. 8 caracteres)", + "users.form.password.placeholder": "Ingrese contraseña (mín. 12 caracteres)", "users.form.password": "Contraseña", "users.form.passwordChangeRequired": "Requerir cambio de contraseña en el primer inicio de sesión", "users.form.title": "Crear Usuario", diff --git a/src/i18n/locales/pt-BR/auth.json b/src/i18n/locales/pt-BR/auth.json index 4d22d93..55dd4a8 100644 --- a/src/i18n/locales/pt-BR/auth.json +++ b/src/i18n/locales/pt-BR/auth.json @@ -33,6 +33,7 @@ "auth.resetPassword.error.invalid": "Este link de redefinição é inválido ou já foi usado.", "auth.resetPassword.error.expired": "Este link de redefinição expirou.", "auth.resetPassword.error.disabled": "A redefinição de senha não está disponível no momento.", + "auth.resetPassword.error.rateLimited": "Muitas solicitações. Tente novamente mais tarde.", "auth.resetPassword.error.failed": "Não foi possível redefinir sua senha. Tente novamente.", "auth.changePassword.title": "Alterar Senha", "auth.changePassword.currentPassword": "Senha Atual", diff --git a/src/i18n/locales/pt-BR/users.json b/src/i18n/locales/pt-BR/users.json index 2e5cfcd..85cc0e4 100644 --- a/src/i18n/locales/pt-BR/users.json +++ b/src/i18n/locales/pt-BR/users.json @@ -18,14 +18,14 @@ "users.form.error.createFailed": "Falha ao criar usuário. Por favor, tente novamente.", "users.form.error.emailInvalid": "E-mail inválido", "users.form.error.emailRequired": "E-mail é obrigatório", - "users.form.error.passwordMinLength": "A senha deve ter pelo menos 8 caracteres", + "users.form.error.passwordMinLength": "A senha deve ter pelo menos 12 caracteres", "users.form.error.passwordRequired": "Senha é obrigatória", "users.form.error.passwordsDoNotMatch": "As senhas não coincidem", "users.form.fullName.placeholder": "João Silva", "users.form.fullName": "Nome Completo", "users.form.isActive": "Conta está ativa", "users.form.isAdmin": "Conceder privilégios de administrador", - "users.form.password.placeholder": "Digite a senha (mín. 8 caracteres)", + "users.form.password.placeholder": "Digite a senha (mín. 12 caracteres)", "users.form.password": "Senha", "users.form.passwordChangeRequired": "Exigir alteração de senha no primeiro login", "users.form.title": "Criar Usuário", diff --git a/src/pages/ResetPassword.test.tsx b/src/pages/ResetPassword.test.tsx index 62a3831..c898112 100644 --- a/src/pages/ResetPassword.test.tsx +++ b/src/pages/ResetPassword.test.tsx @@ -59,6 +59,11 @@ describe("ResetPassword", () => { expect(await screen.findAllByText("Passwords do not match.")).not.toHaveLength(0); expect(resetPassword).not.toHaveBeenCalled(); + + fireEvent.change(document.getElementById("new-password")!, { + target: { value: "Password-two2" }, + }); + expect(screen.queryAllByText("Passwords do not match.")).toHaveLength(0); }); it("shows persistent success and waits for explicit login navigation", async () => { @@ -106,6 +111,27 @@ describe("ResetPassword", () => { await screen.findByText("Password must not be a commonly used password"), ).toBeInTheDocument(); expect(screen.queryByText(/reset link is invalid/i)).not.toBeInTheDocument(); + expect(validatePasswordResetToken).toHaveBeenCalledTimes(2); + }); + + it("revalidates a failed submission to distinguish an invalid token from policy errors", async () => { + vi.mocked(validatePasswordResetToken) + .mockResolvedValueOnce({ valid: true, message: "valid", expires_at: null }) + .mockRejectedValueOnce( + new ApiError(400, { detail: "El enlace ya no es válido" }, "HTTP 400"), + ); + vi.mocked(resetPassword).mockRejectedValue( + new ApiError(400, { detail: "Wording must not determine behavior" }, "HTTP 400"), + ); + renderPage(); + await fillPasswords("New-password1"); + fireEvent.click(screen.getByRole("button", { name: "Reset Password" })); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "This reset link is invalid or has already been used.", + ); + expect(screen.getByRole("button", { name: "Request New Link" })).toBeInTheDocument(); + expect(validatePasswordResetToken).toHaveBeenCalledTimes(2); }); it("offers new link for expired token without exposing token", async () => { @@ -119,4 +145,31 @@ describe("ResetPassword", () => { fireEvent.click(screen.getByRole("button", { name: "Request New Link" })); expect(navigate).toHaveBeenCalledWith("/app/forgot-password"); }); + + it("does not encourage another reset request when token validation is rate limited", async () => { + vi.mocked(validatePasswordResetToken).mockRejectedValue( + new ApiError(429, { detail: "Too many requests" }, "HTTP 429"), + ); + renderPage(); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "Too many requests. Please try again later.", + ); + expect(screen.queryByRole("button", { name: "Request New Link" })).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Return to login" })); + expect(navigate).toHaveBeenCalledWith("/app/login"); + }); + + it("does not offer a dead-end reset request when password reset is disabled", async () => { + vi.mocked(validatePasswordResetToken).mockRejectedValue( + new ApiError(403, { detail: "Password reset is disabled" }, "HTTP 403"), + ); + renderPage(); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "Password reset is currently unavailable.", + ); + expect(screen.queryByRole("button", { name: "Request New Link" })).not.toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Return to login" })).toBeInTheDocument(); + }); }); diff --git a/src/pages/ResetPassword.tsx b/src/pages/ResetPassword.tsx index 0a14c94..9d6f76e 100644 --- a/src/pages/ResetPassword.tsx +++ b/src/pages/ResetPassword.tsx @@ -2,7 +2,7 @@ import { useEffect, useRef, useState } from "react"; import { CircleCheck } from "lucide-react"; import { useIntl } from "react-intl"; import { resetPassword, validatePasswordResetToken } from "@/api/passwordReset"; -import { classifyPasswordResetError } from "@/api/passwordResetErrors"; +import { classifyPasswordResetError, type PasswordResetError } from "@/api/passwordResetErrors"; import { PasswordInput } from "@/components/users/PasswordInput"; import { Button } from "@/components/ui/button"; import { InlineNotification } from "@/components/ui/inline-notification"; @@ -10,7 +10,18 @@ import { Loading } from "@/components/ui/loading"; import { VALIDATION } from "@/lib/constants"; import { useRouter } from "@/router"; -type TokenState = "validating" | "valid" | "invalid" | "expired" | "disabled"; +type TokenState = + "validating" | "valid" | "invalid" | "expired" | "disabled" | "rateLimited" | "failed"; + +function tokenStateFromError( + error: PasswordResetError, +): Exclude { + if (error.kind === "expired") return "expired"; + if (error.kind === "disabled") return "disabled"; + if (error.kind === "rateLimited") return "rateLimited"; + if (error.kind === "badRequest") return "invalid"; + return "failed"; +} export function ResetPassword({ token = "" }: { token?: string }) { const intl = useIntl(); @@ -37,10 +48,7 @@ export function ResetPassword({ token = "" }: { token?: string }) { .then((result) => setTokenState(result.valid ? "valid" : "invalid")) .catch((err) => { if (controller.signal.aborted) return; - const resetError = classifyPasswordResetError(err); - if (resetError.kind === "expired") setTokenState("expired"); - else if (resetError.kind === "disabled") setTokenState("disabled"); - else setTokenState("invalid"); + setTokenState(tokenStateFromError(classifyPasswordResetError(err))); }); return () => controller.abort(); @@ -82,21 +90,38 @@ export function ResetPassword({ token = "" }: { token?: string }) { const resetError = classifyPasswordResetError(err); if (resetError.kind === "expired") setTokenState("expired"); else if (resetError.kind === "disabled") setTokenState("disabled"); - else if (resetError.kind === "validation") setPasswordError(resetError.message); - else if (resetError.kind === "invalid") - setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.invalid" })); - else setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.failed" })); + else if (resetError.kind === "rateLimited") + setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.rateLimited" })); + else if (resetError.kind === "badRequest") { + // Backend uses HTTP 400 for both invalid tokens and password-policy failures. + // Revalidate instead of inspecting human-readable (and potentially localized) detail. + try { + const validation = await validatePasswordResetToken(token); + if (!validation.valid) setTokenState("invalid"); + else if (resetError.message) setPasswordError(resetError.message); + else setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.failed" })); + } catch (validationError) { + setTokenState(tokenStateFromError(classifyPasswordResetError(validationError))); + } + } else setSubmitError(intl.formatMessage({ id: "auth.resetPassword.error.failed" })); } finally { setSubmitting(false); } } - const tokenErrorMessage = - tokenState === "expired" - ? intl.formatMessage({ id: "auth.resetPassword.error.expired" }) - : tokenState === "disabled" - ? intl.formatMessage({ id: "auth.resetPassword.error.disabled" }) - : intl.formatMessage({ id: "auth.resetPassword.error.invalid" }); + const tokenErrorMessage = intl.formatMessage({ + id: + tokenState === "expired" + ? "auth.resetPassword.error.expired" + : tokenState === "disabled" + ? "auth.resetPassword.error.disabled" + : tokenState === "rateLimited" + ? "auth.resetPassword.error.rateLimited" + : tokenState === "failed" + ? "auth.resetPassword.error.failed" + : "auth.resetPassword.error.invalid", + }); + const canRequestNewLink = tokenState === "invalid" || tokenState === "expired"; return (
@@ -156,9 +181,15 @@ export function ResetPassword({ token = "" }: { token?: string }) { ) : ( @@ -169,6 +200,8 @@ export function ResetPassword({ token = "" }: { token?: string }) { onChange={(value) => { setPassword(value); setPasswordError(null); + setConfirmPasswordError(null); + setSubmitError(null); }} label={intl.formatMessage({ id: "auth.resetPassword.password" })} placeholder={intl.formatMessage({ id: "auth.resetPassword.password" })} @@ -182,6 +215,7 @@ export function ResetPassword({ token = "" }: { token?: string }) { onChange={(value) => { setConfirmPassword(value); setConfirmPasswordError(null); + setSubmitError(null); }} label={intl.formatMessage({ id: "auth.resetPassword.confirmPassword" })} placeholder={intl.formatMessage({ id: "auth.resetPassword.confirmPassword" })} From 0192fa0a5f2612b37dcc725e7d81c44909d84e92 Mon Sep 17 00:00:00 2001 From: Vishu Bhatnagar Date: Fri, 14 Aug 2026 13:53:39 +0100 Subject: [PATCH 5/5] fix: cancel pending combobox blur timer Signed-off-by: Vishu Bhatnagar --- src/components/ui/combobox.test.tsx | 19 ++++++++++++++++++- src/components/ui/combobox.tsx | 15 ++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/components/ui/combobox.test.tsx b/src/components/ui/combobox.test.tsx index 03c933e..845c6e9 100644 --- a/src/components/ui/combobox.test.tsx +++ b/src/components/ui/combobox.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest"; -import { render, screen, within, waitFor } from "@testing-library/react"; +import { fireEvent, render, screen, within, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { Combobox, type ComboboxOption } from "./combobox"; @@ -132,6 +132,23 @@ describe("Combobox", () => { await waitFor(() => expect(screen.queryByRole("listbox")).not.toBeInTheDocument()); }); + it("cancels pending blur work when unmounted", () => { + vi.useFakeTimers(); + try { + const { unmount } = render(); + const input = screen.getByRole("combobox"); + + fireEvent.focus(input); + fireEvent.blur(input); + expect(vi.getTimerCount()).toBe(1); + + unmount(); + expect(vi.getTimerCount()).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + it("closes on Escape without selecting", async () => { const user = userEvent.setup(); const onValueChange = vi.fn(); diff --git a/src/components/ui/combobox.tsx b/src/components/ui/combobox.tsx index 7ffa65a..16af90e 100644 --- a/src/components/ui/combobox.tsx +++ b/src/components/ui/combobox.tsx @@ -40,9 +40,19 @@ export function Combobox({ const [activeIndex, setActiveIndex] = React.useState(-1); const containerRef = React.useRef(null); const inputRef = React.useRef(null); + const blurTimeoutRef = React.useRef | null>(null); const listboxId = React.useId(); const optionIdPrefix = React.useId(); + const clearBlurTimeout = React.useCallback(() => { + if (blurTimeoutRef.current !== null) { + clearTimeout(blurTimeoutRef.current); + blurTimeoutRef.current = null; + } + }, []); + + React.useEffect(() => clearBlurTimeout, [clearBlurTimeout]); + const selectedOption = options.find((opt) => opt.value === value); const displayLabel = selectedOption?.label || value || ""; @@ -87,6 +97,7 @@ export function Combobox({ }, [activeIndex, open]); const handleOpen = () => { + clearBlurTimeout(); if (!disabled) { setOpen(true); setSearchValue(""); @@ -151,7 +162,9 @@ export function Combobox({ }; const handleBlur = () => { - setTimeout(() => { + clearBlurTimeout(); + blurTimeoutRef.current = setTimeout(() => { + blurTimeoutRef.current = null; if (!containerRef.current?.contains(document.activeElement)) { setOpen(false); setSearchValue("");