diff --git a/e2e/auth/login-flow.spec.ts b/e2e/auth/login-flow.spec.ts index 6bbf51e..16439b3 100644 --- a/e2e/auth/login-flow.spec.ts +++ b/e2e/auth/login-flow.spec.ts @@ -44,6 +44,42 @@ test.describe("Login flow", () => { expect(token).toBeNull(); }); + test("403 password-change-required response redirects to the change-password page", async ({ + page, + apiMock, + }) => { + await apiMock.mockLogin({ + status: 403, + detail: JSON.stringify({ + detail: "Password change required. Please change your password before continuing.", + }), + }); + + await page.goto(APP.LOGIN); + await page.getByLabel(/email address/i).fill("test@example.com"); + await page.getByLabel(/password/i).fill("old-password"); + await page.getByRole("button", { name: /^sign in$/i }).click(); + + await expect(page).toHaveURL( + new RegExp(`${APP.CHANGE_PASSWORD_REQUIRED}\\?email=test%40example\\.com$`), + ); + }); + + test("403 response with unrelated detail keeps user on the login page", async ({ + page, + apiMock, + }) => { + await apiMock.mockLogin({ status: 403, detail: JSON.stringify({ detail: "Forbidden" }) }); + + await page.goto(APP.LOGIN); + await page.getByLabel(/email address/i).fill("test@example.com"); + await page.getByLabel(/password/i).fill("password123"); + await page.getByRole("button", { name: /^sign in$/i }).click(); + + await expect(page).toHaveURL(new RegExp(`${APP.LOGIN}$`)); + await expect(page.getByRole("alert")).toHaveText(/login failed/i); + }); + test("500 response surfaces generic failure message", async ({ page, apiMock }) => { await apiMock.mockLogin({ status: 500 }); diff --git a/e2e/auth/password-change-required.spec.ts b/e2e/auth/password-change-required.spec.ts new file mode 100644 index 0000000..f039f35 --- /dev/null +++ b/e2e/auth/password-change-required.spec.ts @@ -0,0 +1,165 @@ +import { test, expect } from "../fixtures/api-mock"; +import { APP, TOKEN_STORAGE_KEY } from "../utils/paths"; + +test.describe("Password change required flow", () => { + test.beforeEach(async ({ page, apiMock }) => { + await apiMock.mockSession({ authenticated: false }); + await page.addInitScript((key) => { + window.sessionStorage.removeItem(key); + }, TOKEN_STORAGE_KEY); + }); + + test("successful change lands the user straight in the app, no manual re-login", async ({ + page, + apiMock, + }) => { + await apiMock.mockChangePasswordRequired(); + + await page.goto(`${APP.CHANGE_PASSWORD_REQUIRED}?email=test%40example.com`); + await expect(page.getByLabel(/email address/i)).toHaveValue("test@example.com"); + + await page.getByLabel(/current password/i).fill("old-password"); + await page.getByLabel(/^new password/i).fill("New-password1"); + await page.getByLabel(/confirm new password/i).fill("New-password1"); + await page.getByRole("button", { name: /change password/i }).click(); + + await expect(page).toHaveURL(new RegExp(`${APP.ROOT}$`)); + await expect(page.getByRole("heading", { name: /dashboard/i })).toBeVisible(); + }); + + test("password changed but auto sign-in failed shows a fallback screen back to login", async ({ + page, + }) => { + await page.route("**/auth/change-password-required", async (route) => { + await route.fulfill({ + status: 502, + contentType: "application/json", + body: JSON.stringify({ error: "login_after_change_failed" }), + }); + }); + + await page.goto(`${APP.CHANGE_PASSWORD_REQUIRED}?email=test%40example.com`); + await page.getByLabel(/current password/i).fill("old-password"); + await page.getByLabel(/^new password/i).fill("New-password1"); + await page.getByLabel(/confirm new password/i).fill("New-password1"); + await page.getByRole("button", { name: /change password/i }).click(); + + await expect(page.getByRole("status")).toHaveText(/password changed/i); + await page.getByRole("button", { name: /return to login/i }).click(); + await expect(page).toHaveURL(new RegExp(`${APP.LOGIN}$`)); + }); + + test("invalid old password shows a fallback link to forgot-password", async ({ + page, + apiMock, + }) => { + await apiMock.mockChangePasswordRequired({ status: 401 }); + + await page.goto(`${APP.CHANGE_PASSWORD_REQUIRED}?email=test%40example.com`); + await page.getByLabel(/current password/i).fill("wrong-old-password"); + await page.getByLabel(/^new password/i).fill("New-password1"); + await page.getByLabel(/confirm new password/i).fill("New-password1"); + await page.getByRole("button", { name: /change password/i }).click(); + + await expect(page.getByRole("alert")).toHaveText(/current password is incorrect/i); + await page.getByRole("button", { name: /forgot your password/i }).click(); + await expect(page).toHaveURL(new RegExp(`${APP.FORGOT_PASSWORD}$`)); + }); + + test("account that doesn't need a change shows a distinct message and a return-to-login CTA", async ({ + page, + }) => { + // Raw route, not the apiMock fixture — this case has its own error code + // ({ error: "password_change_not_required" }), distinct from the + // fixture's generic change_password_failed body for a wrong old password. + await page.route("**/auth/change-password-required", async (route) => { + await route.fulfill({ + status: 403, + contentType: "application/json", + body: JSON.stringify({ error: "password_change_not_required" }), + }); + }); + + await page.goto(`${APP.CHANGE_PASSWORD_REQUIRED}?email=test%40example.com`); + await page.getByLabel(/current password/i).fill("correct-password"); + await page.getByLabel(/^new password/i).fill("New-password1"); + await page.getByLabel(/confirm new password/i).fill("New-password1"); + await page.getByRole("button", { name: /change password/i }).click(); + + const alert = page.getByRole("alert"); + await expect(alert).toHaveText(/doesn't need to be changed/i); + await expect(page.getByRole("button", { name: /forgot your password/i })).toHaveCount(0); + await page.getByRole("button", { name: /return to login/i }).click(); + await expect(page).toHaveURL(new RegExp(`${APP.LOGIN}$`)); + }); + + test("missing ?email= shows an error and a return-to-login CTA instead of a submittable form", async ({ + page, + }) => { + await page.goto(APP.CHANGE_PASSWORD_REQUIRED); + + await expect(page.getByRole("alert")).toHaveText(/couldn't tell which account/i); + await expect(page.getByLabel(/current password/i)).toHaveCount(0); + await page.getByRole("button", { name: /return to login/i }).click(); + await expect(page).toHaveURL(new RegExp(`${APP.LOGIN}$`)); + }); + + test("mismatched new passwords are rejected client-side", async ({ page }) => { + await page.goto(`${APP.CHANGE_PASSWORD_REQUIRED}?email=test%40example.com`); + await page.getByLabel(/current password/i).fill("old-password"); + await page.getByLabel(/^new password/i).fill("New-password1"); + await page.getByLabel(/confirm new password/i).fill("Different-password2"); + await page.getByRole("button", { name: /change password/i }).click(); + + await expect(page.getByText(/passwords do not match/i)).toBeVisible(); + }); + + test("focus moves to error input field when validation fails", async ({ page }) => { + await page.goto(`${APP.CHANGE_PASSWORD_REQUIRED}?email=test%40example.com`); + + // Trigger new password validation error + await page.getByLabel(/current password/i).fill("old-password"); + await page.getByLabel(/^new password/i).fill("short"); + await page.getByLabel(/confirm new password/i).fill("short"); + await page.getByRole("button", { name: /change password/i }).click(); + + // Verify focus moved to the new password input + await expect(page.getByLabel(/^new password/i)).toBeFocused(); + }); + + test("focus moves to submit error when server returns error", async ({ page, apiMock }) => { + await apiMock.mockChangePasswordRequired({ status: 401 }); + + await page.goto(`${APP.CHANGE_PASSWORD_REQUIRED}?email=test%40example.com`); + await page.getByLabel(/current password/i).fill("wrong-old-password"); + await page.getByLabel(/^new password/i).fill("New-password1"); + await page.getByLabel(/confirm new password/i).fill("New-password1"); + await page.getByRole("button", { name: /change password/i }).click(); + + // Verify focus moved to the error notification container + const errorContainer = page.locator('[role="alert"]').locator(".."); + await expect(errorContainer).toBeFocused(); + }); + + test("success heading receives focus after password changed but login failed", async ({ + page, + }) => { + await page.route("**/auth/change-password-required", async (route) => { + await route.fulfill({ + status: 502, + contentType: "application/json", + body: JSON.stringify({ error: "login_after_change_failed" }), + }); + }); + + await page.goto(`${APP.CHANGE_PASSWORD_REQUIRED}?email=test%40example.com`); + await page.getByLabel(/current password/i).fill("old-password"); + await page.getByLabel(/^new password/i).fill("New-password1"); + await page.getByLabel(/confirm new password/i).fill("New-password1"); + await page.getByRole("button", { name: /change password/i }).click(); + + // Verify focus moved to the success heading + const successHeading = page.getByRole("heading", { name: /password changed/i }); + await expect(successHeading).toBeFocused(); + }); +}); diff --git a/e2e/fixtures/api-mock.ts b/e2e/fixtures/api-mock.ts index 2858125..1302360 100644 --- a/e2e/fixtures/api-mock.ts +++ b/e2e/fixtures/api-mock.ts @@ -49,6 +49,18 @@ export interface ApiMock { */ mockPermissions(options?: { permissions?: string[] }): Promise; mockUnauthorized(urlPattern: string | RegExp): Promise; + /** + * Mocks POST /auth/change-password-required, the BFF's route used by + * PasswordChangeRequired.tsx (client/src/pages/) after a "password change + * required" login failure. On success it returns the same { user, + * csrfToken } shape as /auth/login — the BFF re-authenticates with the new + * password and establishes a real session as part of this one call. + */ + mockChangePasswordRequired(options?: { + user?: MockUser; + status?: number; + detail?: string; + }): Promise; } export function createApiMock(page: Page): ApiMock { @@ -102,6 +114,28 @@ export function createApiMock(page: Page): ApiMock { }); }, + async mockChangePasswordRequired({ + user = DEFAULT_TEST_USER, + status = 200, + detail = "Invalid credentials", + } = {}) { + await page.route("**/auth/change-password-required", async (route) => { + if (status === 200) { + await route.fulfill({ + status, + contentType: "application/json", + body: JSON.stringify({ user, csrfToken: MOCK_CSRF_TOKEN }), + }); + return; + } + await route.fulfill({ + status, + contentType: "application/json", + body: JSON.stringify({ error: "change_password_failed", detail }), + }); + }); + }, + async mockUnauthorized(urlPattern) { await page.route(urlPattern, async (route) => { await route.fulfill({ diff --git a/e2e/utils/paths.ts b/e2e/utils/paths.ts index 5cd9627..5828af4 100644 --- a/e2e/utils/paths.ts +++ b/e2e/utils/paths.ts @@ -10,6 +10,7 @@ export const APP = { LOGIN: "/app/login", FORGOT_PASSWORD: "/app/forgot-password", CHANGE_PASSWORD: "/app/change-password", // pragma: allowlist secret + CHANGE_PASSWORD_REQUIRED: "/app/change-password-required", // pragma: allowlist secret GATEWAYS: "/app/gateways", SERVERS: "/app/servers", TOOLS: "/app/tools", @@ -40,6 +41,7 @@ export const APP = { export const API = { LOGIN: "**/auth/login", SESSION: "**/auth/session", + CHANGE_PASSWORD_REQUIRED: "**/auth/change-password-required", } as const; /** diff --git a/server/src/index.ts b/server/src/index.ts index 2294db8..2147991 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -16,6 +16,7 @@ import redisPlugin from "./plugins/redis.js"; import sessionPlugin from "./plugins/session.js"; import staticPlugin from "./plugins/static.js"; import appRoute from "./routes/app.js"; +import changePasswordRequiredRoute from "./routes/auth/change-password-required.js"; import loginRoute from "./routes/auth/login.js"; import logoutRoute from "./routes/auth/logout.js"; import sessionRoute from "./routes/auth/session.js"; @@ -37,6 +38,7 @@ fastify.get("/healthz", async () => ({ ok: true })); await fastify.register(loginRoute); await fastify.register(logoutRoute); await fastify.register(sessionRoute); +await fastify.register(changePasswordRequiredRoute); await fastify.register(sseRoutes); await fastify.register(catchAllProxyRoute); await fastify.register(appRoute); diff --git a/server/src/lib/establish-session.ts b/server/src/lib/establish-session.ts new file mode 100644 index 0000000..d9af2cf --- /dev/null +++ b/server/src/lib/establish-session.ts @@ -0,0 +1,92 @@ +// Location: ./client/server/src/lib/establish-session.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// Given an upstream AuthenticationResponse, create the BFF session + rotate +// the CSRF secret + hand back what the caller needs to reply with. Shared by +// routes/auth/login.ts and routes/auth/change-password-required.ts so the +// JWT-lifetime/session-TTL matching and CSRF-rotation security properties +// live in exactly one place. + +import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; + +import { config } from "../config.js"; +import { createSession, setSessionCookie, type SessionUser } from "./session-store.js"; +import { CSRF_COOKIE_NAME } from "../plugins/csrf.js"; + +// Mirrors mcpgateway.schemas.AuthenticationResponse. expires_in is optional — +// not every upstream login-shaped endpoint this BFF calls is guaranteed to +// send it (see lib/upstream-login.ts's Tier-1 /auth/login bypass call). +export interface UpstreamAuthenticationResponse { + access_token: string; + expires_in?: number; + user: SessionUser; +} + +/** + * Thrown when the upstream auth response still reports + * password_change_required=true. This is the single chokepoint every + * session-establishing route goes through, so it's also the single place + * that guarantees a session is never minted for a still-flagged account — + * callers don't each have to re-implement that check correctly. In today's + * two callers this should never actually trip (routes/auth/login.ts only + * gets here after upstream's own 2xx says the account is clear; + * routes/auth/change-password-required.ts only gets here after a successful + * change), but it's the backstop for any future login path (SSO callback, + * admin impersonation, ...) that calls establishSession() directly. + */ +export class PasswordChangeStillRequiredError extends Error { + constructor() { + super("upstream auth response still has password_change_required=true"); + this.name = "PasswordChangeStillRequiredError"; + } +} + +export async function establishSession( + fastify: FastifyInstance, + request: FastifyRequest, + reply: FastifyReply, + auth: UpstreamAuthenticationResponse, +): Promise<{ user: SessionUser; csrfToken: string }> { + if (auth.user?.password_change_required === true) { + throw new PasswordChangeStillRequiredError(); + } + + // The BFF session/cookie must not outlive the bearer token it wraps — use + // the upstream JWT's own lifetime, not a fixed BFF-side default. See + // createSession's comment in lib/session-store.ts. + let ttlSeconds = config.sessionTtlSeconds; + if (Number.isFinite(auth.expires_in) && auth.expires_in! > 0) { + ttlSeconds = auth.expires_in!; + } else if (auth.expires_in !== undefined) { + // Upstream sent expires_in, but it's not a usable positive number — fall + // back, but log it: this means the BFF session can outlive the JWT it + // wraps until the proxy's revoke-on-401 catches up (see session-store.ts). + // A simply *absent* expires_in is not logged here — some upstream + // login-shaped endpoints don't send it by design (see upstream-login.ts). + request.log.warn( + { expires_in: auth.expires_in }, + "upstream login returned invalid expires_in, using BFF default session TTL", + ); + } + + const sessionId = await createSession( + fastify.redis, + { bearerToken: auth.access_token, user: auth.user }, + ttlSeconds, + ); + + setSessionCookie(reply, sessionId, ttlSeconds); + // generateCsrf() only mints a fresh secret when request.cookies has no + // bff_csrf entry — reply.clearCookie() alone doesn't clear that (it only + // queues an outgoing Set-Cookie, request.cookies is untouched), so delete + // it directly to force rotation. Otherwise a secret planted before login + // (subdomain XSS, a plaintext hop with COOKIE_SECURE=false) survives into + // the authenticated session. + delete request.cookies[CSRF_COOKIE_NAME]; + // Cookie holds the CSRF secret (HttpOnly); the SPA needs the derived token + // itself to echo back via X-CSRF-Token — see plugins/csrf.ts. + const csrfToken = await reply.generateCsrf(); + + return { user: auth.user, csrfToken }; +} diff --git a/server/src/lib/revoke-upstream-token.ts b/server/src/lib/revoke-upstream-token.ts new file mode 100644 index 0000000..7bb3687 --- /dev/null +++ b/server/src/lib/revoke-upstream-token.ts @@ -0,0 +1,44 @@ +// Location: ./client/server/src/lib/revoke-upstream-token.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// Revokes an upstream bearer token via FastAPI's bearer-token logout +// (mcpgateway/routers/auth.py POST /auth/logout, blocklist-backed — DB or +// Redis depending on deployment). Without this, a token that's no longer +// reachable from the browser stays cryptographically valid until its +// natural TOKEN_EXPIRY. Best-effort: an upstream failure (network blip, +// already-revoked token) must never block whatever the caller is doing. +// +// Shared by routes/auth/logout.ts (revoking the real session token) and +// routes/auth/change-password-required.ts (revoking the short-lived bypass +// token minted via /auth/login). + +import type { FastifyRequest } from "fastify"; + +import { config } from "../config.js"; +import { upstreamAuthHeader } from "./upstream-auth.js"; + +// The caller is usually waiting on this request, so cap how long a hung +// (not refused) upstream can hold it open. +const UPSTREAM_REVOKE_TIMEOUT_MS = 3000; + +export async function revokeUpstreamToken( + request: FastifyRequest, + bearerToken: string, +): Promise { + try { + const response = await fetch(`${config.contextforgeUrl}/auth/logout`, { + method: "POST", + headers: upstreamAuthHeader(bearerToken), + signal: AbortSignal.timeout(UPSTREAM_REVOKE_TIMEOUT_MS), + }); + if (!response.ok) { + request.log.warn( + { status: response.status }, + "upstream token revocation returned a non-2xx status", + ); + } + } catch (err) { + request.log.warn({ err }, "upstream token revocation failed"); + } +} diff --git a/server/src/lib/upstream-login.ts b/server/src/lib/upstream-login.ts new file mode 100644 index 0000000..c814f61 --- /dev/null +++ b/server/src/lib/upstream-login.ts @@ -0,0 +1,75 @@ +// Location: ./client/server/src/lib/upstream-login.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// Shared "call an upstream login-shaped endpoint, validate the 2xx body" +// logic. Used by routes/auth/login.ts (Tier-2 /auth/email/login) and +// routes/auth/change-password-required.ts (both the Tier-2 precondition +// check and the Tier-1 /auth/login bypass, plus the post-change Tier-2 +// login) so the fetch/timeout/header/JSON-parse/access_token-check pattern +// lives in exactly one place instead of being copied per call site. + +import type { FastifyRequest } from "fastify"; + +import { config } from "../config.js"; +import type { UpstreamAuthenticationResponse } from "./establish-session.js"; + +// A hung (not refused) upstream must not hold the request open indefinitely +// — same rationale as revoke-upstream-token.ts's UPSTREAM_REVOKE_TIMEOUT_MS. +const UPSTREAM_LOGIN_TIMEOUT_MS = 3000; + +export type UpstreamLoginResult = + | { ok: true; auth: UpstreamAuthenticationResponse } + | { ok: false; kind: "unavailable" } + | { ok: false; kind: "rejected"; status: number; detail: string } + | { ok: false; kind: "invalid_response" }; + +/** + * POSTs { email, password } to an upstream login-shaped path and validates + * the 2xx body. `path` is the upstream path only (e.g. "/auth/login" or + * "/auth/email/login") — config.contextforgeUrl is prepended. + */ +export async function upstreamLogin( + request: FastifyRequest, + path: string, + email: string, + password: string, +): Promise { + let response: Response; + try { + response = await fetch(`${config.contextforgeUrl}${path}`, { + method: "POST", + headers: { + "content-type": "application/json", + // Preserve real client IP for upstream audit logging. + "x-forwarded-for": request.ip, + "x-real-ip": request.ip, + }, + body: JSON.stringify({ email, password }), + signal: AbortSignal.timeout(UPSTREAM_LOGIN_TIMEOUT_MS), + }); + } catch (err) { + request.log.error({ err, path }, "upstream login request failed"); + return { ok: false, kind: "unavailable" }; + } + + if (!response.ok) { + const detail = await response.text(); + return { ok: false, kind: "rejected", status: response.status, detail }; + } + + let auth: UpstreamAuthenticationResponse; // pragma: allowlist secret + try { + auth = (await response.json()) as UpstreamAuthenticationResponse; + } catch (err) { + request.log.error({ err, path }, "upstream login returned a non-JSON 2xx body"); + return { ok: false, kind: "invalid_response" }; + } + + if (typeof auth.access_token !== "string" || !auth.access_token) { + request.log.error({ auth, path }, "upstream login 2xx response missing access_token"); + return { ok: false, kind: "invalid_response" }; + } + + return { ok: true, auth }; +} diff --git a/server/src/routes/auth/change-password-required.ts b/server/src/routes/auth/change-password-required.ts new file mode 100644 index 0000000..f1b3ff6 --- /dev/null +++ b/server/src/routes/auth/change-password-required.ts @@ -0,0 +1,185 @@ +// Location: ./client/server/src/routes/auth/change-password-required.ts +// Copyright contributors to the MCP-CONTEXT-FORGE project +// SPDX-License-Identifier: Apache-2.0 +// +// POST /auth/change-password-required: browser -> BFF only, pre-login. Used +// when /auth/login rejected valid credentials with a "password change +// required" 403 (see routes/auth/login.ts). This route: +// +// 1. Verifies the precondition via the SAME endpoint/logic that produced +// the original block: upstream's /auth/email/login, with the OLD +// password. Deliberately NOT a check against a persisted flag — upstream +// computes "needs password change" from several independent sources +// (a persisted flag, password-age expiry, default-password detection), +// and only /auth/email/login's own 403 reflects all of them. A 403 here +// also proves the old password is correct (upstream validates +// credentials before deciding whether to block). A 200 here means the +// account does NOT currently need a change — reject without ever +// minting a bypass token. +// 2. Mints a short-lived bypass token via upstream's plain /auth/login +// ("Tier 1" session auth), which does not enforce the block, using the +// same OLD password step 1 just validated. +// 3. Uses that bypass token once to call the authenticated change-password +// endpoint. +// 4. Revokes the bypass token upstream (best-effort, fire-and-forget — it's +// served its one purpose and shouldn't linger, but nothing downstream +// depends on the revoke actually completing). +// 5. Logs in again for real, via the same upstream /auth/email/login +// login.ts uses, now with the NEW password (the block is gone once the +// password's been changed). +// 6. Establishes a normal BFF session from that — identical to a plain +// login, so a successful password change lands the user straight in +// the app. +// +// On success this returns the exact same { user, csrfToken } shape as +// POST /auth/login, so the SPA can treat it identically to a login. + +import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; + +import { config } from "../../config.js"; +import { establishSession, PasswordChangeStillRequiredError } from "../../lib/establish-session.js"; +import { revokeUpstreamToken } from "../../lib/revoke-upstream-token.js"; +import { setNoStore } from "../../lib/no-store.js"; +import { isForbiddenCrossOrigin } from "../../lib/origin-guard.js"; +import { upstreamAuthHeader } from "../../lib/upstream-auth.js"; +import { upstreamLogin } from "../../lib/upstream-login.js"; + +interface ChangePasswordRequiredBody { + email: string; + oldPassword: string; + newPassword: string; +} + +// Caps the change-password call itself — the two login-shaped calls already +// time out via upstreamLogin()'s own UPSTREAM_LOGIN_TIMEOUT_MS. +const UPSTREAM_REQUEST_TIMEOUT_MS = 3000; + +export default async function changePasswordRequiredRoute(fastify: FastifyInstance): Promise { + fastify.post<{ Body: ChangePasswordRequiredBody }>( + "/auth/change-password-required", + async (request: FastifyRequest<{ Body: ChangePasswordRequiredBody }>, reply: FastifyReply) => { + setNoStore(reply); + + if (isForbiddenCrossOrigin(request)) { + return reply.code(403).send({ error: "cross_site_request_forbidden" }); + } + + const { email, oldPassword, newPassword } = request.body ?? {}; + if (!email || !oldPassword || !newPassword) { + return reply.code(400).send({ error: "email, oldPassword and newPassword are required" }); + } + + // Step 1: precondition + credential check, via the endpoint that + // actually computes "needs password change" (persisted flag, password + // age, default-password detection — see module comment above). + const precondition = await upstreamLogin(request, "/auth/email/login", email, oldPassword); + + if (precondition.ok) { + // Correct old password, but the account doesn't currently need a + // change (stale link, flag cleared elsewhere, etc.) — nothing to do, + // and no bypass token was ever minted. Revoke the token this + // legitimate login just handed us, since we're not using it. + void revokeUpstreamToken(request, precondition.auth.access_token); + return reply.code(403).send({ error: "password_change_not_required" }); + } + + if (precondition.kind === "unavailable") { + return reply.code(502).send({ error: "upstream_unavailable" }); + } + if (precondition.kind === "invalid_response") { + return reply.code(502).send({ error: "upstream_invalid_response" }); + } + if (precondition.status !== 403) { + // 401 (wrong old password), 429 (rate-limited), etc. — pass through + // as-is; don't try to mint a bypass token for a credential the + // precondition check already told us is wrong or blocked. + return reply + .code(precondition.status) + .send({ error: "change_password_failed", detail: precondition.detail }); + } + + // Step 2: mint a bypass token using the same old credentials step 1 + // just validated. Deliberately /auth/login, not /auth/email/login — + // the latter hard-blocks while the account needs a password change, + // the former does not (verified against a live backend). + const bypass = await upstreamLogin(request, "/auth/login", email, oldPassword); + if (!bypass.ok) { + if (bypass.kind === "unavailable") { + return reply.code(502).send({ error: "upstream_unavailable" }); + } + if (bypass.kind === "invalid_response") { + return reply.code(502).send({ error: "upstream_invalid_response" }); + } + return reply + .code(bypass.status) + .send({ error: "change_password_failed", detail: bypass.detail }); + } + const bypassToken = bypass.auth.access_token; + + // Step 3: use the bypass token once, immediately, to change the + // password. Same audit-IP headers as every other upstream call here. + let changeResponse: Response; + try { + changeResponse = await fetch(`${config.contextforgeUrl}/auth/email/change-password`, { + method: "POST", + headers: { + "content-type": "application/json", + "x-forwarded-for": request.ip, + "x-real-ip": request.ip, + ...upstreamAuthHeader(bypassToken), + }, + body: JSON.stringify({ old_password: oldPassword, new_password: newPassword }), + signal: AbortSignal.timeout(UPSTREAM_REQUEST_TIMEOUT_MS), + }); + } catch (err) { + request.log.error({ err }, "upstream change-password failed"); + return reply.code(502).send({ error: "upstream_unavailable" }); + } + + if (!changeResponse.ok) { + // Password change itself failed (e.g. new-password policy violation) + // — the bypass token served no purpose. Fire-and-forget revoke: the + // reply doesn't depend on it, and revokeUpstreamToken never throws. + void revokeUpstreamToken(request, bypassToken); + const detail = await changeResponse.text(); + return reply.code(changeResponse.status).send({ error: "change_password_failed", detail }); + } + + // Step 4: the bypass token has done its one job — revoke it upstream + // rather than let it float until its natural expiry. Best-effort, + // fire-and-forget: nothing below depends on this finishing first. + void revokeUpstreamToken(request, bypassToken); + + // Step 5: the password is changed — log in for real with the new + // password, exactly like login.ts does, to get a full session. + const realLogin = await upstreamLogin(request, "/auth/email/login", email, newPassword); + if (!realLogin.ok) { + // The password WAS changed successfully — this is not a + // change-password failure, it's a (rare) inability to establish a + // session right after. Distinct error so the SPA doesn't show a + // "wrong password"/policy-violation message for a change that + // actually succeeded. + request.log.error({ realLogin }, "password changed but post-change login was rejected"); + return reply.code(502).send({ error: "login_after_change_failed" }); + } + + // Step 6: establish a normal BFF session, identical to login.ts. + try { + const { user, csrfToken } = await establishSession(fastify, request, reply, realLogin.auth); + return reply.send({ user, csrfToken }); + } catch (err) { + if (err instanceof PasswordChangeStillRequiredError) { + // Password changed, but upstream still reports the account as + // flagged (flag not cleared, age not reset, ...) — same "changed + // but couldn't sign back in" story as the branches above. + request.log.error( + { email }, + "password changed but post-change login still flagged password_change_required", + ); + return reply.code(502).send({ error: "login_after_change_failed" }); + } + throw err; + } + }, + ); +} diff --git a/server/src/routes/auth/login.ts b/server/src/routes/auth/login.ts index 0accce5..dd572fe 100644 --- a/server/src/routes/auth/login.ts +++ b/server/src/routes/auth/login.ts @@ -10,16 +10,11 @@ import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify"; import { config } from "../../config.js"; -import { - clearSessionCookie, - createSession, - deleteSession, - setSessionCookie, - SESSION_COOKIE_NAME, - type SessionUser, -} from "../../lib/session-store.js"; +import { clearSessionCookie, deleteSession, SESSION_COOKIE_NAME } from "../../lib/session-store.js"; +import { establishSession, PasswordChangeStillRequiredError } from "../../lib/establish-session.js"; import { setNoStore } from "../../lib/no-store.js"; import { isForbiddenCrossOrigin } from "../../lib/origin-guard.js"; +import { upstreamLogin } from "../../lib/upstream-login.js"; import { CSRF_COOKIE_NAME } from "../../plugins/csrf.js"; interface LoginBody { @@ -32,9 +27,10 @@ interface LoginBody { // the failed attempt and a mutating request made right after can ride that // leftover cookie into a confusing downstream 403 instead of a clean 401. // Called from every same-origin failure exit below (missing credentials, -// upstream unreachable, non-2xx upstream, malformed/incomplete 2xx body) so -// the cleanup can't drift out of sync with the SPA's AuthContext.login(), -// which resets its own state on any rejected login call. +// upstream unreachable, non-2xx upstream, malformed/incomplete 2xx body, +// still-flagged account) so the cleanup can't drift out of sync with the +// SPA's AuthContext.login(), which resets its own state on any rejected +// login call. // Deliberately NOT called from the isForbiddenCrossOrigin() branch above — // that guards against a cross-site page silently POSTing to /auth/login to // mass-clear victims' legitimate sessions; only same-origin login attempts @@ -61,15 +57,6 @@ async function clearStaleSession( reply.clearCookie(CSRF_COOKIE_NAME, { path: "/", domain: config.cookieDomain }); } -// Mirrors mcpgateway.schemas.AuthenticationResponse. `user` is forwarded to -// the browser verbatim (see SessionUser) — the BFF only needs access_token -// and expires_in. -interface UpstreamAuthenticationResponse { - access_token: string; - expires_in: number; - user: SessionUser; -} - export default async function loginRoute(fastify: FastifyInstance): Promise { fastify.post<{ Body: LoginBody }>( "/auth/login", @@ -86,85 +73,44 @@ export default async function loginRoute(fastify: FastifyInstance): Promise 0) { - ttlSeconds = auth.expires_in; - } else { - // Upstream returned a bogus expires_in — fall back, but log it: this - // means the BFF session can outlive the JWT it wraps until the - // proxy's revoke-on-401 catches up (see session-store.ts). - request.log.warn( - { expires_in: auth.expires_in }, - "upstream login returned invalid expires_in, using BFF default session TTL", - ); - } - - const sessionId = await createSession( - fastify.redis, - { - bearerToken: auth.access_token, - user: auth.user, - }, - ttlSeconds, - ); - - setSessionCookie(reply, sessionId, ttlSeconds); - // generateCsrf() only mints a fresh secret when request.cookies has no - // bff_csrf entry — reply.clearCookie() alone doesn't clear that (it - // only queues an outgoing Set-Cookie, request.cookies is untouched), - // so delete it directly to force rotation. Otherwise a secret planted - // before login (subdomain XSS, a plaintext hop with COOKIE_SECURE=false) - // survives into the authenticated session. - delete request.cookies[CSRF_COOKIE_NAME]; - // Cookie holds the CSRF secret (HttpOnly); the SPA needs the derived - // token itself to echo back via X-CSRF-Token — see plugins/csrf.ts. - const csrfToken = await reply.generateCsrf(); - - return reply.send({ user: auth.user, csrfToken }); }, ); } diff --git a/server/src/routes/auth/logout.ts b/server/src/routes/auth/logout.ts index 918d9b7..0f9aefc 100644 --- a/server/src/routes/auth/logout.ts +++ b/server/src/routes/auth/logout.ts @@ -26,31 +26,9 @@ import { SESSION_COOKIE_NAME, } from "../../lib/session-store.js"; import { CSRF_COOKIE_NAME } from "../../plugins/csrf.js"; -import { upstreamAuthHeader } from "../../lib/upstream-auth.js"; +import { revokeUpstreamToken } from "../../lib/revoke-upstream-token.js"; import { setNoStore } from "../../lib/no-store.js"; -// The user is waiting on this request, so cap how long a hung (not refused) -// upstream can hold it open. -const UPSTREAM_REVOKE_TIMEOUT_MS = 3000; - -async function revokeUpstreamToken(request: FastifyRequest, bearerToken: string): Promise { - try { - const response = await fetch(`${config.contextforgeUrl}/auth/logout`, { - method: "POST", - headers: upstreamAuthHeader(bearerToken), - signal: AbortSignal.timeout(UPSTREAM_REVOKE_TIMEOUT_MS), - }); - if (!response.ok) { - request.log.warn( - { status: response.status }, - "upstream token revocation returned a non-2xx status", - ); - } - } catch (err) { - request.log.warn({ err }, "upstream token revocation failed"); - } -} - export default async function logoutRoute(fastify: FastifyInstance): Promise { fastify.post( "/auth/logout", diff --git a/server/test/auth.test.ts b/server/test/auth.test.ts index dbe090e..962360c 100644 --- a/server/test/auth.test.ts +++ b/server/test/auth.test.ts @@ -2,7 +2,7 @@ // Copyright contributors to the MCP-CONTEXT-FORGE project // SPDX-License-Identifier: Apache-2.0 -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { config } from "../src/config.js"; import { buildTestApp, type TestApp } from "./helpers/build-app.js"; @@ -220,6 +220,306 @@ describe("POST /auth/login", () => { expect(response.statusCode).toBe(400); expect(fetchSpy).not.toHaveBeenCalled(); }); + + it("refuses to establish a session if upstream 2xxs but the user is still flagged password_change_required (defensive backstop)", async () => { + const app = await buildTestApp(); + mockUpstreamLogin(true, { + access_token: "upstream-jwt", // pragma: allowlist secret + user: { email: "user@example.com", password_change_required: true }, + }); + + const response = await app.fastify.inject({ + method: "POST", + url: "/auth/login", + payload: { email: "user@example.com", password: "secret" }, // pragma: allowlist secret + }); + + expect(response.statusCode).toBe(403); + expect(response.json().error).toBe("login_failed"); + expect(JSON.parse(response.json().detail).detail).toMatch(/password change required/i); + // clearStaleSession always emits a clearing Set-Cookie for bff_sid (see + // its doc comment) — assert it's cleared, not merely check for absence. + const cleared = response.cookies.find((c) => c.name === "bff_sid"); + expect(cleared?.value).toBe(""); + }); +}); + +describe("POST /auth/change-password-required", () => { + afterEach(() => vi.unstubAllGlobals()); + + const OLD_PASSWORD = "old-secret"; // pragma: allowlist secret + const NEW_PASSWORD = "New-secret1"; // pragma: allowlist secret + + interface UpstreamCall { + url: string; + authorization: string | undefined; + headers: Record; + } + + interface MockLegOptions { + ok?: boolean; + status?: number; + body?: unknown; + } + + interface MockUpstreamOptions { + precondition?: MockLegOptions; // POST /auth/email/login (old password) + bypassLogin?: MockLegOptions; // POST /auth/login (old password) + changePassword?: MockLegOptions; // POST /auth/email/change-password + revoke?: MockLegOptions; // POST /auth/logout + realLogin?: MockLegOptions; // POST /auth/email/login (new password) + } + + /** Mocks every upstream leg the route can call, and records every call made. */ + function mockUpstream(options: MockUpstreamOptions = {}): UpstreamCall[] { + const legs = { + // Default: upstream's own precondition check blocks with 403 — this is + // the ONLY signal the route trusts (see finding #1: not a persisted + // flag, which is why this leg's body carries no + // password_change_required field at all). + precondition: { + ok: false, + status: 403, + body: { + detail: "Password change required. Please change your password before continuing.", + }, + ...options.precondition, + }, + bypassLogin: { + ok: true, + status: 200, + body: { access_token: "bypass-jwt", user: { email: "user@example.com" } }, // pragma: allowlist secret + ...options.bypassLogin, + }, + changePassword: { ok: true, status: 200, body: {}, ...options.changePassword }, + revoke: { ok: true, status: 200, body: {}, ...options.revoke }, + realLogin: { + ok: true, + status: 200, + body: { + access_token: "real-jwt", // pragma: allowlist secret + expires_in: 1200, + user: { email: "user@example.com" }, + }, + ...options.realLogin, + }, + }; + + const calls: UpstreamCall[] = []; + vi.stubGlobal( + "fetch", + vi.fn(async (url: string, init?: RequestInit) => { + const headers = (init?.headers as Record | undefined) ?? {}; + const bodyPassword = init?.body + ? (JSON.parse(String(init.body)) as { password?: string }).password + : undefined; + calls.push({ url: String(url), authorization: headers.authorization, headers }); + + let leg: MockLegOptions & { ok: boolean; status: number; body: unknown }; + if (String(url).endsWith("/auth/email/change-password")) leg = legs.changePassword; + else if (String(url).endsWith("/auth/logout")) leg = legs.revoke; + else if (String(url).endsWith("/auth/login")) leg = legs.bypassLogin; + else if (String(url).endsWith("/auth/email/login")) { + leg = bodyPassword === NEW_PASSWORD ? legs.realLogin : legs.precondition; + } else throw new Error(`unexpected upstream fetch: ${url}`); + + return { + ok: leg.ok, + status: leg.status, + json: async () => leg.body, + text: async () => JSON.stringify(leg.body), + }; + }), + ); + return calls; + } + + async function requestChange(payload?: Record) { + return app!.fastify.inject({ + method: "POST", + url: "/auth/change-password-required", + payload: payload ?? { + email: "user@example.com", + oldPassword: OLD_PASSWORD, + newPassword: NEW_PASSWORD, + }, + }); + } + + let app: TestApp | undefined; + beforeEach(async () => { + app = await buildTestApp(); + }); + + it("re-authenticates with the old password, changes it, then establishes a real session with the new password", async () => { + mockUpstream(); + const response = await requestChange(); + + expect(response.statusCode).toBe(200); + expect(response.json()).toEqual({ + user: { email: "user@example.com" }, + csrfToken: expect.any(String), + }); + const setCookieNames = response.cookies.map((c) => c.name); + expect(setCookieNames).toContain("bff_sid"); + expect(setCookieNames).toContain("bff_csrf"); + }); + + it("verifies the precondition via /auth/email/login's own 403, not a persisted flag — so a password-age or default-password block (never persisted) is honored too", async () => { + // The precondition leg's default body (set up above) carries no + // password_change_required field whatsoever — only its 403 status + // matters. If the route still keyed off a persisted flag, this would 403 + // with password_change_not_required instead of succeeding. + mockUpstream(); + const response = await requestChange(); + + expect(response.statusCode).toBe(200); + }); + + it("checks the precondition (old password, /auth/email/login) before ever minting a bypass token", async () => { + const calls = mockUpstream(); + await requestChange(); + + const urls = calls.map((c) => c.url); + expect(urls[0]).toBe(`${config.contextforgeUrl}/auth/email/login`); + expect(urls).toContain(`${config.contextforgeUrl}/auth/login`); + expect(urls).toContain(`${config.contextforgeUrl}/auth/email/change-password`); + // The follow-up real login also hits /auth/email/login — appears twice. + expect(urls.filter((u) => u === `${config.contextforgeUrl}/auth/email/login`)).toHaveLength(2); + }); + + it("sends client-IP audit headers on the change-password call, same as every other upstream call in this flow", async () => { + const calls = mockUpstream(); + await requestChange(); + + const changeCall = calls.find( + (c) => c.url === `${config.contextforgeUrl}/auth/email/change-password`, + ); + expect(changeCall?.headers["x-forwarded-for"]).toBeTruthy(); + expect(changeCall?.headers["x-real-ip"]).toBeTruthy(); + }); + + it("revokes the bypass token upstream after a successful change", async () => { + const calls = mockUpstream(); + await requestChange(); + + const revokeCall = calls.find((c) => c.url === `${config.contextforgeUrl}/auth/logout`); + expect(revokeCall?.authorization).toBe("Bearer bypass-jwt"); + }); + + it("never leaks either upstream token (bypass or real) to the browser", async () => { + mockUpstream(); + const response = await requestChange(); + + const raw = JSON.stringify(response.json()); + expect(raw).not.toContain("bypass-jwt"); + expect(raw).not.toContain("real-jwt"); + }); + + it("passes through the precondition failure status when the old password is wrong, without minting a bypass token", async () => { + const calls = mockUpstream({ + precondition: { ok: false, status: 401, body: { detail: "Invalid email or password" } }, + }); + const response = await requestChange(); + + expect(response.statusCode).toBe(401); + expect(response.json().error).toBe("change_password_failed"); + expect(response.cookies.map((c) => c.name)).not.toContain("bff_sid"); + expect(calls.map((c) => c.url)).toEqual([`${config.contextforgeUrl}/auth/email/login`]); + }); + + it("rejects (and revokes the unused token) when the account does not actually require a password change", async () => { + const calls = mockUpstream({ + precondition: { + ok: true, + status: 200, + body: { access_token: "unused-jwt", user: { email: "user@example.com" } }, // pragma: allowlist secret + }, + }); + const response = await requestChange(); + + expect(response.statusCode).toBe(403); + expect(response.json()).toEqual({ error: "password_change_not_required" }); + expect(response.cookies.map((c) => c.name)).not.toContain("bff_sid"); + // No bypass token was ever minted — only the precondition call happened, + // plus revoking the token that call itself returned. + expect(calls.map((c) => c.url)).toEqual([ + `${config.contextforgeUrl}/auth/email/login`, + `${config.contextforgeUrl}/auth/logout`, + ]); + expect(calls[1]?.authorization).toBe("Bearer unused-jwt"); + }); + + it("passes through the change-password failure status when the new password is rejected, and still revokes the bypass token", async () => { + const calls = mockUpstream({ + changePassword: { + ok: false, + status: 400, + body: { detail: "Password must not be a commonly used password" }, + }, + }); + const response = await requestChange(); + + expect(response.statusCode).toBe(400); + expect(response.json().error).toBe("change_password_failed"); + expect(response.cookies.map((c) => c.name)).not.toContain("bff_sid"); + const revokeCall = calls.find((c) => c.url === `${config.contextforgeUrl}/auth/logout`); + expect(revokeCall?.authorization).toBe("Bearer bypass-jwt"); + }); + + it("reports login_after_change_failed (not change_password_failed) when the password changed but the follow-up login fails", async () => { + const calls = mockUpstream({ + realLogin: { ok: false, status: 401, body: { detail: "unexpected" } }, + }); + const response = await requestChange(); + + expect(response.statusCode).toBe(502); + expect(response.json()).toEqual({ error: "login_after_change_failed" }); + expect(response.cookies.map((c) => c.name)).not.toContain("bff_sid"); + // The password change itself did happen before the follow-up login failed. + expect(calls.map((c) => c.url)).toContain( + `${config.contextforgeUrl}/auth/email/change-password`, + ); + }); + + it("reports login_after_change_failed when the password changed but the follow-up login still reports the account as flagged", async () => { + mockUpstream({ + realLogin: { + body: { + access_token: "real-jwt", // pragma: allowlist secret + expires_in: 1200, + user: { email: "user@example.com", password_change_required: true }, + }, + }, + }); + const response = await requestChange(); + + expect(response.statusCode).toBe(502); + expect(response.json()).toEqual({ error: "login_after_change_failed" }); + expect(response.cookies.map((c) => c.name)).not.toContain("bff_sid"); + }); + + it("returns 502 when upstream is unreachable", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => { + throw new Error("upstream unreachable"); + }), + ); + + const response = await requestChange(); + expect(response.statusCode).toBe(502); + }); + + it("rejects a request missing fields before calling upstream", async () => { + const fetchSpy = vi.fn(); + vi.stubGlobal("fetch", fetchSpy); + + const response = await requestChange({ email: "user@example.com", oldPassword: OLD_PASSWORD }); + + expect(response.statusCode).toBe(400); + expect(fetchSpy).not.toHaveBeenCalled(); + }); }); describe("GET /auth/session", () => { diff --git a/server/test/helpers/build-app.ts b/server/test/helpers/build-app.ts index a4218ee..b943568 100644 --- a/server/test/helpers/build-app.ts +++ b/server/test/helpers/build-app.ts @@ -13,6 +13,7 @@ import { type Redis } from "ioredis"; import cookiePlugin from "../../src/plugins/cookie.js"; import csrfPlugin from "../../src/plugins/csrf.js"; import sessionPlugin from "../../src/plugins/session.js"; +import changePasswordRequiredRoute from "../../src/routes/auth/change-password-required.js"; import loginRoute from "../../src/routes/auth/login.js"; import logoutRoute from "../../src/routes/auth/logout.js"; import sessionRoute from "../../src/routes/auth/session.js"; @@ -58,6 +59,7 @@ export async function buildTestApp(opts: { withProxy?: boolean } = {}): Promise< await fastify.register(loginRoute); await fastify.register(logoutRoute); await fastify.register(sessionRoute); + await fastify.register(changePasswordRequiredRoute); if (opts.withProxy) { await fastify.register(catchAllProxyRoute); diff --git a/src/App.tsx b/src/App.tsx index ec28ebd..47a621a 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -7,6 +7,7 @@ import { Login } from "./pages/Login"; import { ForgotPassword } from "./pages/ForgotPassword"; import { ResetPassword } from "./pages/ResetPassword"; import { ChangePassword } from "./pages/ChangePassword"; +import { PasswordChangeRequired } from "./pages/PasswordChangeRequired"; import { Dashboard } from "./pages/Dashboard"; import { Gateways } from "./pages/Gateways"; import { CreateServer } from "./pages/CreateServer"; @@ -46,6 +47,7 @@ function PublicRoutes() { + ); } diff --git a/src/api/changePasswordRequiredErrors.test.ts b/src/api/changePasswordRequiredErrors.test.ts new file mode 100644 index 0000000..c4a4fc7 --- /dev/null +++ b/src/api/changePasswordRequiredErrors.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; +import { ApiError } from "./client"; +import { classifyChangePasswordRequiredError } from "./changePasswordRequiredErrors"; + +describe("classifyChangePasswordRequiredError", () => { + it.each([ + [401, "invalidOldPassword"], + [403, "invalidOldPassword"], + [500, "failed"], + ])("maps HTTP %s to %s", (status, kind) => { + expect( + classifyChangePasswordRequiredError(new ApiError(status, null, `HTTP ${status}`)), + ).toEqual({ + kind, + }); + }); + + it("preserves 400 detail as a policy-violation message", () => { + const detail = "Password must not be a commonly used password"; + expect(classifyChangePasswordRequiredError(new ApiError(400, { detail }, "HTTP 400"))).toEqual({ + kind: "policyViolation", + message: detail, + }); + }); + + it("preserves 422 detail as a policy-violation message", () => { + const detail = "Password too short"; + expect(classifyChangePasswordRequiredError(new ApiError(422, { detail }, "HTTP 422"))).toEqual({ + kind: "policyViolation", + message: detail, + }); + }); + + it("unwraps the double-JSON-encoded detail the BFF route actually forwards (raw upstream response text)", () => { + // server/src/routes/auth/change-password-required.ts sends + // { error: "change_password_failed", detail: await changeResponse.text() } — + // `detail` here is itself the raw JSON text of upstream's error body. + const body = { + error: "change_password_failed", + detail: JSON.stringify({ + detail: "Password must be at least 22 characters long (privileged account)", + }), + }; + expect(classifyChangePasswordRequiredError(new ApiError(400, body, "HTTP 400"))).toEqual({ + kind: "policyViolation", + message: "Password must be at least 22 characters long (privileged account)", + }); + }); + + it("handles a 400 without detail", () => { + expect(classifyChangePasswordRequiredError(new ApiError(400, null, "HTTP 400"))).toEqual({ + kind: "policyViolation", + message: null, + }); + }); + + it("maps a login_after_change_failed body to changedButLoginFailed, ahead of status-based checks", () => { + const body = { error: "login_after_change_failed" }; + expect(classifyChangePasswordRequiredError(new ApiError(502, body, "HTTP 502"))).toEqual({ + kind: "changedButLoginFailed", + }); + }); + + it("maps a password_change_not_required body to notRequired, not invalidOldPassword, despite the shared 403 status", () => { + const body = { error: "password_change_not_required" }; + expect(classifyChangePasswordRequiredError(new ApiError(403, body, "HTTP 403"))).toEqual({ + kind: "notRequired", + }); + }); + + it("falls back to invalidOldPassword for a 403 without the password_change_not_required body", () => { + expect(classifyChangePasswordRequiredError(new ApiError(403, null, "HTTP 403"))).toEqual({ + kind: "invalidOldPassword", + }); + }); + + it("falls back to failed for a non-ApiError", () => { + expect(classifyChangePasswordRequiredError(new Error("network"))).toEqual({ kind: "failed" }); + }); +}); diff --git a/src/api/changePasswordRequiredErrors.ts b/src/api/changePasswordRequiredErrors.ts new file mode 100644 index 0000000..cffa4de --- /dev/null +++ b/src/api/changePasswordRequiredErrors.ts @@ -0,0 +1,43 @@ +import { ApiError } from "./client"; +import { extractUpstreamApiErrorDetail } from "@/utils/errors"; + +export type ChangePasswordRequiredError = + | { kind: "invalidOldPassword" } + // Correct old password, but the account doesn't currently need a change + // (stale link, flag cleared elsewhere, ...). Distinct from + // invalidOldPassword: credentials were fine, so the forgot-password + // fallback would be the wrong next step to offer. + | { kind: "notRequired" } + | { kind: "policyViolation"; message: string | null } + // Password WAS changed successfully — the BFF just couldn't log back in + // and establish a session right after. Not a validation failure. + | { kind: "changedButLoginFailed" } + | { kind: "failed" }; + +/** + * Classify POST /auth/change-password-required failures. The BFF route + * (server/src/routes/auth/change-password-required.ts) forwards whichever + * upstream call failed first: a 401/403 means the re-authentication step + * (old password) was rejected; `{ error: "password_change_not_required" }` + * (also 403) means credentials were fine but no change is needed; a 400/422 + * means the new password itself was rejected (policy); + * `{ error: "login_after_change_failed" }` means the change succeeded but + * the follow-up login/session-establishment didn't. Scoped to this endpoint + * only — do not merge with classifyLoginError or classifyPasswordResetError, + * which assign 403 different meanings for their own endpoints. + */ +export function classifyChangePasswordRequiredError(error: unknown): ChangePasswordRequiredError { + if (!(error instanceof ApiError)) return { kind: "failed" }; + + const body = error.body as { error?: string } | null; + if (body?.error === "login_after_change_failed") return { kind: "changedButLoginFailed" }; + if (body?.error === "password_change_not_required") return { kind: "notRequired" }; + + if (error.status === 401 || error.status === 403) return { kind: "invalidOldPassword" }; + + if (error.status === 400 || error.status === 422) { + return { kind: "policyViolation", message: extractUpstreamApiErrorDetail(error.body) }; + } + + return { kind: "failed" }; +} diff --git a/src/api/client.ts b/src/api/client.ts index b469e2e..7e845b4 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -23,7 +23,12 @@ const LOGIN_PATH = "/app/login"; const API_PREFIX = "/api"; const SESSION_CHECK_PATH = "/auth/session"; -const BFF_OWNED_AUTH_PATHS = new Set(["/auth/login", "/auth/logout", SESSION_CHECK_PATH]); +const BFF_OWNED_AUTH_PATHS = new Set([ + "/auth/login", + "/auth/logout", + "/auth/change-password-required", + SESSION_CHECK_PATH, +]); export class ApiError extends Error { constructor( diff --git a/src/api/loginErrors.test.ts b/src/api/loginErrors.test.ts new file mode 100644 index 0000000..03d3e61 --- /dev/null +++ b/src/api/loginErrors.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from "vitest"; +import { ApiError } from "./client"; +import { classifyLoginError } from "./loginErrors"; + +describe("classifyLoginError", () => { + it("maps 401 to invalidCredentials regardless of body", () => { + expect(classifyLoginError(new ApiError(401, null, "HTTP 401"))).toEqual({ + kind: "invalidCredentials", + }); + }); + + it("detects password-change-required from a double-JSON-encoded 403 detail", () => { + const body = { + error: "login_failed", + detail: JSON.stringify({ + detail: "Password change required. Please change your password before continuing.", + }), + }; + expect(classifyLoginError(new ApiError(403, body, "HTTP 403"))).toEqual({ + kind: "passwordChangeRequired", + }); + }); + + it("falls back to failed for an unrelated 403", () => { + const body = { error: "login_failed", detail: JSON.stringify({ detail: "Forbidden" }) }; + expect(classifyLoginError(new ApiError(403, body, "HTTP 403"))).toEqual({ + kind: "failed", + status: 403, + }); + }); + + it("does not throw on non-JSON detail (e.g. a plain-text 429 body)", () => { + const body = { error: "login_failed", detail: "Too many requests" }; + expect(classifyLoginError(new ApiError(429, body, "HTTP 429"))).toEqual({ + kind: "failed", + status: 429, + }); + }); + + it("falls back to failed for a malformed/missing body or non-ApiError", () => { + expect(classifyLoginError(new ApiError(403, null, "HTTP 403"))).toEqual({ + kind: "failed", + status: 403, + }); + expect(classifyLoginError(new Error("network"))).toEqual({ kind: "failed", status: 0 }); + }); +}); diff --git a/src/api/loginErrors.ts b/src/api/loginErrors.ts new file mode 100644 index 0000000..34bc6b3 --- /dev/null +++ b/src/api/loginErrors.ts @@ -0,0 +1,28 @@ +import { ApiError } from "./client"; +import { extractUpstreamApiErrorDetail } from "@/utils/errors"; + +export type LoginError = + | { kind: "invalidCredentials" } + | { kind: "passwordChangeRequired" } + | { kind: "failed"; status: number }; + +const PASSWORD_CHANGE_REQUIRED_PATTERN = /password change required/i; + +/** + * Classify POST /auth/login failures. Scoped to the login endpoint only — + * do not merge with classifyPasswordResetError (./passwordResetErrors), which + * assigns a different meaning to 403 for the password-reset-request flow. + */ +export function classifyLoginError(error: unknown): LoginError { + if (!(error instanceof ApiError)) return { kind: "failed", status: 0 }; + if (error.status === 401) return { kind: "invalidCredentials" }; + + if (error.status === 403) { + const detail = extractUpstreamApiErrorDetail(error.body); + if (detail && PASSWORD_CHANGE_REQUIRED_PATTERN.test(detail)) { + return { kind: "passwordChangeRequired" }; + } + } + + return { kind: "failed", status: error.status }; +} diff --git a/src/auth/AuthContext.test.tsx b/src/auth/AuthContext.test.tsx index 7c5a78c..0abf15a 100644 --- a/src/auth/AuthContext.test.tsx +++ b/src/auth/AuthContext.test.tsx @@ -50,6 +50,15 @@ function TestComponent() { > Login + ); @@ -341,6 +350,91 @@ describe("AuthContext", () => { expect(screen.queryByTestId("user-email")).not.toBeInTheDocument(); }); + it("handles a successful completePasswordChangeRequired identically to login", async () => { + vi.mocked(api.get).mockRejectedValueOnce(new ApiError(401, "Unauthorized", "")); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.queryByTestId("loading")).not.toBeInTheDocument(); + }); + + const mockUser = { + email: "test@example.com", + full_name: "Test User", + is_admin: false, + is_active: true, + auth_provider: "local", + email_verified: true, + password_change_required: false, + }; + + vi.mocked(api.post).mockResolvedValueOnce({ + user: mockUser, + csrfToken: "test-csrf-token", + }); + + screen.getByText("CompletePasswordChangeRequired").click(); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("authenticated"); + expect(screen.getByTestId("user-email")).toHaveTextContent("test@example.com"); + }); + + expect(api.post).toHaveBeenCalledWith( + "/auth/change-password-required", + { email: "test@example.com", oldPassword: "old-pass", newPassword: "new-pass" }, + { authenticated: false }, + ); + expect(setCsrfToken).toHaveBeenCalledWith("test-csrf-token"); + }); + + it("clears stale auth state and CSRF token when completePasswordChangeRequired fails", async () => { + // Same scenario as login()'s equivalent test: a user reaches this + // pre-auth page (bookmark, still-open tab) while still holding a + // previously-authenticated state. + const mockUser = { + email: "user@example.com", + full_name: "Test User", + is_admin: false, + is_active: true, + auth_provider: "local", + email_verified: true, + password_change_required: false, + }; + + vi.mocked(api.get).mockResolvedValueOnce({ + authenticated: true, + user: mockUser, + csrfToken: "stale-csrf-token", + }); + + render( + + + , + ); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("authenticated"); + }); + + vi.mocked(api.post).mockRejectedValueOnce(new ApiError(401, "Unauthorized", "")); + + screen.getByText("CompletePasswordChangeRequired").click(); + + await waitFor(() => { + expect(screen.getByTestId("auth-status")).toHaveTextContent("guest"); + }); + + expect(screen.queryByTestId("user-email")).not.toBeInTheDocument(); + expect(setCsrfToken).toHaveBeenLastCalledWith(null); + }); + it("handles successful logout", async () => { const mockUser = { email: "user@example.com", diff --git a/src/auth/AuthContext.tsx b/src/auth/AuthContext.tsx index 35b9479..75a8667 100644 --- a/src/auth/AuthContext.tsx +++ b/src/auth/AuthContext.tsx @@ -1,4 +1,12 @@ -import { createContext, useCallback, useContext, useState, useEffect, useRef } from "react"; +import { + createContext, + useCallback, + useContext, + useState, + useEffect, + useMemo, + useRef, +} from "react"; import type { ReactNode } from "react"; import { api, ApiError, setCsrfToken } from "../api/client"; import { permissionsApi } from "../api/permissions"; @@ -38,6 +46,17 @@ interface AuthState { interface AuthContextValue extends AuthState { login: (email: string, password: string) => Promise; // pragma: allowlist secret + /** + * Completes the "password change required" flow: BFF re-authenticates with + * the old password, changes it, then logs in again with the new one and + * establishes a real session — same response shape as login(), so this + * updates auth state identically. See PasswordChangeRequired.tsx. + */ + completePasswordChangeRequired: ( + email: string, + oldPassword: string, // pragma: allowlist secret + newPassword: string, // pragma: allowlist secret + ) => Promise; logout: () => Promise; setSelectedTeamId: (teamId: string | null) => void; /** Caller's effective permissions from GET /rbac/my/permissions (in-memory only). */ @@ -153,6 +172,41 @@ export function AuthProvider({ children }: { children: ReactNode }) { [], ); + const completePasswordChangeRequired = useCallback( + async ( + email: string, + oldPassword: string, // pragma: allowlist secret + newPassword: string, // pragma: allowlist secret + ): Promise => { + try { + const data = await api.post( + "/auth/change-password-required", + { email, oldPassword, newPassword }, + { authenticated: false }, + ); + + setCsrfToken(data.csrfToken); + authVersion.current += 1; + setState({ + user: data.user, + isAuthenticated: true, + isLoading: false, + selectedTeamId: null, + }); + } catch (err) { + // Same reasoning as login()'s catch: don't leave a stale CSRF token + // or a previously "authenticated" state around on failure — a user + // who lands on this pre-auth page with an existing session (bookmark, + // still-open tab) must not keep that state if the BFF call fails. + setCsrfToken(null); + authVersion.current += 1; + setState({ user: null, isAuthenticated: false, isLoading: false, selectedTeamId: null }); + throw err; + } + }, + [], + ); + const logout = useCallback(async (): Promise => { try { // Empty body would still send Content-Type: application/json, which Fastify's @@ -205,22 +259,22 @@ export function AuthProvider({ children }: { children: ReactNode }) { [perms], ); - return ( - - {children} - + const value = useMemo( + () => ({ + ...state, + login, + completePasswordChangeRequired, + logout, + setSelectedTeamId, + permissions: perms.permissions, + permissionsLoading: perms.loading, + permissionsError: perms.error, + hasPermission, + }), + [state, login, completePasswordChangeRequired, logout, setSelectedTeamId, perms, hasPermission], ); + + return {children}; } // --------------------------------------------------------------------------- diff --git a/src/components/auth/AuthCard.tsx b/src/components/auth/AuthCard.tsx new file mode 100644 index 0000000..4414911 --- /dev/null +++ b/src/components/auth/AuthCard.tsx @@ -0,0 +1,30 @@ +import type { ReactNode } from "react"; + +/** + * Shared
/
chrome for the auth pages (ForgotPassword, + * ResetPassword, PasswordChangeRequired). `compact` switches to the + * narrower/rounder success-panel sizing used once a page's success state is + * entered. + */ +export function AuthCard({ + titleId, + compact = false, + children, +}: { + titleId: string; + compact?: boolean; + children: ReactNode; +}) { + return ( +
+
+ {children} +
+
+ ); +} diff --git a/src/components/auth/AuthSuccessPanel.tsx b/src/components/auth/AuthSuccessPanel.tsx new file mode 100644 index 0000000..e0b974a --- /dev/null +++ b/src/components/auth/AuthSuccessPanel.tsx @@ -0,0 +1,53 @@ +import { useEffect, useRef } from "react"; +import { CircleCheck } from "lucide-react"; +import { Button } from "@/components/ui/button"; + +/** + * Shared success state for ResetPassword / PasswordChangeRequired: a + * CircleCheck heading (focus-managed for screen-reader/keyboard users), + * body copy, and a single CTA. Not used by ForgotPassword, which shows its + * success state as an InlineNotification instead of this pattern. + */ +export function AuthSuccessPanel({ + titleId, + title, + body, + ctaLabel, + onCta, +}: { + titleId: string; + title: string; + body: string; + ctaLabel: string; + onCta: () => void; +}) { + const headingRef = useRef(null); + useEffect(() => { + // This component only mounts once the success state is entered, so + // focusing on mount is equivalent to the old useEffect([succeeded]). + headingRef.current?.focus(); + }, []); + + return ( +
+
+

+

+

{body}

+
+ +
+ ); +} diff --git a/src/components/layout/HeaderQuickNav.test.tsx b/src/components/layout/HeaderQuickNav.test.tsx index 2a9ef75..6798a1a 100644 --- a/src/components/layout/HeaderQuickNav.test.tsx +++ b/src/components/layout/HeaderQuickNav.test.tsx @@ -49,6 +49,7 @@ const defaultAuthContext = { isLoading: false, selectedTeamId: null, login: vi.fn(), + completePasswordChangeRequired: vi.fn(), logout: vi.fn(), setSelectedTeamId: vi.fn(), permissions: [], diff --git a/src/components/mcp-servers/AdvancedSettings.test.tsx b/src/components/mcp-servers/AdvancedSettings.test.tsx index 6cdec5b..bfaddee 100644 --- a/src/components/mcp-servers/AdvancedSettings.test.tsx +++ b/src/components/mcp-servers/AdvancedSettings.test.tsx @@ -19,6 +19,7 @@ const makeAuthContext = (selectedTeamId: string | null = null) => isAuthenticated: false, isLoading: false, login: vi.fn(), + completePasswordChangeRequired: vi.fn(), logout: vi.fn(), setSelectedTeamId: vi.fn(), permissions: [], diff --git a/src/components/prompts/PromptForm.test.tsx b/src/components/prompts/PromptForm.test.tsx index 5a59abc..447dc20 100644 --- a/src/components/prompts/PromptForm.test.tsx +++ b/src/components/prompts/PromptForm.test.tsx @@ -75,6 +75,7 @@ describe("PromptForm", () => { isAuthenticated: true, isLoading: false, login: vi.fn(), + completePasswordChangeRequired: vi.fn(), logout: vi.fn(), setSelectedTeamId: vi.fn(), permissions: [], @@ -117,6 +118,7 @@ describe("PromptForm", () => { isAuthenticated: true, isLoading: false, login: vi.fn(), + completePasswordChangeRequired: vi.fn(), logout: vi.fn(), setSelectedTeamId: vi.fn(), permissions: [], @@ -200,6 +202,7 @@ describe("PromptForm", () => { isAuthenticated: true, isLoading: false, login: vi.fn(), + completePasswordChangeRequired: vi.fn(), logout: vi.fn(), setSelectedTeamId: vi.fn(), permissions: [], diff --git a/src/components/users/PasswordInput.tsx b/src/components/users/PasswordInput.tsx index f9091ae..100d631 100644 --- a/src/components/users/PasswordInput.tsx +++ b/src/components/users/PasswordInput.tsx @@ -17,77 +17,83 @@ interface PasswordInputProps { hint?: string; } -export function PasswordInput({ - id, - value, - onChange, - placeholder, - label, - required = false, - autoComplete = "new-password", - error, - hint, -}: PasswordInputProps) { - const intl = useIntl(); - const [showPassword, setShowPassword] = React.useState(false); - const errorId = `${id}-error`; - const hintId = `${id}-hint`; +export const PasswordInput = React.forwardRef( + function PasswordInput( + { + id, + value, + onChange, + placeholder, + label, + required = false, + autoComplete = "new-password", + error, + hint, + }, + ref, + ) { + const intl = useIntl(); + const [showPassword, setShowPassword] = React.useState(false); + const errorId = `${id}-error`; + const hintId = `${id}-hint`; - return ( -
- -
- onChange(event.target.value)} - placeholder={placeholder} - className="h-10 border-neutral-300 pr-10 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 dark:border-neutral-700" - aria-invalid={!!error} - aria-describedby={error ? errorId : hint ? hintId : undefined} - /> - + +
+ onChange(event.target.value)} + placeholder={placeholder} + className="h-10 border-neutral-300 pr-10 shadow-none focus-visible:ring-1 focus-visible:ring-ring focus-visible:ring-offset-0 dark:border-neutral-700" + aria-invalid={!!error} + aria-describedby={error ? errorId : hint ? hintId : undefined} + /> + +
+ {error ? ( + + ) : hint ? ( +

+ {hint} +

+ ) : null}
- {error ? ( - - ) : hint ? ( -

- {hint} -

- ) : null} -
- ); -} + ); + }, +); diff --git a/src/hooks/usePromptForm.test.ts b/src/hooks/usePromptForm.test.ts index da30dcc..e1b729c 100644 --- a/src/hooks/usePromptForm.test.ts +++ b/src/hooks/usePromptForm.test.ts @@ -42,6 +42,7 @@ function mockAuth(selectedTeamId: string | null = null) { isAuthenticated: true, isLoading: false, login: vi.fn(), + completePasswordChangeRequired: vi.fn(), logout: vi.fn(), setSelectedTeamId: vi.fn(), permissions: [], diff --git a/src/i18n/locales/en-US/auth.json b/src/i18n/locales/en-US/auth.json index df243a1..fa2bc54 100644 --- a/src/i18n/locales/en-US/auth.json +++ b/src/i18n/locales/en-US/auth.json @@ -40,6 +40,27 @@ "auth.changePassword.newPassword": "New Password", "auth.changePassword.confirmPassword": "Confirm New Password", "auth.changePassword.submit": "Change Password", + "auth.passwordChangeRequired.title": "Password Change Required", + "auth.passwordChangeRequired.description": "Your password needs to be changed before you can sign in. Enter your current password and choose a new one.", + "auth.passwordChangeRequired.email": "Email Address", + "auth.passwordChangeRequired.oldPassword": "Current Password", + "auth.passwordChangeRequired.newPassword": "New Password", + "auth.passwordChangeRequired.confirmPassword": "Confirm New Password", + "auth.passwordChangeRequired.passwordHint": "Use at least 12 characters and 3 of: uppercase, lowercase, number, or special character. Privileged accounts may require more.", + "auth.passwordChangeRequired.submit": "Change Password", + "auth.passwordChangeRequired.submitting": "Changing password…", + "auth.passwordChangeRequired.successTitle": "Password changed", + "auth.passwordChangeRequired.success": "Your password was changed for ContextForge, but we couldn't sign you in automatically. Please log in with your new password.", + "auth.passwordChangeRequired.returnToLogin": "Return to login", + "auth.passwordChangeRequired.forgotPasswordFallback": "Forgot your password instead?", + "auth.passwordChangeRequired.error.oldPasswordRequired": "Enter your current password.", + "auth.passwordChangeRequired.error.tooShort": "Password must be at least 12 characters.", + "auth.passwordChangeRequired.error.complexity": "Password must contain at least 3 of: uppercase, lowercase, number, or special character.", + "auth.passwordChangeRequired.error.mismatch": "Passwords do not match.", + "auth.passwordChangeRequired.error.invalidOldPassword": "Your current password is incorrect.", + "auth.passwordChangeRequired.error.notRequired": "Your password doesn't need to be changed. You can sign in normally.", + "auth.passwordChangeRequired.error.missingEmail": "We couldn't tell which account this is for. Please sign in again to restart this process.", + "auth.passwordChangeRequired.error.failed": "We could not change your password. Please try again.", "auth.logout": "Sign Out", "auth.login.error.invalidCredentials": "Invalid credentials.", "auth.login.error.failed": "Login failed ({status}).", diff --git a/src/i18n/locales/es-ES/auth.json b/src/i18n/locales/es-ES/auth.json index b63afe5..94ee65d 100644 --- a/src/i18n/locales/es-ES/auth.json +++ b/src/i18n/locales/es-ES/auth.json @@ -40,6 +40,27 @@ "auth.changePassword.newPassword": "Nueva Contraseña", "auth.changePassword.confirmPassword": "Confirmar Nueva Contraseña", "auth.changePassword.submit": "Cambiar Contraseña", + "auth.passwordChangeRequired.title": "Cambio de Contraseña Requerido", + "auth.passwordChangeRequired.description": "Debes cambiar tu contraseña antes de poder iniciar sesión. Ingresa tu contraseña actual y elige una nueva.", + "auth.passwordChangeRequired.email": "Dirección de Correo Electrónico", + "auth.passwordChangeRequired.oldPassword": "Contraseña Actual", + "auth.passwordChangeRequired.newPassword": "Nueva Contraseña", + "auth.passwordChangeRequired.confirmPassword": "Confirmar Nueva Contraseña", + "auth.passwordChangeRequired.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.passwordChangeRequired.submit": "Cambiar Contraseña", + "auth.passwordChangeRequired.submitting": "Cambiando contraseña…", + "auth.passwordChangeRequired.successTitle": "Contraseña cambiada", + "auth.passwordChangeRequired.success": "Tu contraseña de ContextForge fue cambiada, pero no pudimos iniciar sesión automáticamente. Inicia sesión con tu nueva contraseña.", + "auth.passwordChangeRequired.returnToLogin": "Volver al inicio de sesión", + "auth.passwordChangeRequired.forgotPasswordFallback": "¿Olvidaste tu contraseña?", + "auth.passwordChangeRequired.error.oldPasswordRequired": "Ingresa tu contraseña actual.", + "auth.passwordChangeRequired.error.tooShort": "La contraseña debe tener al menos 12 caracteres.", + "auth.passwordChangeRequired.error.complexity": "La contraseña debe contener al menos 3 de estos tipos: mayúscula, minúscula, número o carácter especial.", + "auth.passwordChangeRequired.error.mismatch": "Las contraseñas no coinciden.", + "auth.passwordChangeRequired.error.invalidOldPassword": "Tu contraseña actual es incorrecta.", + "auth.passwordChangeRequired.error.notRequired": "Tu contraseña no necesita ser cambiada. Puedes iniciar sesión normalmente.", + "auth.passwordChangeRequired.error.missingEmail": "No pudimos identificar la cuenta. Inicia sesión de nuevo para reiniciar este proceso.", + "auth.passwordChangeRequired.error.failed": "No pudimos cambiar tu contraseña. Inténtalo de nuevo.", "auth.logout": "Cerrar Sesión", "auth.login.error.invalidCredentials": "Credenciales inválidas.", "auth.login.error.failed": "Error al iniciar sesión ({status}).", diff --git a/src/i18n/locales/pt-BR/auth.json b/src/i18n/locales/pt-BR/auth.json index 55dd4a8..4a9c280 100644 --- a/src/i18n/locales/pt-BR/auth.json +++ b/src/i18n/locales/pt-BR/auth.json @@ -40,6 +40,27 @@ "auth.changePassword.newPassword": "Nova Senha", "auth.changePassword.confirmPassword": "Confirmar Nova Senha", "auth.changePassword.submit": "Alterar Senha", + "auth.passwordChangeRequired.title": "Alteração de Senha Necessária", + "auth.passwordChangeRequired.description": "Sua senha precisa ser alterada antes que você possa entrar. Digite sua senha atual e escolha uma nova.", + "auth.passwordChangeRequired.email": "Endereço de E-mail", + "auth.passwordChangeRequired.oldPassword": "Senha Atual", + "auth.passwordChangeRequired.newPassword": "Nova Senha", + "auth.passwordChangeRequired.confirmPassword": "Confirmar Nova Senha", + "auth.passwordChangeRequired.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.passwordChangeRequired.submit": "Alterar Senha", + "auth.passwordChangeRequired.submitting": "Alterando senha…", + "auth.passwordChangeRequired.successTitle": "Senha alterada", + "auth.passwordChangeRequired.success": "Sua senha do ContextForge foi alterada, mas não conseguimos fazer login automaticamente. Faça login com sua nova senha.", + "auth.passwordChangeRequired.returnToLogin": "Voltar ao login", + "auth.passwordChangeRequired.forgotPasswordFallback": "Esqueceu a senha?", + "auth.passwordChangeRequired.error.oldPasswordRequired": "Digite sua senha atual.", + "auth.passwordChangeRequired.error.tooShort": "A senha deve ter pelo menos 12 caracteres.", + "auth.passwordChangeRequired.error.complexity": "A senha deve conter pelo menos 3 destes tipos: maiúscula, minúscula, número ou caractere especial.", + "auth.passwordChangeRequired.error.mismatch": "As senhas não coincidem.", + "auth.passwordChangeRequired.error.invalidOldPassword": "Sua senha atual está incorreta.", + "auth.passwordChangeRequired.error.notRequired": "Sua senha não precisa ser alterada. Você pode entrar normalmente.", + "auth.passwordChangeRequired.error.missingEmail": "Não conseguimos identificar a conta. Entre novamente para reiniciar este processo.", + "auth.passwordChangeRequired.error.failed": "Não foi possível alterar sua senha. Tente novamente.", "auth.logout": "Sair", "auth.login.error.invalidCredentials": "Credenciais inválidas.", "auth.login.error.failed": "Falha ao entrar ({status}).", diff --git a/src/lib/passwordPolicy.test.ts b/src/lib/passwordPolicy.test.ts new file mode 100644 index 0000000..a1c6b80 --- /dev/null +++ b/src/lib/passwordPolicy.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from "vitest"; +import { countPasswordCharacterClasses, MIN_PASSWORD_CHARACTER_CLASSES } from "./passwordPolicy"; + +describe("countPasswordCharacterClasses", () => { + it("counts all four classes for a password that genuinely has them", () => { + expect(countPasswordCharacterClasses("Password123!")).toBe(4); + }); + + it("does not count a trailing space as a special character", () => { + expect(countPasswordCharacterClasses("Password123 ")).toBe( + MIN_PASSWORD_CHARACTER_CLASSES, // upper, lower, digit — no genuine special char + ); + }); + + it("does not count accented/non-ASCII letters as a special character", () => { + // "á" is neither [a-z] nor [A-Z] nor \d nor ASCII punctuation — the old + // `/[^A-Za-z0-9]/` pattern over-counted it as "special". + expect(countPasswordCharacterClasses("Passworda1")).toBe(3); + expect(countPasswordCharacterClasses("Passwordá1")).toBe(3); + }); + + it("counts common ASCII punctuation as the special-character class", () => { + for (const char of ["!", "@", "#", "$", "%", "-", "_", "."]) { + expect(countPasswordCharacterClasses(`Password1${char}`)).toBe(4); + } + }); +}); diff --git a/src/lib/passwordPolicy.ts b/src/lib/passwordPolicy.ts new file mode 100644 index 0000000..9b5f12c --- /dev/null +++ b/src/lib/passwordPolicy.ts @@ -0,0 +1,15 @@ +// Shared 3-of-4-character-class password complexity check, used by +// ResetPassword.tsx and PasswordChangeRequired.tsx so the policy can't drift +// between the two forms. +// +// The "special character" class is an explicit ASCII punctuation set, not +// `/[^A-Za-z0-9]/` — that broader pattern also matches whitespace/control +// characters (so a trailing space would count) and non-ASCII letters (á, ñ, +// ç — which the other three classes already treat as neither upper- nor +// lowercase, so counting them here just double-credits the same character). +const CHARACTER_CLASS_PATTERNS = [/[a-z]/, /[A-Z]/, /\d/, /[!"#$%&'()*+,\-./:;<=>?@[\]^_`{|}~]/]; +export const MIN_PASSWORD_CHARACTER_CLASSES = 3; + +export function countPasswordCharacterClasses(password: string): number { + return CHARACTER_CLASS_PATTERNS.filter((pattern) => pattern.test(password)).length; +} diff --git a/src/pages/ForgotPassword.tsx b/src/pages/ForgotPassword.tsx index c7553e3..3e05b45 100644 --- a/src/pages/ForgotPassword.tsx +++ b/src/pages/ForgotPassword.tsx @@ -2,6 +2,7 @@ import { useState } from "react"; import { useIntl } from "react-intl"; import { requestPasswordReset } from "@/api/passwordReset"; import { classifyPasswordResetError } from "@/api/passwordResetErrors"; +import { AuthCard } from "@/components/auth/AuthCard"; import { Button } from "@/components/ui/button"; import { InlineNotification } from "@/components/ui/inline-notification"; import { Input } from "@/components/ui/input"; @@ -47,79 +48,72 @@ export function ForgotPassword() { } return ( -
-
+

-

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

-

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

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

+ {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 && ( + + )}
- ) : ( - -
- - { - setEmail(event.target.value); - setEmailError(null); - }} - aria-invalid={!!emailError} - aria-describedby={emailError ? "forgot-password-email-error" : undefined} - /> - {emailError && ( - - )} -
- {error && } - - - - )} -
-
+ {error && } + + + + )} + ); } diff --git a/src/pages/Login.test.tsx b/src/pages/Login.test.tsx index 5962984..119a553 100644 --- a/src/pages/Login.test.tsx +++ b/src/pages/Login.test.tsx @@ -97,6 +97,56 @@ describe("Login", () => { }); }); + it("redirects to the password-change-required page on a 403 password-change-required response", async () => { + const body = { + error: "login_failed", + detail: JSON.stringify({ + detail: "Password change required. Please change your password before continuing.", + }), + }; + const error = new ApiError(403, body, "HTTP 403"); + + vi.mocked(useAuth).mockReturnValue({ + isAuthenticated: false, + login: mockLogin.mockRejectedValue(error), + } as unknown as ReturnType); + + renderWithI18n(); + + fireEvent.change(screen.getByLabelText(/Email/i), { target: { value: "test@example.com" } }); + fireEvent.change(screen.getByLabelText(/Password/i), { target: { value: "oldpass" } }); + fireEvent.submit(screen.getByRole("button", { name: /Sign in/i })); + + await waitFor(() => { + expect(mockNavigate).toHaveBeenCalledWith( + "/app/change-password-required?email=test%40example.com", + ); + }); + }); + + it("displays the generic failed error for an unrelated 403", async () => { + const body = { error: "login_failed", detail: JSON.stringify({ detail: "Forbidden" }) }; + const error = new ApiError(403, body, "HTTP 403"); + + vi.mocked(useAuth).mockReturnValue({ + isAuthenticated: false, + login: mockLogin.mockRejectedValue(error), + } as unknown as ReturnType); + + renderWithI18n(); + + fireEvent.change(screen.getByLabelText(/Email/i), { target: { value: "test@example.com" } }); + fireEvent.change(screen.getByLabelText(/Password/i), { target: { value: "pass" } }); + fireEvent.submit(screen.getByRole("button", { name: /Sign in/i })); + + await waitFor(() => { + expect(screen.getByRole("alert")).toHaveTextContent("Login failed (403)."); + }); + expect(mockNavigate).not.toHaveBeenCalledWith( + expect.stringContaining("change-password-required"), + ); + }); + it("displays generic failed error on non-401 ApiError", async () => { const error = new ApiError(500, "ApiError", "") as ApiError & { status?: number }; error.status = 500; diff --git a/src/pages/Login.tsx b/src/pages/Login.tsx index 70102ef..4a96cdb 100644 --- a/src/pages/Login.tsx +++ b/src/pages/Login.tsx @@ -3,6 +3,7 @@ import { useIntl } from "react-intl"; import { useAuth } from "../auth/useAuth"; import { useRouter, resolveNextParam } from "../router"; import { ApiError } from "../api/client"; +import { classifyLoginError } from "../api/loginErrors"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; @@ -31,12 +32,15 @@ export function Login() { await login(email, password); navigate(returnTo); } catch (err) { - if (err instanceof ApiError) { - setError( - err.status === 401 - ? intl.formatMessage({ id: "auth.login.error.invalidCredentials" }) - : intl.formatMessage({ id: "auth.login.error.failed" }, { status: err.status }), - ); + const loginError = classifyLoginError(err); + if (loginError.kind === "passwordChangeRequired") { + navigate(`/app/change-password-required?email=${encodeURIComponent(email)}`); + return; + } + if (loginError.kind === "invalidCredentials") { + setError(intl.formatMessage({ id: "auth.login.error.invalidCredentials" })); + } else if (err instanceof ApiError) { + setError(intl.formatMessage({ id: "auth.login.error.failed" }, { status: err.status })); } else { setError(intl.formatMessage({ id: "auth.login.error.unexpected" })); } diff --git a/src/pages/PasswordChangeRequired.test.tsx b/src/pages/PasswordChangeRequired.test.tsx new file mode 100644 index 0000000..342f024 --- /dev/null +++ b/src/pages/PasswordChangeRequired.test.tsx @@ -0,0 +1,176 @@ +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { ApiError } from "@/api/client"; +import { I18nProvider } from "@/i18n"; +import { useAuth } from "@/auth/useAuth"; +import { useRouter, resolveNextParam } from "@/router"; +import { PasswordChangeRequired } from "./PasswordChangeRequired"; + +vi.mock("@/auth/useAuth", () => ({ useAuth: vi.fn() })); +vi.mock("@/router", () => ({ useRouter: vi.fn(), resolveNextParam: vi.fn() })); + +describe("PasswordChangeRequired", () => { + const navigate = vi.fn(); + const completePasswordChangeRequired = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(useRouter).mockReturnValue({ navigate } as unknown as ReturnType); + vi.mocked(resolveNextParam).mockReturnValue("/app/"); + vi.mocked(useAuth).mockReturnValue({ + isAuthenticated: false, + completePasswordChangeRequired, + } as unknown as ReturnType); + window.history.pushState({}, "", "/app/change-password-required?email=user%40example.com"); + }); + + function renderPage() { + return render( + + + , + ); + } + + async function fillPasswords( + oldPassword: string, + newPassword: string, + confirmation = newPassword, + ) { + await waitFor(() => expect(document.getElementById("old-password")).toBeInTheDocument()); + fireEvent.change(document.getElementById("old-password")!, { target: { value: oldPassword } }); + fireEvent.change(document.getElementById("new-password")!, { target: { value: newPassword } }); + fireEvent.change(document.getElementById("confirm-password")!, { + target: { value: confirmation }, + }); + } + + it("pre-fills the read-only email from the query param", () => { + renderPage(); + expect(screen.getByLabelText("Email Address")).toHaveValue("user@example.com"); + expect(screen.getByLabelText("Email Address")).toBeDisabled(); + }); + + it("redirects into the app if already authenticated", () => { + vi.mocked(useAuth).mockReturnValue({ + isAuthenticated: true, + completePasswordChangeRequired, + } as unknown as ReturnType); + + renderPage(); + expect(navigate).toHaveBeenCalledWith("/app/"); + }); + + it("rejects mismatched new passwords without an API request", async () => { + renderPage(); + await fillPasswords("old-pass", "New-password1", "New-password2"); + fireEvent.click(screen.getByRole("button", { name: "Change Password" })); + + expect(await screen.findByText("Passwords do not match.")).toBeInTheDocument(); + expect(completePasswordChangeRequired).not.toHaveBeenCalled(); + }); + + it("enforces minimum length and complexity before submission", async () => { + renderPage(); + await fillPasswords("old-pass", "short-A1!"); + fireEvent.click(screen.getByRole("button", { name: "Change Password" })); + expect(await screen.findByText("Password must be at least 12 characters.")).toBeInTheDocument(); + + await fillPasswords("old-pass", "alllowercasepassword"); + fireEvent.click(screen.getByRole("button", { name: "Change Password" })); + expect( + await screen.findByText( + "Password must contain at least 3 of: uppercase, lowercase, number, or special character.", + ), + ).toBeInTheDocument(); + expect(completePasswordChangeRequired).not.toHaveBeenCalled(); + }); + + it("on success, calls AuthContext and navigates straight into the app (no static success screen)", async () => { + completePasswordChangeRequired.mockResolvedValue(undefined); + renderPage(); + await fillPasswords("old-pass", "New-password1"); + fireEvent.click(screen.getByRole("button", { name: "Change Password" })); + + await waitFor(() => { + expect(completePasswordChangeRequired).toHaveBeenCalledWith( + "user@example.com", + "old-pass", + "New-password1", + ); + }); + // No local success screen is rendered — AuthContext's own state flip + // (isAuthenticated: true on next render) is what drives navigation. + expect(screen.queryByRole("status")).not.toBeInTheDocument(); + }); + + it("shows an invalid-old-password message with a forgot-password fallback", async () => { + completePasswordChangeRequired.mockRejectedValue(new ApiError(401, null, "HTTP 401")); + renderPage(); + await fillPasswords("wrong-old-pass", "New-password1"); + fireEvent.click(screen.getByRole("button", { name: "Change Password" })); + + expect(await screen.findByRole("alert")).toHaveTextContent(/current password is incorrect/i); + const fallback = screen.getByRole("button", { name: /forgot your password/i }); + fireEvent.click(fallback); + expect(navigate).toHaveBeenCalledWith("/app/forgot-password"); + }); + + it("surfaces a backend policy-violation message on the new password field", async () => { + completePasswordChangeRequired.mockRejectedValue( + new ApiError(400, { detail: "Password must not be a commonly used password" }, "HTTP 400"), + ); + renderPage(); + await fillPasswords("old-pass", "New-password1"); + fireEvent.click(screen.getByRole("button", { name: "Change Password" })); + + expect( + await screen.findByText("Password must not be a commonly used password"), + ).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: /forgot your password/i })).not.toBeInTheDocument(); + }); + + it("shows a distinct message with a return-to-login fallback when the account doesn't need a change (correct credentials)", async () => { + completePasswordChangeRequired.mockRejectedValue( + new ApiError(403, { error: "password_change_not_required" }, "HTTP 403"), + ); + renderPage(); + await fillPasswords("old-pass", "New-password1"); + fireEvent.click(screen.getByRole("button", { name: "Change Password" })); + + const alert = await screen.findByRole("alert"); + expect(alert).toHaveTextContent(/doesn't need to be changed/i); + // Not the "wrong password" copy, and no forgot-password offer — the + // credentials were correct. + expect(alert).not.toHaveTextContent(/current password is incorrect/i); + expect(screen.queryByRole("button", { name: /forgot your password/i })).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Return to login" })); + expect(navigate).toHaveBeenCalledWith("/app/login"); + }); + + it("shows an error and a return-to-login CTA instead of a submittable form when ?email= is missing", () => { + window.history.pushState({}, "", "/app/change-password-required"); + renderPage(); + + expect(screen.getByRole("alert")).toHaveTextContent(/couldn't tell which account/i); + expect(document.getElementById("old-password")).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Change Password" })).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: "Return to login" })); + expect(navigate).toHaveBeenCalledWith("/app/login"); + }); + + it("shows the fallback success screen when the password changed but auto sign-in failed", async () => { + completePasswordChangeRequired.mockRejectedValue( + new ApiError(502, { error: "login_after_change_failed" }, "HTTP 502"), + ); + renderPage(); + await fillPasswords("old-pass", "New-password1"); + fireEvent.click(screen.getByRole("button", { name: "Change Password" })); + + expect(await screen.findByRole("status")).toHaveTextContent("Password changed"); + fireEvent.click(screen.getByRole("button", { name: "Return to login" })); + expect(navigate).toHaveBeenCalledWith("/app/login"); + }); +}); diff --git a/src/pages/PasswordChangeRequired.tsx b/src/pages/PasswordChangeRequired.tsx new file mode 100644 index 0000000..c1d9582 --- /dev/null +++ b/src/pages/PasswordChangeRequired.tsx @@ -0,0 +1,287 @@ +import { useEffect, useMemo, useRef, useState } from "react"; +import { useIntl } from "react-intl"; +import { useAuth } from "@/auth/useAuth"; +import { classifyChangePasswordRequiredError } from "@/api/changePasswordRequiredErrors"; +import { AuthCard } from "@/components/auth/AuthCard"; +import { AuthSuccessPanel } from "@/components/auth/AuthSuccessPanel"; +import { PasswordInput } from "@/components/users/PasswordInput"; +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 { VALIDATION } from "@/lib/constants"; +import { + countPasswordCharacterClasses, + MIN_PASSWORD_CHARACTER_CLASSES, +} from "@/lib/passwordPolicy"; +import { useRouter, resolveNextParam } from "@/router"; + +/** + * Public, pre-auth screen the user lands on after Login.tsx detects a + * "password change required" 403 (see classifyLoginError). Distinct from the + * private, authenticated self-service /app/change-password stub — this page + * re-authenticates with the old password, changes it, then logs in again + * with the new password via AuthContext.completePasswordChangeRequired, + * which lands the user in a real session exactly like a normal login. + */ +export function PasswordChangeRequired() { + const intl = useIntl(); + const { isAuthenticated, completePasswordChangeRequired } = useAuth(); + const { navigate } = useRouter(); + const returnTo = resolveNextParam(window.location.search); + const email = useMemo(() => new URLSearchParams(window.location.search).get("email") ?? "", []); + const [oldPassword, setOldPassword] = useState(""); + const [newPassword, setNewPassword] = useState(""); + const [confirmPassword, setConfirmPassword] = useState(""); + const [oldPasswordError, setOldPasswordError] = useState(null); + const [newPasswordError, setNewPasswordError] = useState(null); + const [confirmPasswordError, setConfirmPasswordError] = useState(null); + const [submitError, setSubmitError] = useState(null); + const [showForgotPasswordFallback, setShowForgotPasswordFallback] = useState(false); + const [showReturnToLoginFallback, setShowReturnToLoginFallback] = useState(false); + const [submitting, setSubmitting] = useState(false); + // Stale link / manual nav / bookmark with no ?email= — the read-only email + // field would otherwise be silently blank and submittable, and the BFF's + // generic 400 for a missing email gets misclassified as a new-password + // policy violation by the shared classifier. + const emailMissing = !email; + // Only used for the rare "password changed but couldn't auto sign-in" + // fallback (see classifyChangePasswordRequiredError's changedButLoginFailed) + // — the happy path navigates straight into the app instead of showing this. + const [changedButLoginFailed, setChangedButLoginFailed] = useState(false); + const submitErrorRef = useRef(null); + const oldPasswordInputRef = useRef(null); + const newPasswordInputRef = useRef(null); + const confirmPasswordInputRef = useRef(null); + + useEffect(() => { + if (isAuthenticated) { + navigate(returnTo); + } + }, [isAuthenticated, navigate, returnTo]); + + useEffect(() => { + if (submitError) { + submitErrorRef.current?.focus(); + } + }, [submitError]); + + useEffect(() => { + if (oldPasswordError) { + oldPasswordInputRef.current?.focus(); + } + }, [oldPasswordError]); + + useEffect(() => { + if (newPasswordError) { + newPasswordInputRef.current?.focus(); + } + }, [newPasswordError]); + + useEffect(() => { + if (confirmPasswordError) { + confirmPasswordInputRef.current?.focus(); + } + }, [confirmPasswordError]); + + async function handleSubmit(event: React.FormEvent) { + event.preventDefault(); + setOldPasswordError(null); + setNewPasswordError(null); + setConfirmPasswordError(null); + setSubmitError(null); + setShowForgotPasswordFallback(false); + setShowReturnToLoginFallback(false); + + if (!oldPassword) { + setOldPasswordError( + intl.formatMessage({ id: "auth.passwordChangeRequired.error.oldPasswordRequired" }), + ); + return; + } + if (newPassword.length < VALIDATION.MIN_PASSWORD_LENGTH) { + setNewPasswordError(intl.formatMessage({ id: "auth.passwordChangeRequired.error.tooShort" })); + return; + } + const characterTypes = countPasswordCharacterClasses(newPassword); + if (characterTypes < MIN_PASSWORD_CHARACTER_CLASSES) { + setNewPasswordError( + intl.formatMessage({ id: "auth.passwordChangeRequired.error.complexity" }), + ); + return; + } + if (newPassword !== confirmPassword) { + setConfirmPasswordError( + intl.formatMessage({ id: "auth.passwordChangeRequired.error.mismatch" }), + ); + return; + } + + setSubmitting(true); + try { + // On success, AuthContext's state update flips isAuthenticated, and the + // effect above navigates into the app — no local success state needed. + await completePasswordChangeRequired(email, oldPassword, newPassword); + setOldPassword(""); + setNewPassword(""); + setConfirmPassword(""); + } catch (err) { + const changeError = classifyChangePasswordRequiredError(err); + if (changeError.kind === "changedButLoginFailed") { + setOldPassword(""); + setNewPassword(""); + setConfirmPassword(""); + setChangedButLoginFailed(true); + } else if (changeError.kind === "invalidOldPassword") { + setSubmitError( + intl.formatMessage({ id: "auth.passwordChangeRequired.error.invalidOldPassword" }), + ); + setShowForgotPasswordFallback(true); + } else if (changeError.kind === "notRequired") { + // Credentials were correct — forgot-password would be the wrong + // fallback here, offer a way back to the (now-usable) login form + // instead. + setSubmitError(intl.formatMessage({ id: "auth.passwordChangeRequired.error.notRequired" })); + setShowReturnToLoginFallback(true); + } else if (changeError.kind === "policyViolation") { + setNewPasswordError( + changeError.message ?? + intl.formatMessage({ id: "auth.passwordChangeRequired.error.failed" }), + ); + } else { + setSubmitError(intl.formatMessage({ id: "auth.passwordChangeRequired.error.failed" })); + } + } finally { + setSubmitting(false); + } + } + + return ( + + {changedButLoginFailed ? ( + navigate("/app/login")} + /> + ) : ( + <> +

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

+

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

+ + {emailMissing ? ( +
+ + +
+ ) : ( +
+
+ + +
+ { + setOldPassword(value); + setOldPasswordError(null); + setSubmitError(null); + setShowForgotPasswordFallback(false); + }} + autoComplete="current-password" + label={intl.formatMessage({ id: "auth.passwordChangeRequired.oldPassword" })} + placeholder={intl.formatMessage({ id: "auth.passwordChangeRequired.oldPassword" })} + required + error={oldPasswordError ?? undefined} + /> + { + setNewPassword(value); + setNewPasswordError(null); + setConfirmPasswordError(null); + }} + label={intl.formatMessage({ id: "auth.passwordChangeRequired.newPassword" })} + placeholder={intl.formatMessage({ id: "auth.passwordChangeRequired.newPassword" })} + required + hint={intl.formatMessage({ id: "auth.passwordChangeRequired.passwordHint" })} + error={newPasswordError ?? undefined} + /> + { + setConfirmPassword(value); + setConfirmPasswordError(null); + }} + label={intl.formatMessage({ id: "auth.passwordChangeRequired.confirmPassword" })} + placeholder={intl.formatMessage({ + id: "auth.passwordChangeRequired.confirmPassword", + })} + required + error={confirmPasswordError ?? undefined} + /> + {submitError && ( +
+ +
+ )} + + {showForgotPasswordFallback && ( + + )} + {showReturnToLoginFallback && ( + + )} + + )} + + )} +
+ ); +} diff --git a/src/pages/ResetPassword.tsx b/src/pages/ResetPassword.tsx index 9d6f76e..a3bc491 100644 --- a/src/pages/ResetPassword.tsx +++ b/src/pages/ResetPassword.tsx @@ -1,13 +1,18 @@ -import { useEffect, useRef, useState } from "react"; -import { CircleCheck } from "lucide-react"; +import { useEffect, useState } from "react"; import { useIntl } from "react-intl"; import { resetPassword, validatePasswordResetToken } from "@/api/passwordReset"; import { classifyPasswordResetError, type PasswordResetError } from "@/api/passwordResetErrors"; +import { AuthCard } from "@/components/auth/AuthCard"; +import { AuthSuccessPanel } from "@/components/auth/AuthSuccessPanel"; 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 { + countPasswordCharacterClasses, + MIN_PASSWORD_CHARACTER_CLASSES, +} from "@/lib/passwordPolicy"; import { useRouter } from "@/router"; type TokenState = @@ -34,7 +39,6 @@ export function ResetPassword({ token = "" }: { token?: string }) { const [submitError, setSubmitError] = useState(null); const [submitting, setSubmitting] = useState(false); const [succeeded, setSucceeded] = useState(false); - const successHeadingRef = useRef(null); useEffect(() => { const controller = new AbortController(); @@ -54,10 +58,6 @@ export function ResetPassword({ token = "" }: { token?: string }) { return () => controller.abort(); }, [token]); - useEffect(() => { - if (succeeded) successHeadingRef.current?.focus(); - }, [succeeded]); - async function handleSubmit(event: React.FormEvent) { event.preventDefault(); setPasswordError(null); @@ -68,10 +68,8 @@ export function ResetPassword({ token = "" }: { token?: string }) { 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) { + const characterTypes = countPasswordCharacterClasses(password); + if (characterTypes < MIN_PASSWORD_CHARACTER_CLASSES) { setPasswordError(intl.formatMessage({ id: "auth.resetPassword.error.complexity" })); return; } @@ -124,119 +122,90 @@ export function ResetPassword({ token = "" }: { token?: string }) { const canRequestNewLink = tokenState === "invalid" || tokenState === "expired"; return ( -
-
- {succeeded ? ( -
-
-

+ {succeeded ? ( + navigate("/app/login")} + /> + ) : ( + <> +

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

+

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

+ + {tokenState === "validating" ? ( +
+ +
+ ) : tokenState !== "valid" ? ( +
+ +

-

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

+ {intl.formatMessage({ + id: canRequestNewLink + ? "auth.resetPassword.requestNewLink" + : "auth.resetPassword.returnToLogin", + })} +
- -
- ) : ( - <> -

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

-

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

- - {tokenState === "validating" ? ( -
- -
- ) : tokenState !== "valid" ? ( -
- - -
- ) : ( -
- { - setPassword(value); - setPasswordError(null); - setConfirmPasswordError(null); - setSubmitError(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); - setSubmitError(null); - }} - label={intl.formatMessage({ id: "auth.resetPassword.confirmPassword" })} - placeholder={intl.formatMessage({ id: "auth.resetPassword.confirmPassword" })} - required - error={confirmPasswordError ?? undefined} - /> - {submitError && } - - - )} - - )} -
-
+ ) : ( +
+ { + setPassword(value); + setPasswordError(null); + setConfirmPasswordError(null); + setSubmitError(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); + setSubmitError(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/Users.test.tsx b/src/pages/Users.test.tsx index b326aba..efe4866 100644 --- a/src/pages/Users.test.tsx +++ b/src/pages/Users.test.tsx @@ -90,6 +90,7 @@ function makeAuthContext(email = "admin@example.com") { isLoading: false, selectedTeamId: null, login: vi.fn(), + completePasswordChangeRequired: vi.fn(), logout: vi.fn(), setSelectedTeamId: vi.fn(), permissions: [], diff --git a/src/router/index.tsx b/src/router/index.tsx index b3280ea..38f65b6 100644 --- a/src/router/index.tsx +++ b/src/router/index.tsx @@ -241,7 +241,11 @@ export function Redirect({ to }: { to: string }) { // --------------------------------------------------------------------------- // Exact paths that are always public. -const DEFAULT_PUBLIC_PATHS: readonly string[] = ["/app/login", "/app/forgot-password"]; +const DEFAULT_PUBLIC_PATHS: readonly string[] = [ + "/app/login", + "/app/forgot-password", + "/app/change-password-required", +]; // Path prefixes whose subtrees are always public. const DEFAULT_PUBLIC_PREFIXES: readonly string[] = ["/app/reset-password/"]; diff --git a/src/utils/errors.test.ts b/src/utils/errors.test.ts index e23bb8e..cba65f0 100644 --- a/src/utils/errors.test.ts +++ b/src/utils/errors.test.ts @@ -1,5 +1,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { extractApiErrorDetail, sanitizeError, withErrorHandling } from "./errors"; +import { + extractApiErrorDetail, + extractUpstreamApiErrorDetail, + sanitizeError, + withErrorHandling, +} from "./errors"; describe("extractApiErrorDetail", () => { it("returns null for null", () => { @@ -51,6 +56,36 @@ describe("extractApiErrorDetail", () => { }); }); +describe("extractUpstreamApiErrorDetail", () => { + it("unwraps a JSON-encoded detail string (BFF-owned auth routes forward raw upstream text)", () => { + const detail = JSON.stringify({ + detail: "Password must be at least 22 characters long (privileged account)", + }); + expect(extractUpstreamApiErrorDetail({ detail })).toBe( + "Password must be at least 22 characters long (privileged account)", + ); + }); + + it("unwraps a JSON-encoded FastAPI validation-error array", () => { + const detail = JSON.stringify({ detail: [{ msg: "too short" }] }); + expect(extractUpstreamApiErrorDetail({ detail })).toBe("too short"); + }); + + it("falls back to the raw string when detail is plain text, not JSON", () => { + expect(extractUpstreamApiErrorDetail({ detail: "Too many requests" })).toBe( + "Too many requests", + ); + }); + + it("returns null when detail is missing", () => { + expect(extractUpstreamApiErrorDetail({})).toBeNull(); + }); + + it("returns null for a non-object body", () => { + expect(extractUpstreamApiErrorDetail(null)).toBeNull(); + }); +}); + describe("sanitizeError", () => { beforeEach(() => { vi.spyOn(console, "error").mockImplementation(() => {}); diff --git a/src/utils/errors.ts b/src/utils/errors.ts index 45bccfb..dedd14f 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -48,6 +48,34 @@ export function extractApiErrorDetail(body: unknown): string | null { return null; } +/** + * Extracts the real error message from a BFF-owned auth route's forwarded + * failure envelope. Routes like server/src/routes/auth/login.ts and + * change-password-required.ts forward a rejected upstream response as + * `{ error: , detail: }` — when + * upstream's body was JSON, `detail` here is itself a JSON-encoded string, so + * naively treating it as the message (extractApiErrorDetail's usual contract) + * renders the raw JSON text. Parse it defensively: other failures (e.g. a + * bare rate-limit string) may not be JSON at all, in which case `detail` IS + * already the message. + * + * @param body - The error body from a BFF-owned auth route's ApiError + * @returns Extracted error message or null if not found + */ +export function extractUpstreamApiErrorDetail(body: unknown): string | null { + if (!body || typeof body !== "object") return null; + const envelope = body as { detail?: string }; + if (typeof envelope.detail !== "string") return null; + + try { + const inner = extractApiErrorDetail(JSON.parse(envelope.detail)); + if (inner) return inner; + } catch { + // Not JSON — upstream sent plain text, `detail` is already the message. + } + return envelope.detail; +} + /** * Sanitizes error messages to prevent information leakage *