diff --git a/docs/contract-gaps.md b/docs/contract-gaps.md index 3cbb42e..bd2b0be 100644 --- a/docs/contract-gaps.md +++ b/docs/contract-gaps.md @@ -103,6 +103,36 @@ case") is exactly the defect RUK-258 removed. --- +### `password_set` on /me — read by the frontend, not yet on the wire + +| Field | Where it is needed | What is on the wire | Ticket | Stub | +| -------------- | --------------------------------------------------- | --------------------------------------- | ------- | ---------------- | +| `password_set` | profile password card, `/set-password` (RUK-289 UI) | the key is absent from the recorded 200 | RUK-289 | none — see below | + +The backend that serves this field is written but sits on an unmerged branch, so +the recorded fixture predates it and the field arrives as `undefined` until that +branch ships. The frontend types it optional and treats `undefined` as "unknown" +rather than as `false`: the two mean different things, and collapsing them would +draw a set-password form for every operator, whose every save is then a 400. + +**There is no mapper stub, because there is no mapper.** `/api/me` is an +unnarrowed pass-through (`NextResponse.json(data)`), so nothing substitutes a +placeholder value the way `notify-channel-mapper.ts` does for `updated_at` — the +key is simply not there. The Stub column has nothing to point at. + +**This row is prose, in the sense the `updated_at` row above means it** — the +Class-B stub-scanner is pinned to `maintenance-mapper.ts` and cannot cover it. +Unlike that row, however, this one is not unprotected: `contract-gaps.test.ts` +asserts that `password_set` is **absent** from `me.json`, which is green today +and goes red on the day the fixture is re-recorded against a backend that sends +it. That is the day this row is owed deletion, along with the assertion itself. + +The pass-through assertion in `me.contract.test.ts` deliberately does NOT serve +this purpose: it compares the route's echo against the same fixture that fed its +mock, so it is a tautology on key sets and stays green whatever the fixture +holds. It proves the route narrows nothing; it says nothing about which fields +exist. + ## Class B′ — the backend sends it, the frontend does not read it The opposite direction. It does not break a screen, but it means data the diff --git a/src/app/(app)/set-password/page.tsx b/src/app/(app)/set-password/page.tsx new file mode 100644 index 0000000..b89a447 --- /dev/null +++ b/src/app/(app)/set-password/page.tsx @@ -0,0 +1,11 @@ +import { SetPasswordPage } from "@/features/settings/set-password-page"; + +/** + * Deliberately NOT wrapped in `AppShell`: this is a single-purpose page reached + * from the profile, and the app chrome around one form is noise. It sits under + * `(app)` for the session and the provider tree, which the form's mutation + * needs and which `(public)` does not mount. + */ +export default function Page() { + return ; +} diff --git a/src/app/(public)/login/page.tsx b/src/app/(public)/login/page.tsx index 29c7a89..0ecc280 100644 --- a/src/app/(public)/login/page.tsx +++ b/src/app/(public)/login/page.tsx @@ -5,6 +5,12 @@ import { credentialsSignInAction, requestOtpAction, } from "@/server/auth/built-in-sign-in-actions"; +import { + abandonPasswordResetAction, + confirmPasswordResetAction, + requestPasswordResetAction, +} from "@/server/auth/password-reset-actions"; +import { readPasswordResetBinding } from "@/server/auth/otp-nonce-cookie"; import { signIn } from "@/server/auth/auth-config"; import { safeNext } from "@/server/auth/safe-next"; @@ -41,6 +47,16 @@ export default async function Page({ // break-glass fallback rather than a 500. const providers = await resolveAuthProviders(); + // A password reset spans an email round-trip, so the user leaves this tab and + // comes back — reload is the flow's primary re-entry, not an edge case. The + // cookie is httpOnly and readable only here, so the step has to be resolved + // server-side and handed down. + // + // ONLY the address crosses. The nonce stays in the cookie: it is httpOnly + // precisely so browser JavaScript cannot read the binding, and passing the + // whole binding to a client component would give that away for nothing. + const resetBinding = await readPasswordResetBinding(); + /** * The built-in methods post through server actions rather than a client * `fetch`: NextAuth attaches its CSRF token only when `signIn` runs on the @@ -66,6 +82,10 @@ export default async function Page({ otpSignInAction={otpSignInAction} passwordSignInAction={passwordSignInAction} changeEmailAction={changeEmailAction} + requestPasswordResetAction={requestPasswordResetAction} + confirmPasswordResetAction={confirmPasswordResetAction} + abandonPasswordResetAction={abandonPasswordResetAction} + resetInProgressEmail={resetBinding?.email} /> ); } diff --git a/src/app/api/me/password/__tests__/route.test.ts b/src/app/api/me/password/__tests__/route.test.ts new file mode 100644 index 0000000..68ef13e --- /dev/null +++ b/src/app/api/me/password/__tests__/route.test.ts @@ -0,0 +1,184 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { POST } from "@/app/api/me/password/route"; + +const readActiveSession = vi.fn(); +const changeBackendPassword = vi.fn(); +const isSameOriginRequest = vi.fn(() => true); + +vi.mock("@/server/auth/session-token", () => ({ + readActiveSession: () => readActiveSession(), +})); +vi.mock("@/server/auth/backend-token-exchange", () => ({ + changeBackendPassword: (...args: unknown[]) => changeBackendPassword(...args), +})); +vi.mock("@/server/backend/security/csrf", () => ({ + isSameOriginRequest: () => isSameOriginRequest(), +})); + +function post(body: unknown) { + return new Request("https://app.test/api/me/password", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + isSameOriginRequest.mockReturnValue(true); + readActiveSession.mockResolvedValue({ + accessToken: "access-1", + refreshToken: "refresh-1", + accessTokenExpiresAt: Date.now() + 600_000, + }); +}); + +describe("the session is read once and sent verbatim", () => { + // AC-6. `readActiveSession` rotates the refresh token when the access token + // is near expiry, so a second read can hand back a different token — and the + // backend answers a superseded `refresh_token` with a 401 and NO change. + it("reads the session exactly once per request", async () => { + changeBackendPassword.mockResolvedValue({ ok: true }); + + await POST(post({ current_password: "old-password", new_password: "new-password-here" })); + + expect(readActiveSession).toHaveBeenCalledTimes(1); + }); + + // Without this the backend revokes EVERY session including the caller's, and + // the user is signed out by their own success. + it("sends the refresh token so the caller's own session survives", async () => { + changeBackendPassword.mockResolvedValue({ ok: true }); + + await POST(post({ current_password: "old-password", new_password: "new-password-here" })); + + expect(changeBackendPassword).toHaveBeenCalledWith( + expect.objectContaining({ accessToken: "access-1", refreshToken: "refresh-1" }), + ); + }); + + it("never retries the mutation", async () => { + changeBackendPassword.mockResolvedValue({ ok: false, kind: "session-stale" }); + + await POST(post({ new_password: "new-password-here" })); + + // One call, however it failed. A retry would send a token the refresh had + // already replaced, and the password would silently stay unchanged. + expect(changeBackendPassword).toHaveBeenCalledTimes(1); + }); + + it("omits current_password entirely when the account has none", async () => { + changeBackendPassword.mockResolvedValue({ ok: true }); + + await POST(post({ new_password: "new-password-here" })); + + // Absent, not empty: the backend rejects the field outright for an account + // with no password, so "" and undefined are different requests. + expect(changeBackendPassword).toHaveBeenCalledWith( + expect.objectContaining({ currentPassword: undefined }), + ); + }); +}); + +describe("no failure may answer 401", () => { + // AC-7, and the reason this route exists at all. `bffFetch` navigates to + // /login on any 401 carrying AUTH_REQUIRED, behind a never-resolving promise + // — so answering a wrong current password with a 401 signs the operator out + // on their most common mistake, with no message. + it("answers a wrong current password with a renderable status", async () => { + changeBackendPassword.mockResolvedValue({ ok: false, kind: "wrong-current-password" }); + + const response = await POST(post({ current_password: "wrong", new_password: "new-password-here" })); + + expect(response.status).toBe(422); + expect(response.status).not.toBe(401); + await expect(response.json()).resolves.toMatchObject({ code: "WRONG_CURRENT_PASSWORD" }); + }); + + it("distinguishes a stale session from a wrong password", async () => { + changeBackendPassword.mockResolvedValue({ ok: false, kind: "session-stale" }); + + const response = await POST(post({ new_password: "new-password-here" })); + + // Different meaning, different copy: nothing the user typed was wrong and + // the password was NOT changed. + expect(response.status).toBe(409); + await expect(response.json()).resolves.toMatchObject({ code: "SESSION_STALE" }); + }); + + it("passes a backend 400 through without parsing its prose", async () => { + changeBackendPassword.mockResolvedValue({ + ok: false, + kind: "rejected", + message: "validation error: the current password is required", + }); + + const response = await POST(post({ new_password: "new-password-here" })); + + expect(response.status).toBe(400); + await expect(response.json()).resolves.toMatchObject({ code: "INVALID_REQUEST" }); + }); + + it("answers an outage as an outage", async () => { + changeBackendPassword.mockResolvedValue({ ok: false, kind: "unavailable" }); + + const response = await POST(post({ new_password: "new-password-here" })); + + expect(response.status).toBe(503); + }); +}); + +describe("the guards every mutating route here carries", () => { + it("refuses a cross-origin request", async () => { + isSameOriginRequest.mockReturnValue(false); + + const response = await POST(post({ new_password: "new-password-here" })); + + expect(response.status).toBe(403); + expect(changeBackendPassword).not.toHaveBeenCalled(); + }); + + it("answers 401 when there is genuinely no session", async () => { + // The one legitimate 401: no session at all, which IS the redirect case. + readActiveSession.mockResolvedValue(null); + + const response = await POST(post({ new_password: "new-password-here" })); + + expect(response.status).toBe(401); + await expect(response.json()).resolves.toMatchObject({ code: "AUTH_REQUIRED" }); + }); + + it("answers a malformed body with a 400, not a 500", async () => { + const malformed = new Request("https://app.test/api/me/password", { + method: "POST", + headers: { "content-type": "application/json" }, + body: "{not json", + }); + + const response = await POST(malformed); + + // An uncaught throw here would be a Next 500 whose body is not the envelope + // `bffFetch` parses, so the card would surface framework noise. + expect(response.status).toBe(400); + expect(changeBackendPassword).not.toHaveBeenCalled(); + }); + + it("rejects a request with no new password", async () => { + const response = await POST(post({ current_password: "old-password" })); + + expect(response.status).toBe(400); + expect(changeBackendPassword).not.toHaveBeenCalled(); + }); +}); + +describe("success", () => { + it("answers 204 with no body", async () => { + changeBackendPassword.mockResolvedValue({ ok: true }); + + const response = await POST(post({ new_password: "new-password-here" })); + + expect(response.status).toBe(204); + await expect(response.text()).resolves.toBe(""); + }); +}); diff --git a/src/app/api/me/password/route.ts b/src/app/api/me/password/route.ts new file mode 100644 index 0000000..32de993 --- /dev/null +++ b/src/app/api/me/password/route.ts @@ -0,0 +1,112 @@ +import { NextResponse } from "next/server"; + +import { changeBackendPassword } from "@/server/auth/backend-token-exchange"; +import { readActiveSession } from "@/server/auth/session-token"; +import { routeErrorResponse } from "@/server/backend/errors/bff-error"; +import { readJsonBody } from "@/server/backend/http/read-json-body"; +import { isSameOriginRequest } from "@/server/backend/security/csrf"; + +interface ChangePasswordBody { + /** Omitted when the account has no password yet — the backend rejects it then. */ + current_password?: string; + new_password?: string; +} + +/** + * POST /api/me/password — proxy to backend `POST /api/v1/me/password`. + * + * This route deliberately does NOT use `authenticatedBackendRequest`, and does + * not let a backend 401 reach `routeErrorResponse`. Both defaults are wrong + * here in the same direction: + * + * - the wrapper refreshes and RETRIES the mutation on a 401, but this endpoint + * takes a `refresh_token` in its body, so the retry would send the token the + * refresh just replaced — the backend answers 401 again and the password is + * never changed; + * - `routeErrorResponse` maps every `BackendUnauthorizedError` to + * `AUTH_REQUIRED`, which `bffFetch` answers by navigating to `/login` behind + * a never-resolving promise. A wrong current password is a 401, so the most + * common mistake on this form would sign the operator out with no message, + * indistinguishable from an expired session. + * + * So: read the session ONCE, send that exact token, never retry, and answer + * every failure with a non-401 status the card can render in place. + */ +export async function POST(request: Request) { + if (!isSameOriginRequest(request)) { + return NextResponse.json( + { error: "Cross-origin requests are not allowed", code: "FORBIDDEN" }, + { status: 403 }, + ); + } + + // Read once. `readActiveSession` rotates the refresh token when the access + // token is close to expiry, so calling it twice — or letting anything else + // refresh between the read and the send — puts a superseded token in the body. + const session = await readActiveSession(); + if (!session) { + return NextResponse.json({ error: "Sign-in is required", code: "AUTH_REQUIRED" }, { status: 401 }); + } + + // Caught rather than left to propagate: `readJsonBody` throws a + // `BffValidationError` on malformed JSON, and an uncaught throw out of a route + // handler is a Next 500 whose body is not the `{error, code}` envelope + // `bffFetch` parses — the card would print framework noise into the form. The + // catch is scoped to the parse alone, so the deliberate hand-rolled handling + // of the backend's own failures below still bypasses the generic mapper. + let body: ChangePasswordBody | undefined; + try { + body = await readJsonBody(request); + } catch (error) { + return routeErrorResponse(error); + } + + const newPassword = body?.new_password; + if (typeof newPassword !== "string" || !newPassword) { + return NextResponse.json( + { error: "A new password is required", code: "INVALID_REQUEST" }, + { status: 400 }, + ); + } + + const outcome = await changeBackendPassword({ + accessToken: session.accessToken, + currentPassword: body?.current_password || undefined, + newPassword, + // Keeps THIS session alive while the backend revokes the others. Without + // it the backend revokes every session including the caller's, and the user + // is signed out by their own success. + refreshToken: session.refreshToken, + }); + + if (outcome.ok) { + return new NextResponse(null, { status: 204 }); + } + + switch (outcome.kind) { + case "wrong-current-password": + // 422, not 401: the status is what decides whether `bffFetch` navigates + // away, and this is a field the user can correct in place. + return NextResponse.json( + { error: "That current password isn't right", code: "WRONG_CURRENT_PASSWORD" }, + { status: 422 }, + ); + case "session-stale": + // Also not a 401, for the same mechanical reason — but the meaning is + // different and so is the copy: nothing the user typed was wrong, and + // the password was NOT changed. + return NextResponse.json( + { error: "Your session expired before the change was saved", code: "SESSION_STALE" }, + { status: 409 }, + ); + case "rejected": + // The backend's own message, passed through as an opaque string. Never + // parsed: its three 400s share one code and differ only in prose. + return NextResponse.json({ error: outcome.message, code: "INVALID_REQUEST" }, { status: 400 }); + default: + return NextResponse.json( + { error: "Password change is unavailable right now", code: "BACKEND_UNAVAILABLE" }, + { status: 503 }, + ); + } +} diff --git a/src/domain/admin/user.ts b/src/domain/admin/user.ts index 87819b2..51b51f3 100644 --- a/src/domain/admin/user.ts +++ b/src/domain/admin/user.ts @@ -27,6 +27,25 @@ export interface User { * optional so the FE tolerates the pre-RUK-202 wire that omits it entirely. */ timezone?: string | null; + /** + * Whether this account has a password credential (RUK-289, `GET`/`PATCH /me`). + * + * THREE values, not two, and the difference decides which form the profile + * card draws: + * + * - `true` — a password exists. + * - `false` — no password, OR the backend's read of the credential failed + * (it logs and answers `false` with a 200). So `false` is a + * usable hint for choosing a form and is NOT a security + * assertion; a wrong guess is answered by the endpoint with a + * 400, never accepted silently. + * - absent — the deployed backend predates the field. + * + * Optional for that last reason, and `undefined` must never be coerced to + * `false`: they mean different things and the UI treats them differently + * (SPEC §1.4, §2.4). Same shape as `timezone` above, for the same reason. + */ + password_set?: boolean; /** * Telegram handle used to name this person in notification text (RUK-217). * Stored **verbatim**, including any leading `@` — `@username` and `username` are diff --git a/src/domain/auth/__tests__/password-policy.test.ts b/src/domain/auth/__tests__/password-policy.test.ts new file mode 100644 index 0000000..3e3cad9 --- /dev/null +++ b/src/domain/auth/__tests__/password-policy.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; + +import { isPasswordWithinPolicy } from "@/domain/auth/sign-in-method"; + +describe("password policy is measured in bytes, as the backend measures it", () => { + it("rejects 11 ASCII characters and accepts 12", () => { + expect(isPasswordWithinPolicy("a".repeat(11))).toBe(false); + expect(isPasswordWithinPolicy("a".repeat(12))).toBe(true); + }); + + it("rejects 257 ASCII characters and accepts 256", () => { + expect(isPasswordWithinPolicy("a".repeat(257))).toBe(false); + expect(isPasswordWithinPolicy("a".repeat(256))).toBe(true); + }); + + // The case that makes this suite bite. A naive `.length >= 12` implementation + // REJECTS this — 11 characters — while the backend ACCEPTS it, because the + // string is 22 bytes. Getting this wrong on the reset path surfaces as + // "that code is wrong" about a perfectly good code. + it("accepts 11 Cyrillic characters, which are 22 bytes", () => { + const password = "паролькудлин"; + expect(password.length).toBe(12); + expect(new TextEncoder().encode(password).length).toBe(24); + + const eleven = "паролькудли"; + expect(eleven.length).toBe(11); + expect(new TextEncoder().encode(eleven).length).toBe(22); + expect(isPasswordWithinPolicy(eleven)).toBe(true); + }); + + // The mirror: 6 Cyrillic characters are exactly 12 bytes, so the backend + // accepts them and so must we, however short they look. + it("accepts 6 Cyrillic characters, which are exactly 12 bytes", () => { + expect(new TextEncoder().encode("пароль").length).toBe(12); + expect(isPasswordWithinPolicy("пароль")).toBe(true); + }); + + it("rejects a 256-character Cyrillic password, which is 512 bytes", () => { + expect(isPasswordWithinPolicy("я".repeat(256))).toBe(false); + }); +}); diff --git a/src/domain/auth/sign-in-method.ts b/src/domain/auth/sign-in-method.ts index 9786eff..fff97f1 100644 --- a/src/domain/auth/sign-in-method.ts +++ b/src/domain/auth/sign-in-method.ts @@ -40,3 +40,32 @@ export function isKnownSignInMethodType(value: string): value is SignInMethodTyp export function isWellFormedOtpCode(value: string): boolean { return /^\d{6}$/.test(value.trim()); } + +/** + * The backend's password length policy, in BYTES (RUK-289). + * + * Bytes, not characters, and the distinction is not pedantic. The backend + * validates with ozzo's `Length` rather than `RuneLength`, which measures Go's + * `len()` — so "аброакадабра" is 12 characters but 24 bytes, and a 6-character + * Cyrillic password is 12 bytes and passes. Measuring `.length` here would + * diverge from the server for every non-ASCII password. + */ +export const PASSWORD_MIN_BYTES = 12; +export const PASSWORD_MAX_BYTES = 256; + +/** + * Whether a password satisfies the backend's policy, measured the way the + * backend measures it. + * + * Duplicating a server-side rule on the client is usually a smell. Here it is + * required: on the reset path a policy violation is collapsed into the same + * 401 as a wrong code, so without this check a user with a short password is + * told the code they just read off their screen is wrong. The maximum matters + * for the same reason — an over-long password lands in that same collapse. + * + * This is a UX guard, never an authorization one. The backend re-checks. + */ +export function isPasswordWithinPolicy(password: string): boolean { + const bytes = new TextEncoder().encode(password).length; + return bytes >= PASSWORD_MIN_BYTES && bytes <= PASSWORD_MAX_BYTES; +} diff --git a/src/features/_shared/queries/use-me-query.ts b/src/features/_shared/queries/use-me-query.ts index 2a9e982..e9bec14 100644 --- a/src/features/_shared/queries/use-me-query.ts +++ b/src/features/_shared/queries/use-me-query.ts @@ -114,3 +114,35 @@ export function useUpdateMyTags() { }, }); } + +/** What the change-password card sends. `current_password` is omitted, never + * empty, when the account has none — the backend rejects the field outright in + * that state, so "" and absent are different requests. */ +export type ChangePasswordArgs = { + current_password?: string; + new_password: string; +}; + +/** + * Set or change the caller's password (RUK-289) via `POST /api/me/password`. + * + * Unlike its siblings this cannot seed the cache from the response: the backend + * answers 204 with an empty body, so there is no user object to write. It + * INVALIDATES instead — `password_set` has just flipped, and `useMeQuery` holds + * its answer for `staleTime`, which would leave the card drawing the form for + * the state the user was in a minute ago. + */ +export function useChangePassword() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async (args: ChangePasswordArgs): Promise => { + await bffFetch("/api/me/password", { + method: "POST", + body: JSON.stringify(args), + }); + }, + onSuccess: () => { + void queryClient.invalidateQueries({ queryKey: meKey() }); + }, + }); +} diff --git a/src/features/auth/__tests__/accept-invite-no-query-provider.test.tsx b/src/features/auth/__tests__/accept-invite-no-query-provider.test.tsx index 239718b..65baa21 100644 --- a/src/features/auth/__tests__/accept-invite-no-query-provider.test.tsx +++ b/src/features/auth/__tests__/accept-invite-no-query-provider.test.tsx @@ -67,6 +67,9 @@ describe("public pages render without a QueryClientProvider", () => { otpSignInAction: async () => ({}), passwordSignInAction: async () => ({}), changeEmailAction: async () => {}, + requestPasswordResetAction: async () => ({}), + confirmPasswordResetAction: async () => ({}), + abandonPasswordResetAction: async () => {}, }; expect(() => diff --git a/src/features/auth/__tests__/login-page-methods.test.tsx b/src/features/auth/__tests__/login-page-methods.test.tsx index 2cea0bd..e078bfd 100644 --- a/src/features/auth/__tests__/login-page-methods.test.tsx +++ b/src/features/auth/__tests__/login-page-methods.test.tsx @@ -21,6 +21,9 @@ const actions = { otpSignInAction: async () => ({}), passwordSignInAction: async () => ({}), changeEmailAction: async () => {}, + requestPasswordResetAction: async () => ({}), + confirmPasswordResetAction: async () => ({}), + abandonPasswordResetAction: async () => {}, }; const PASSWORD: SignInMethod = { id: "email_password", type: "password", display_name: "Password" }; diff --git a/src/features/auth/__tests__/login-page-reset.test.tsx b/src/features/auth/__tests__/login-page-reset.test.tsx new file mode 100644 index 0000000..6a158cb --- /dev/null +++ b/src/features/auth/__tests__/login-page-reset.test.tsx @@ -0,0 +1,119 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; + +import { LoginPage } from "@/features/auth/login-page"; +import type { SignInMethod } from "@/domain/auth/sign-in-method"; + +afterEach(() => cleanup()); + +const PASSWORD_METHOD: SignInMethod = { + id: "email_password", + type: "password", + display_name: "Password", +}; +const CODE_METHOD: SignInMethod = { id: "email_otp", type: "code", display_name: "Email code" }; + +function renderLogin( + overrides: Partial> = {}, +): React.ComponentProps { + const props: React.ComponentProps = { + methods: [PASSWORD_METHOD], + signInAction: vi.fn(async () => {}), + requestOtpAction: vi.fn(async () => ({})), + otpSignInAction: vi.fn(async () => ({})), + passwordSignInAction: vi.fn(async () => ({})), + changeEmailAction: vi.fn(async () => {}), + requestPasswordResetAction: vi.fn(async () => ({})), + confirmPasswordResetAction: vi.fn(async () => ({ done: true })), + abandonPasswordResetAction: vi.fn(async () => {}), + ...overrides, + }; + render(); + return props; +} + +describe("the entry point into the reset flow", () => { + // Without this the feature has no way in and nothing else would fail. + it("offers 'Forgot password?' beside the password form", () => { + renderLogin(); + + expect(screen.getByRole("button", { name: "Forgot password?" })).toBeTruthy(); + }); + + it("opens the reset flow when it is clicked", () => { + renderLogin(); + fireEvent.click(screen.getByRole("button", { name: "Forgot password?" })); + + expect(screen.getByLabelText("Reset your password")).toBeTruthy(); + // The sign-in form is gone: the page shows one thing to do at a time. + expect(screen.queryByLabelText("Email")).toBeNull(); + }); + + // The affordance belongs to the password form, so an instance advertising + // only emailed codes must not offer to reset a password it does not accept. + it("offers nothing when the backend advertises no password method", () => { + renderLogin({ methods: [CODE_METHOD] }); + + expect(screen.queryByRole("button", { name: "Forgot password?" })).toBeNull(); + }); +}); + +describe("rehydration after a reload", () => { + // A reset spans an email round-trip, so the user leaves the tab. Without this + // they come back to step one and silently lose the code they were sent. + it("resumes at step two when the server hands down a live binding", () => { + renderLogin({ resetInProgressEmail: "op@example.test" }); + + expect(screen.getByLabelText("Enter the 6-digit code")).toBeTruthy(); + expect(screen.getByText(/Sent to op@example.test/)).toBeTruthy(); + }); + + // SPEC §2.1: the advertised method list is the authority on what the page + // offers. A cookie must not resurrect a method an operator has switched off. + it("ignores a live binding when password sign-in is no longer offered", () => { + renderLogin({ resetInProgressEmail: "op@example.test", methods: [CODE_METHOD] }); + + expect(screen.queryByLabelText("Enter the 6-digit code")).toBeNull(); + expect(screen.getByLabelText("Email code")).toBeTruthy(); + }); + + it("starts at step one when there is no binding", () => { + renderLogin(); + fireEvent.click(screen.getByRole("button", { name: "Forgot password?" })); + + expect(screen.getByLabelText("Reset your password")).toBeTruthy(); + }); +}); + +describe("what the user is told afterwards", () => { + // The backend has revoked every session and returned no tokens, so there is + // nothing to sign into — the confirmation is the entire outcome. SPEC §2.1 + // names shipping without it as the failure to avoid. + it("confirms the change and returns to the sign-in form", async () => { + renderLogin({ resetInProgressEmail: "op@example.test" }); + + fireEvent.change(screen.getByLabelText("Enter the 6-digit code"), { + target: { value: "123456" }, + }); + fireEvent.change(screen.getByLabelText("New password"), { + target: { value: "a-long-enough-password" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Set new password" })); + + await waitFor(() => { + expect(screen.getByRole("status").textContent).toMatch(/password updated/i); + }); + // Back on the sign-in form, which is where the new password is used. + expect(screen.getByLabelText("Password")).toBeTruthy(); + }); + + it("returns to sign-in without a confirmation when the user backs out", () => { + renderLogin(); + fireEvent.click(screen.getByRole("button", { name: "Forgot password?" })); + fireEvent.click(screen.getByRole("button", { name: "Back to sign in" })); + + expect(screen.getByLabelText("Password")).toBeTruthy(); + expect(screen.queryByRole("status")).toBeNull(); + }); +}); diff --git a/src/features/auth/__tests__/login-page.test.tsx b/src/features/auth/__tests__/login-page.test.tsx index 5b3e982..36293b2 100644 --- a/src/features/auth/__tests__/login-page.test.tsx +++ b/src/features/auth/__tests__/login-page.test.tsx @@ -18,6 +18,9 @@ function renderLogin(error?: string) { requestOtpAction={async () => ({})} otpSignInAction={async () => ({})} passwordSignInAction={async () => ({})} + requestPasswordResetAction={async () => ({})} + confirmPasswordResetAction={async () => ({})} + abandonPasswordResetAction={async () => {}} changeEmailAction={async () => {}} />, ); diff --git a/src/features/auth/__tests__/password-reset-flow.test.tsx b/src/features/auth/__tests__/password-reset-flow.test.tsx new file mode 100644 index 0000000..293558a --- /dev/null +++ b/src/features/auth/__tests__/password-reset-flow.test.tsx @@ -0,0 +1,395 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; + +import { PasswordResetFlow } from "@/features/auth/password-reset-flow"; +import { MAX_CODE_ATTEMPTS } from "@/features/auth/use-code-timers"; + +afterEach(() => cleanup()); + +const LONG_ENOUGH = "a-long-enough-password"; + +function setup(overrides: Partial> = {}) { + const props = { + requestCode: vi.fn(async () => ({})), + confirm: vi.fn(async () => ({ done: true })), + abandon: vi.fn(async () => {}), + onDone: vi.fn(), + onCancel: vi.fn(), + ...overrides, + }; + render(); + return props; +} + +/** Walks step one so the code + password fields are on screen. */ +async function reachCodeStep(props: ReturnType) { + fireEvent.change(screen.getByLabelText("Reset your password"), { + target: { value: "op@example.test" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Email me a code" })); + await waitFor(() => expect(props.requestCode).toHaveBeenCalled()); + await screen.findByLabelText("Enter the 6-digit code"); +} + +async function submitCode(code: string, password: string) { + fireEvent.change(screen.getByLabelText("Enter the 6-digit code"), { target: { value: code } }); + fireEvent.change(screen.getByLabelText("New password"), { target: { value: password } }); + fireEvent.click(screen.getByRole("button", { name: "Set new password" })); +} + +describe("the reset flow's two steps", () => { + it("asks for an address, then for a code and a new password", async () => { + const props = setup(); + await reachCodeStep(props); + + expect(screen.getByLabelText("New password")).toBeTruthy(); + expect(screen.getByText(/Sent to op@example.test/)).toBeTruthy(); + }); + + it("hands the confirmation up rather than trying to sign the user in", async () => { + // The backend answers 204 with no tokens and has revoked every session, so + // there is nothing to sign into — the page owns what the user is told. + const props = setup(); + await reachCodeStep(props); + await submitCode("123456", LONG_ENOUGH); + + await waitFor(() => expect(props.onDone).toHaveBeenCalled()); + }); +}); + +describe("the client-side length check", () => { + // AC-3, at the UI. Without this the request goes out, the backend answers the + // same 401 it uses for a wrong code, and the user is told the code they just + // read off their screen is wrong. + it("refuses a short password without calling the server", async () => { + const props = setup(); + await reachCodeStep(props); + await submitCode("123456", "short"); + + expect(props.confirm).not.toHaveBeenCalled(); + expect(screen.getByRole("alert").textContent).toMatch(/at least 12 characters/i); + }); + + // The case a `.length >= 12` implementation gets wrong: 11 characters, 22 + // bytes, accepted by the backend. + it("accepts an 11-character Cyrillic password", async () => { + const props = setup(); + await reachCodeStep(props); + await submitCode("123456", "паролькудли"); + + await waitFor(() => expect(props.confirm).toHaveBeenCalled()); + }); +}); + +describe("the local attempt budget", () => { + // The literal is pinned beside the constant's definition, in + // `use-code-timers.test.tsx`, which also pins `CODE_TTL_SECONDS`. The loops + // below use the constant, so that assertion is what stops this suite being + // self-fulfilling. + + it("does not give up before the budget is spent", async () => { + const props = setup({ confirm: vi.fn(async () => ({ error: "password_reset_failed" })) }); + await reachCodeStep(props); + + // Four failures, hard-coded rather than derived: an off-by-one that let the + // binding survive one submit too long would otherwise pass. + for (let i = 0; i < 4; i++) { + await submitCode("000000", LONG_ENOUGH); + await waitFor(() => expect(props.confirm).toHaveBeenCalledTimes(i + 1)); + } + + expect(props.abandon).not.toHaveBeenCalled(); + expect(screen.getByLabelText("Enter the 6-digit code")).toBeTruthy(); + }); + + // AC-13. The backend collapses "wrong code", "expired" and "attempts + // exhausted" into one answer, so the client cannot learn from a response that + // the budget is gone. Without a local count the binding survives its full TTL + // and every reload drops the user back onto a dead code. + it("returns to step one once the budget is spent, discarding the binding", async () => { + const props = setup({ confirm: vi.fn(async () => ({ error: "password_reset_failed" })) }); + await reachCodeStep(props); + + for (let i = 0; i < MAX_CODE_ATTEMPTS; i++) { + await submitCode("000000", LONG_ENOUGH); + await waitFor(() => expect(props.confirm).toHaveBeenCalledTimes(i + 1)); + } + + await waitFor(() => expect(props.abandon).toHaveBeenCalled()); + expect(screen.getByLabelText("Reset your password")).toBeTruthy(); + }); + + // The message has to survive the trip back to step one: `restart()` clears + // every other field, and an error set separately afterwards would be one + // edit away from being wiped by the obvious `setError(undefined)`. + it("explains why it returned to step one", async () => { + const props = setup({ confirm: vi.fn(async () => ({ error: "password_reset_failed" })) }); + await reachCodeStep(props); + + for (let i = 0; i < MAX_CODE_ATTEMPTS; i++) { + await submitCode("000000", LONG_ENOUGH); + await waitFor(() => expect(props.confirm).toHaveBeenCalledTimes(i + 1)); + } + + await waitFor(() => expect(screen.getByLabelText("Reset your password")).toBeTruthy()); + expect(screen.getByRole("alert")).toBeTruthy(); + }); + + it("does not spend the budget on a locally rejected password", async () => { + const props = setup(); + await reachCodeStep(props); + + for (let i = 0; i < MAX_CODE_ATTEMPTS + 2; i++) { + await submitCode("123456", "short"); + } + + // Nothing was sent, so nothing was spent and the binding is still good. + expect(props.confirm).not.toHaveBeenCalled(); + expect(props.abandon).not.toHaveBeenCalled(); + expect(screen.getByLabelText("Enter the 6-digit code")).toBeTruthy(); + }); +}); + +describe("the double-submit guard", () => { + // Each stray submit spends one of five attempts and the backend floors every + // response to ~300 ms, so a double-click is a live risk, not a theoretical + // one. + it("sends one request when the button is clicked twice", async () => { + let release: (value: { done: true }) => void = () => {}; + const confirm = vi.fn( + () => + new Promise<{ done: true }>((resolve) => { + release = resolve; + }), + ); + const props = setup({ confirm }); + await reachCodeStep(props); + + fireEvent.change(screen.getByLabelText("Enter the 6-digit code"), { target: { value: "123456" } }); + fireEvent.change(screen.getByLabelText("New password"), { target: { value: LONG_ENOUGH } }); + const button = screen.getByRole("button", { name: "Set new password" }); + fireEvent.click(button); + fireEvent.click(button); + + expect(confirm).toHaveBeenCalledTimes(1); + release({ done: true }); + await waitFor(() => expect(props.onDone).toHaveBeenCalled()); + }); +}); + +describe("failures the user must be able to tell apart", () => { + it("sends the user back to step one when the binding is gone", async () => { + const props = setup({ + confirm: vi.fn(async () => ({ error: "password_reset_session_mismatch" })), + }); + await reachCodeStep(props); + await submitCode("123456", LONG_ENOUGH); + + await waitFor(() => expect(props.abandon).toHaveBeenCalled()); + expect(screen.getByLabelText("Reset your password")).toBeTruthy(); + }); + + // An outage is a fact about the service. Folding it into the wrong-code copy + // tells every user their input was wrong while nothing of theirs was. + it("says the service is unavailable rather than blaming the code", async () => { + const props = setup({ confirm: vi.fn(async () => ({ error: "password_reset_unavailable" })) }); + await reachCodeStep(props); + await submitCode("123456", LONG_ENOUGH); + + await waitFor(() => { + expect(screen.getByRole("alert").textContent).toMatch(/unavailable right now/i); + }); + // Still on step two: nothing about the code was wrong, so the user keeps it. + expect(screen.getByLabelText("Enter the 6-digit code")).toBeTruthy(); + }); + + it("keeps the user on step two after a wrong code, so attempts are not wasted", async () => { + const props = setup({ confirm: vi.fn(async () => ({ error: "password_reset_failed" })) }); + await reachCodeStep(props); + await submitCode("000000", LONG_ENOUGH); + + await waitFor(() => expect(props.confirm).toHaveBeenCalled()); + expect(props.abandon).not.toHaveBeenCalled(); + expect(screen.getByLabelText("Enter the 6-digit code")).toBeTruthy(); + }); +}); + +describe("expiry and the resend cooldown", () => { + // Ported from the sign-in flow's suite. Both flows moved onto the shared + // `useCodeTimers`, but only the sign-in side had behavioural coverage of it — + // so a mutation disabling expiry or the cooldown stayed green here while + // failing there. + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("counts down from five minutes", async () => { + const props = setup(); + await reachCodeStep(props); + + expect(screen.getByRole("timer").textContent).toContain("5:00"); + }); + + it("disables the fields and drops the submit button once expired", async () => { + const props = setup(); + await reachCodeStep(props); + + await vi.advanceTimersByTimeAsync(300_000); + + await waitFor(() => + expect((screen.getByLabelText("Enter the 6-digit code") as HTMLInputElement).disabled).toBe(true), + ); + expect((screen.getByLabelText("New password") as HTMLInputElement).disabled).toBe(true); + expect(screen.queryByRole("button", { name: "Set new password" })).toBeNull(); + expect(screen.getByRole("alert").textContent).toMatch(/expired/i); + }); + + it("refuses a submit on an expired code", async () => { + const props = setup(); + await reachCodeStep(props); + fireEvent.change(screen.getByLabelText("Enter the 6-digit code"), { + target: { value: "123456" }, + }); + fireEvent.change(screen.getByLabelText("New password"), { target: { value: LONG_ENOUGH } }); + + await vi.advanceTimersByTimeAsync(300_000); + // The button is gone, so submit the form directly — a stray Enter keypress + // reaches the handler even when the control does not. + fireEvent.submit(screen.getByLabelText("Enter the 6-digit code").closest("form")!); + + expect(props.confirm).not.toHaveBeenCalled(); + }); + + it("blocks resend during the cooldown, then allows it", async () => { + const props = setup(); + await reachCodeStep(props); + + const resend = () => screen.getByRole("button", { name: /Request a new code/ }); + expect(resend().hasAttribute("disabled")).toBe(true); + + await vi.advanceTimersByTimeAsync(30_000); + + await waitFor(() => expect(resend().hasAttribute("disabled")).toBe(false)); + }); + + it("lets an expired code be replaced even before the cooldown ends", async () => { + // Expiry must not trap the user: with no valid code left, the only useful + // control has to stay live. + const props = setup(); + await reachCodeStep(props); + + await vi.advanceTimersByTimeAsync(300_000); + + await waitFor(() => + expect(screen.getByRole("button", { name: /Request a new code/ }).hasAttribute("disabled")).toBe(false), + ); + }); + + it("restarts the cooldown when a request fails, rather than leaving resend hot", async () => { + const props = setup({ requestCode: vi.fn(async () => ({ error: "otp_rate_limited" })) }); + + fireEvent.change(screen.getByLabelText("Reset your password"), { + target: { value: "op@example.test" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Email me a code" })); + await waitFor(() => expect(props.requestCode).toHaveBeenCalled()); + + // Still on step one — the request failed — and a 429 answered by immediate + // retries is what caused it, so the button must not be hot. + expect(screen.getByLabelText("Reset your password")).toBeTruthy(); + }); +}); + +describe("leaving the flow", () => { + // Backing out and resetting a DIFFERENT address must not carry the previous + // password forward: the user would submit for that account a secret they + // never knowingly re-entered. + it("clears the typed password when returning to step one", async () => { + const props = setup({ + confirm: vi.fn(async () => ({ error: "password_reset_session_mismatch" })), + }); + await reachCodeStep(props); + await submitCode("123456", LONG_ENOUGH); + + await waitFor(() => expect(screen.getByLabelText("Reset your password")).toBeTruthy()); + + // Back to step two for another address; the field must be empty. + fireEvent.change(screen.getByLabelText("Reset your password"), { + target: { value: "other@example.test" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Email me a code" })); + await screen.findByLabelText("New password"); + + expect((screen.getByLabelText("New password") as HTMLInputElement).value).toBe(""); + }); +}); + +describe("what a screen reader is told", () => { + // The alert fires once when the error appears. Returning focus to the field + // afterwards must still explain why it was rejected, which is what the + // description association is for — the sign-in flow does the same. + it("points the code field at the error, and at the countdown otherwise", async () => { + const props = setup({ confirm: vi.fn(async () => ({ error: "password_reset_failed" })) }); + await reachCodeStep(props); + + expect(screen.getByLabelText("Enter the 6-digit code").getAttribute("aria-describedby")).toBe( + "reset-countdown", + ); + + await submitCode("000000", LONG_ENOUGH); + + await waitFor(() => { + expect(screen.getByLabelText("Enter the 6-digit code").getAttribute("aria-describedby")).toBe( + "reset-error", + ); + }); + }); + + it("points the password field at the error when the error is about the password", async () => { + const props = setup(); + await reachCodeStep(props); + + // Keeps the hint too: the requirement is still what the user needs to hear. + await submitCode("123456", "short"); + + expect(screen.getByLabelText("New password").getAttribute("aria-describedby")).toBe( + "reset-error reset-password-hint", + ); + }); +}); + +describe("rehydration after a reload", () => { + // The flow spans an email round-trip, so the user leaves the tab. The server + // page reads the cookie and hands down the step; the nonce never crosses. + it("resumes at step two with the bound address", () => { + render( + ({}))} + confirm={vi.fn(async () => ({ done: true }))} + abandon={vi.fn(async () => {})} + onDone={vi.fn()} + onCancel={vi.fn()} + />, + ); + + expect(screen.getByLabelText("Enter the 6-digit code")).toBeTruthy(); + expect(screen.getByText(/Sent to op@example.test/)).toBeTruthy(); + }); +}); + +describe("the destructive consequence is stated before the submit", () => { + it("warns on both steps that this signs the user out everywhere", async () => { + const props = setup(); + expect(screen.getByText(/signs you out everywhere/i)).toBeTruthy(); + + await reachCodeStep(props); + expect(screen.getByText(/signs you out of every device/i)).toBeTruthy(); + }); +}); diff --git a/src/features/auth/__tests__/use-code-timers.test.tsx b/src/features/auth/__tests__/use-code-timers.test.tsx new file mode 100644 index 0000000..870c7ed --- /dev/null +++ b/src/features/auth/__tests__/use-code-timers.test.tsx @@ -0,0 +1,99 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { act, cleanup, render, screen } from "@testing-library/react"; + +import { CODE_TTL_SECONDS, MAX_CODE_ATTEMPTS, useCodeTimers } from "@/features/auth/use-code-timers"; + +afterEach(() => { + cleanup(); + vi.useRealTimers(); +}); + +function Probe({ active }: { active: boolean }) { + const timers = useCodeTimers(active); + return ( +
+ {timers.remaining} + +
+ ); +} + +function remaining() { + return Number(screen.getByTestId("remaining").textContent); +} + +describe("the countdown is derived from a deadline, not decremented", () => { + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: false }); + vi.setSystemTime(new Date("2026-09-07T12:00:00Z")); + }); + + // The case a decrementing counter gets wrong. A backgrounded tab, a laptop + // waking from sleep and a bfcache restore all suspend the interval; a counter + // then resumes from where it stopped and OVER-reports the time left, showing + // "expires in 4:12" for a code the backend has already discarded. + it("catches up after the tab is suspended, rather than resuming where it paused", () => { + render(); + act(() => { + screen.getByRole("button", { name: "start" }).click(); + }); + expect(remaining()).toBe(CODE_TTL_SECONDS); + + // The clock jumps two minutes while the interval fires only once — a + // throttled or suspended tab. A decrementing counter would read 299 here, + // because it only ever subtracts what it observed. + act(() => { + vi.setSystemTime(new Date("2026-09-07T12:02:00Z")); + vi.advanceTimersByTime(1000); + }); + + // 120s of wall clock plus the 1s the tick itself advanced. + expect(remaining()).toBe(CODE_TTL_SECONDS - 121); + expect(remaining()).toBeLessThan(CODE_TTL_SECONDS - 100); + }); + + it("floors at zero rather than going negative", () => { + render(); + act(() => { + screen.getByRole("button", { name: "start" }).click(); + }); + + act(() => { + vi.setSystemTime(new Date("2026-09-07T12:10:00Z")); + vi.advanceTimersByTime(1000); + }); + + expect(remaining()).toBe(0); + }); + + it("stops ticking once the step is left", () => { + const { rerender } = render(); + act(() => { + screen.getByRole("button", { name: "start" }).click(); + }); + + rerender(); + const frozen = remaining(); + + act(() => { + vi.setSystemTime(new Date("2026-09-07T12:00:30Z")); + vi.advanceTimersByTime(5000); + }); + + // No interval is running, so nothing recomputes: the value is whatever the + // last active tick left, and `active` gating is what tears it down. + expect(remaining()).toBe(frozen); + }); +}); + +describe("the contract constants", () => { + // Pinned as literals. Every other suite derives its loops from these, so + // without this the whole budget story is self-fulfilling. + it("match the backend's configuration", () => { + expect(CODE_TTL_SECONDS).toBe(300); + expect(MAX_CODE_ATTEMPTS).toBe(5); + }); +}); diff --git a/src/features/auth/login-page.tsx b/src/features/auth/login-page.tsx index 1a0f9c3..5365a05 100644 --- a/src/features/auth/login-page.tsx +++ b/src/features/auth/login-page.tsx @@ -1,6 +1,7 @@ "use client"; import { AlertTriangle, ChevronRight } from "lucide-react"; +import { useState } from "react"; import { Button } from "@/shared/ui/shadcn/button"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/shared/ui/shadcn/tooltip"; @@ -8,6 +9,13 @@ import { BrandIcon, MaintMark, type BrandProvider } from "@/shared/ui/icons/bran import type { SignInMethod } from "@/domain/auth/sign-in-method"; import { OtpSignInFlow } from "@/features/auth/otp-sign-in-flow"; import { PasswordSignInForm } from "@/features/auth/password-sign-in-form"; +// Statically imported, deliberately. It looks like a candidate for `dynamic()` +// — it is only reachable behind a click — but it can also be the FIRST thing +// this page paints: `resetInProgressEmail` resumes the flow after the user +// comes back from their email client, and a lazy chunk would turn that +// re-entry into two sequential round-trips. Its own weight is a few KB; every +// primitive it uses is already on this route. +import { PasswordResetFlow } from "@/features/auth/password-reset-flow"; export interface LoginPageProps { error?: string; @@ -32,6 +40,26 @@ export interface LoginPageProps { passwordSignInAction: (email: string, password: string) => Promise<{ error?: string }>; /** Abandons the current OTP flow so another address can be used. */ changeEmailAction: () => Promise; + /** Step one of the password reset (RUK-289): mails a code, binds this browser. */ + requestPasswordResetAction: (email: string) => Promise<{ error?: string }>; + /** Step two: redeems the code, installs the password, ends every session. */ + confirmPasswordResetAction: (args: { + email: string; + code: string; + newPassword: string; + }) => Promise<{ error?: string; done?: boolean }>; + /** Abandons the reset flow, discarding its binding. */ + abandonPasswordResetAction: () => Promise; + /** + * Rehydrated from the reset cookie by the server page. A reset spans an email + * round-trip, so the user WILL leave the tab and come back; without this the + * component remounts at step one while a live binding sits on the server. + * + * Only the address crosses, never the nonce: the cookie is httpOnly so that + * browser JavaScript cannot read the binding, and passing the whole thing + * down would undo exactly that. + */ + resetInProgressEmail?: string; } /** @@ -72,10 +100,28 @@ const BREAK_GLASS_METHODS: SignInMethod[] = [ /** Disabled providers are gated until backend support ships. */ const COMING_SOON_TOOLTIP = "Coming soon — additional providers are on the way"; -export function LoginPage({ error, methods, signInAction, ...actions }: LoginPageProps) { +export function LoginPage({ + error, + methods, + signInAction, + requestPasswordResetAction, + confirmPasswordResetAction, + abandonPasswordResetAction, + resetInProgressEmail, + ...actions +}: LoginPageProps) { const resolvedFailed = methods === undefined; const builtIn = (resolvedFailed ? BREAK_GLASS_METHODS : methods).filter((m) => !OAUTH_IDS.has(m.id)); + const offersPassword = builtIn.some((m) => m.type === "password"); + // A live binding only rehydrates if this page is still drawing the form the + // affordance lives in. An operator who toggled password sign-in off between + // the code being sent and the tab being reloaded gets the normal page: the + // advertised method list is the authority on what is offered, and a cookie + // must not resurrect a withdrawn one. + const [resetting, setResetting] = useState(Boolean(resetInProgressEmail) && offersPassword); + const [resetDone, setResetDone] = useState(false); + return (
@@ -101,28 +147,63 @@ export function LoginPage({ error, methods, signInAction, ...actions }: LoginPag ) : null} -
- {OAUTH_PROVIDERS.map((p) => - p.enabled ? ( -
- +
+ ) : ( + {p.label} -
+ + ), + )} + + {builtIn.map((method) => ( + { + setResetDone(false); + setResetting(true); + }} + {...actions} + /> + ))} + + )} {resolvedFailed ? (

@@ -153,11 +234,16 @@ function BuiltInMethod({ otpSignInAction, passwordSignInAction, changeEmailAction, -}: BuiltInMethodActions & { method: SignInMethod }) { + onForgotPassword, +}: BuiltInMethodActions & { method: SignInMethod; onForgotPassword: () => void }) { if (method.type === "password") { return (

- +
); } diff --git a/src/features/auth/otp-sign-in-flow.tsx b/src/features/auth/otp-sign-in-flow.tsx index 7f07e4a..a9adf6c 100644 --- a/src/features/auth/otp-sign-in-flow.tsx +++ b/src/features/auth/otp-sign-in-flow.tsx @@ -1,11 +1,12 @@ "use client"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useState } from "react"; import { Button } from "@/shared/ui/shadcn/button"; import { Input } from "@/shared/ui/shadcn/input"; import { Label } from "@/shared/ui/shadcn/label"; import { isWellFormedOtpCode } from "@/domain/auth/sign-in-method"; +import { useCodeTimers } from "@/features/auth/use-code-timers"; /** * Two-step email one-time-code sign-in (RUK-288). @@ -16,16 +17,6 @@ import { isWellFormedOtpCode } from "@/domain/auth/sign-in-method"; * `check-bundle-budget.mjs` guards against. */ -/** Backend `otp_ttl`. The real expiry lives server-side and is never returned. */ -const CODE_TTL_SECONDS = 300; -/** - * Advisory only. The backend has no resend cooldown, and its per-IP bucket is - * shared with the password and OAuth endpoints — so an unthrottled resend - * button would lock the user out of the *other* ways in. The server's 429 is - * the real backstop. - */ -const RESEND_COOLDOWN_SECONDS = 30; - type Step = "email" | "code"; export interface OtpSignInFlowProps { @@ -41,21 +32,12 @@ export function OtpSignInFlow({ label, requestCode, submitCode, onChangeEmail }: const [code, setCode] = useState(""); const [error, setError] = useState(); const [pending, setPending] = useState(false); - const [remaining, setRemaining] = useState(0); - const [cooldown, setCooldown] = useState(0); - - // One interval drives both counters. Started on entry to step two and torn - // down on leaving it, so a backgrounded tab cannot leave a timer running. - useEffect(() => { - if (step !== "code") return; - const id = setInterval(() => { - setRemaining((r) => (r > 0 ? r - 1 : 0)); - setCooldown((c) => (c > 0 ? c - 1 : 0)); - }, 1000); - return () => clearInterval(id); - }, [step]); - - const expired = step === "code" && remaining === 0; + + // Shared with the password-reset flow (RUK-289): the TTL and the attempt + // budget are contract facts about the same backend mechanism, and two copies + // would drift. + const timers = useCodeTimers(step === "code"); + const { remaining, cooldown, expired } = timers; const send = useCallback( async (address: string) => { @@ -66,41 +48,32 @@ export function OtpSignInFlow({ label, requestCode, submitCode, onChangeEmail }: if (result.error) { setError(result.error); - // A failed request starts a fresh cooldown rather than leaving the - // button hot: a 429 answered by immediate retries is what caused it. - setCooldown(RESEND_COOLDOWN_SECONDS); + timers.startCooldown(); return; } - // Counted from response receipt, so the client is always slightly - // optimistic relative to the server. That is the safe direction: the - // backend, not this timer, decides whether a code is still valid. - setRemaining(CODE_TTL_SECONDS); - setCooldown(RESEND_COOLDOWN_SECONDS); + timers.start(); setCode(""); setStep("code"); }, - [requestCode], + [requestCode, timers], ); - const inFlight = useRef(false); - async function onSubmitCode(event: React.FormEvent) { event.preventDefault(); - // Each stray submit spends one of five attempts, and the backend floors - // every response to ~300ms, so a double-click is a live risk. - if (inFlight.current || pending || expired) return; + if (pending || expired) return; if (!isWellFormedOtpCode(code)) { setError("invalid_code_format"); return; } - inFlight.current = true; - setPending(true); - setError(undefined); - const result = await submitCode(email, code.trim()); - inFlight.current = false; - setPending(false); - if (!result.error) return; + const result = await timers.guard(async () => { + setPending(true); + setError(undefined); + const outcome = await submitCode(email, code.trim()); + setPending(false); + return outcome; + }); + if (!result || !result.error) return; if (result.error === "otp_session_mismatch") { // The binding is gone — the server has already cleared it — so step two @@ -110,8 +83,7 @@ export function OtpSignInFlow({ label, requestCode, submitCode, onChangeEmail }: // is the primary action and nothing is throttled. setStep("email"); setCode(""); - setRemaining(0); - setCooldown(0); + timers.reset(); } setError(result.error); } @@ -121,7 +93,7 @@ export function OtpSignInFlow({ label, requestCode, submitCode, onChangeEmail }: setStep("email"); setCode(""); setError(undefined); - setRemaining(0); + timers.reset(); } if (step === "email") { diff --git a/src/features/auth/password-reset-flow.tsx b/src/features/auth/password-reset-flow.tsx new file mode 100644 index 0000000..82f75a0 --- /dev/null +++ b/src/features/auth/password-reset-flow.tsx @@ -0,0 +1,320 @@ +"use client"; + +import { useEffect, useState } from "react"; + +import { Button } from "@/shared/ui/shadcn/button"; +import { Input } from "@/shared/ui/shadcn/input"; +import { Label } from "@/shared/ui/shadcn/label"; +import { isWellFormedOtpCode, isPasswordWithinPolicy } from "@/domain/auth/sign-in-method"; +import { flowErrorMessage } from "@/features/auth/otp-sign-in-flow"; +import { MAX_CODE_ATTEMPTS, useCodeTimers } from "@/features/auth/use-code-timers"; + +/** + * "Forgot password" — a two-step emailed-code flow that ends in a new password + * (RUK-289). + * + * A sibling of `OtpSignInFlow` rather than a mode of it. That component's steps + * end in a session (its success path is a redirect thrown by NextAuth, so there + * is no success state to render), while this one ends signed OUT with something + * to say. Sharing the shell would mean a prop deciding which of two endpoints, + * which of two cookies and which of two terminal states applies — the timers + * are what the two genuinely share, and those are extracted into a hook. + */ + +type Step = "email" | "code"; + +export interface PasswordResetFlowProps { + /** Rehydrated from the reset cookie by the server page, after a reload. */ + initialEmail?: string; + initialStep?: Step; + requestCode: (email: string) => Promise<{ error?: string }>; + confirm: (args: { + email: string; + code: string; + newPassword: string; + }) => Promise<{ error?: string; done?: boolean }>; + abandon: () => Promise; + /** Returns to the sign-in form; the confirmation is owned by the page. */ + onDone: () => void; + onCancel: () => void; +} + +export function PasswordResetFlow({ + initialEmail, + initialStep, + requestCode, + confirm, + abandon, + onDone, + onCancel, +}: PasswordResetFlowProps) { + const [step, setStep] = useState(initialStep ?? "email"); + const [email, setEmail] = useState(initialEmail ?? ""); + const [code, setCode] = useState(""); + const [password, setPassword] = useState(""); + const [error, setError] = useState(); + const [pending, setPending] = useState(false); + // Counts THIS browser's submits against the backend's per-code budget. The + // backend collapses "wrong code", "expired" and "attempts exhausted" into one + // indistinguishable answer, so without a local count a user who burns every + // attempt keeps a live cookie for its full TTL and every reload drops them + // back onto a permanently dead code. + const [attempts, setAttempts] = useState(0); + + const timers = useCodeTimers(step === "code"); + const budgetSpent = attempts >= MAX_CODE_ATTEMPTS; + + // Rehydrating straight into step two starts with the countdown at zero, which + // reads as "expired" and hides the submit button — so a user returning from + // their email finds a form they cannot send. The real expiry is server-side + // and never returned; this is the same optimistic local clock the flow uses + // after a fresh request, and the backend remains the authority. + useEffect(() => { + if (initialStep === "code") timers.start(); + // Once, on mount: `start` is stable and re-running it would reset the + // countdown under a user who is mid-flow. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + async function send(address: string) { + setPending(true); + setError(undefined); + const result = await requestCode(address); + setPending(false); + + if (result.error) { + setError(result.error); + timers.startCooldown(); + return; + } + timers.start(); + setCode(""); + setAttempts(0); + setStep("code"); + } + + /** + * Leaves step two for step one, discarding the binding server-side. + * + * Takes the message to show rather than leaving the caller to set it + * afterwards: the caller's `setError` used to land in a later batch and + * survive only because nothing here touched `error`. Adding the obvious + * `setError(undefined)` to this function — it clears every other field — + * would have silently deleted the only explanation the user gets. + */ + async function restart(message?: string) { + await abandon(); + timers.reset(); + setStep("email"); + setCode(""); + // Cleared with the code, not left behind. Without this, someone who backs + // out and resets a DIFFERENT address finds the previous password already + // filled in, and submits for that account a secret they never knowingly + // re-entered — besides holding a plaintext password in state longer than + // the flow has any use for it. + setPassword(""); + setAttempts(0); + setError(message); + } + + async function onSubmitCode(event: React.FormEvent) { + event.preventDefault(); + if (pending || timers.expired || budgetSpent) return; + + if (!isWellFormedOtpCode(code)) { + setError("invalid_code_format"); + return; + } + // Checked here as well as in the action so the user is told which field is + // wrong before a round-trip. The action is the authority; this is the + // message. + if (!isPasswordWithinPolicy(password)) { + setError("password_policy_violation"); + return; + } + + await timers.guard(async () => { + setPending(true); + setError(undefined); + const result = await confirm({ email, code: code.trim(), newPassword: password }); + setPending(false); + + if (result.done) { + onDone(); + return; + } + + // Counted for every answer the server gave, including a mismatch: the + // backend claims an attempt BEFORE it compares the code, so a stale nonce + // costs one exactly as a wrong digit does. + const spent = attempts + 1; + setAttempts(spent); + + if (result.error === "password_reset_session_mismatch" || spent >= MAX_CODE_ATTEMPTS) { + // Either the binding is already gone server-side, or the attempts are + // spent — both make step two a dead end, and leaving a live cookie + // behind would rehydrate the user onto a code that can never be + // redeemed. + await restart(result.error); + return; + } + setError(result.error); + }); + } + + if (step === "email") { + return ( +
{ + e.preventDefault(); + const trimmed = email.trim(); + if (!trimmed || pending) return; + void send(trimmed); + }} + > + +

+ We'll email you a code if that address has an account. Setting a new password signs you out + everywhere. +

+ setEmail(e.target.value)} + aria-describedby={error ? "reset-error" : undefined} + /> + {error ? : null} + + + + ); + } + + // Only expiry can be true here. Spending the budget is always followed by + // `restart()` in the same React batch, so the flow leaves this step before a + // `budgetSpent` render can happen — the submit guard above still checks it, + // because a guard that is cheap and states the intent is worth keeping, but + // the rendering below would be dead code. + const dead = timers.expired; + // A dead code makes resend the only way forward, so the cooldown is waived + // rather than made to run out first. + const throttled = timers.cooldown > 0 && !dead; + + return ( +
+ +

+ Sent to {email}.{" "} + +

+ setCode(e.target.value.replace(/\D/g, ""))} + // Points at whichever of the two actually renders below, matching the + // sign-in flow: the alert fires once, and a screen-reader user who + // returns focus here afterwards would otherwise hear nothing saying why + // the code was rejected. + aria-describedby={error || dead ? "reset-error" : "reset-countdown"} + /> + + setPassword(e.target.value)} + aria-describedby={ + error === "password_policy_violation" ? "reset-error reset-password-hint" : "reset-password-hint" + } + /> + {/* + * "Characters" rather than bytes: the policy is 12 BYTES, which is not a + * unit to put in front of an operator. The hint is the ASCII worst case, + * so it can only ever under-promise — an 11-character Cyrillic password + * is 22 bytes and is accepted. Erring that way shows a user their + * password was taken when the hint implied otherwise, never the reverse. + */} +

+ At least 12 characters. This signs you out of every device. +

+ {dead ? ( + + ) : error ? ( + + ) : ( +

+ Expires in {formatRemaining(timers.remaining)} +

+ )} + {!dead ? ( + + ) : null} + + + ); +} + +function formatRemaining(seconds: number): string { + const m = Math.floor(seconds / 60); + const s = seconds % 60; + return `${m}:${String(s).padStart(2, "0")}`; +} + +function ResetError({ code }: { code: string }) { + return ( + + ); +} + +/** + * Reset-specific copy, falling through to the shared map for the codes both + * flows raise. Kept separate where the wording has to differ: sign-in's + * mismatch copy tells the user to go back and sign in, which is not where + * someone mid-reset is trying to go. + */ +export function resetErrorMessage(code: string): string { + switch (code) { + case "password_reset_session_mismatch": + return "This code can't be checked in this browser. Request a new one to continue."; + case "password_reset_failed": + return "That code isn't valid, or it has been used too many times. Request a new one."; + case "password_policy_violation": + return "Choose a longer password — at least 12 characters."; + case "password_reset_unavailable": + // A fact about the service, not about the account or the code. Saying so + // stops an outage from reading as "you typed something wrong". + return "Password reset is unavailable right now. Try again shortly."; + default: + return flowErrorMessage(code); + } +} diff --git a/src/features/auth/password-sign-in-form.tsx b/src/features/auth/password-sign-in-form.tsx index 2ef421b..0540a79 100644 --- a/src/features/auth/password-sign-in-form.tsx +++ b/src/features/auth/password-sign-in-form.tsx @@ -15,17 +15,19 @@ import { flowErrorMessage } from "@/features/auth/otp-sign-in-flow"; * `email_password`; the backend decides which internally, so this form does not * change when the second arrives. * - * Only the sign-in form lives here. Forced change, set-password-by-invite, - * forgot-password and change-in-profile are deliberately out of scope — those - * endpoints do not exist yet and their screens ship with them. + * The "Forgot password?" affordance lives INSIDE this form (RUK-289), so it + * appears and disappears with the password method itself: an instance that is + * not advertising password sign-in must not offer to reset one. */ export interface PasswordSignInFormProps { label: string; submit: (email: string, password: string) => Promise<{ error?: string }>; + /** Opens the reset flow. Absent when the deployment has no reset endpoint. */ + onForgotPassword?: () => void; } -export function PasswordSignInForm({ label, submit }: PasswordSignInFormProps) { +export function PasswordSignInForm({ label, submit, onForgotPassword }: PasswordSignInFormProps) { const [email, setEmail] = useState(""); const [password, setPassword] = useState(""); const [error, setError] = useState(); @@ -77,6 +79,11 @@ export function PasswordSignInForm({ label, submit }: PasswordSignInFormProps) { + {onForgotPassword ? ( + + ) : null} ); } diff --git a/src/features/auth/use-code-timers.ts b/src/features/auth/use-code-timers.ts new file mode 100644 index 0000000..61559ab --- /dev/null +++ b/src/features/auth/use-code-timers.ts @@ -0,0 +1,140 @@ +"use client"; + +import { useCallback, useEffect, useRef, useState } from "react"; + +/** + * The countdown, resend cooldown and double-submit guard shared by the two + * emailed-code flows: sign-in (RUK-288) and password reset (RUK-289). + * + * Extracted rather than duplicated because the numbers are contract facts, not + * styling: `CODE_TTL_SECONDS` equals the backend's `otp_ttl`, and both flows + * spend attempts from the same per-user budget. Two copies would drift, and the + * copy that drifted would tell a user their live code had expired. + */ + +/** Backend `otp_ttl`. The real expiry lives server-side and is never returned. */ +export const CODE_TTL_SECONDS = 300; + +/** + * Advisory only. The backend has no resend cooldown, and its per-IP bucket is + * shared with the password and OAuth endpoints — so an unthrottled resend + * button would lock the user out of the *other* ways in. The server's 429 is + * the real backstop. + */ +export const RESEND_COOLDOWN_SECONDS = 30; + +/** + * The backend's `auth.otp_max_attempts`. It claims an attempt BEFORE comparing + * the code, so a submit with a stale nonce spends one exactly like a wrong + * digit does — which is why the caller counts every submit rather than only the + * plausible ones. + * + * Deliberately the configured value (5) and not the backend's ceiling for that + * setting (10). Erring high would hand the user five doomed submits against an + * exhausted code; erring low costs at worst one premature "request a new code", + * which is the state they were heading for anyway. + */ +export const MAX_CODE_ATTEMPTS = 5; + +export interface CodeTimers { + /** Seconds until the code is presumed dead. Zero means expired. */ + remaining: number; + /** Seconds until resend is offered again. Zero means it is available. */ + cooldown: number; + /** True once `remaining` has run out on the code step. */ + expired: boolean; + /** Starts both timers — call on a successful code request. */ + start: () => void; + /** Stops both — call when leaving the code step. */ + reset: () => void; + /** Restarts only the cooldown, for a request that failed. */ + startCooldown: () => void; + /** + * Runs `fn` unless a submit is already in flight, and reports whether it ran. + * Each stray submit spends one of the attempts and the backend floors every + * response to ~300 ms, so a double-click is a live risk rather than a + * theoretical one. + */ + guard: (fn: () => Promise) => Promise; +} + +/** Whole seconds from now until `deadline`, floored at zero. */ +function secondsUntil(deadline: number, now: number): number { + return Math.max(0, Math.ceil((deadline - now) / 1000)); +} + +export function useCodeTimers(active: boolean): CodeTimers { + // Deadlines, not counters. A decrementing counter drifts whenever the tab is + // throttled — a backgrounded tab, a laptop waking from sleep, a bfcache + // restore — and it drifts in the direction that OVER-reports the time left, + // so the user is shown "expires in 4:12" for a code the backend has already + // discarded. Deriving from a timestamp survives all three, and the displayed + // value stays optimistic only by the network delay it was always optimistic + // by. + const codeDeadline = useRef(0); + const cooldownDeadline = useRef(0); + const [remaining, setRemaining] = useState(0); + const [cooldown, setCooldown] = useState(0); + const inFlight = useRef(false); + + // One interval drives both counters, torn down when the step is left, so a + // backgrounded tab cannot leave a timer running. + useEffect(() => { + if (!active) return; + const tick = () => { + const now = Date.now(); + setRemaining(secondsUntil(codeDeadline.current, now)); + setCooldown(secondsUntil(cooldownDeadline.current, now)); + }; + const id = setInterval(tick, 1000); + // Once immediately, so a tab returning to the foreground corrects on the + // frame it wakes rather than a second later. + tick(); + return () => clearInterval(id); + }, [active]); + + const start = useCallback(() => { + // Counted from response receipt, so the client is always slightly + // optimistic relative to the server. That is the safe direction: the + // backend, not this timer, decides whether a code is still valid. + const now = Date.now(); + codeDeadline.current = now + CODE_TTL_SECONDS * 1000; + cooldownDeadline.current = now + RESEND_COOLDOWN_SECONDS * 1000; + setRemaining(CODE_TTL_SECONDS); + setCooldown(RESEND_COOLDOWN_SECONDS); + }, []); + + const reset = useCallback(() => { + codeDeadline.current = 0; + cooldownDeadline.current = 0; + setRemaining(0); + setCooldown(0); + }, []); + + const startCooldown = useCallback(() => { + // A failed request starts a fresh cooldown rather than leaving the button + // hot: a 429 answered by immediate retries is what caused it. + cooldownDeadline.current = Date.now() + RESEND_COOLDOWN_SECONDS * 1000; + setCooldown(RESEND_COOLDOWN_SECONDS); + }, []); + + const guard = useCallback(async (fn: () => Promise): Promise => { + if (inFlight.current) return undefined; + inFlight.current = true; + try { + return await fn(); + } finally { + inFlight.current = false; + } + }, []); + + return { + remaining, + cooldown, + expired: active && remaining === 0, + start, + reset, + startCooldown, + guard, + }; +} diff --git a/src/features/settings/__tests__/password-card.test.tsx b/src/features/settings/__tests__/password-card.test.tsx new file mode 100644 index 0000000..d43c77d --- /dev/null +++ b/src/features/settings/__tests__/password-card.test.tsx @@ -0,0 +1,204 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; + +import { PasswordCard } from "@/features/settings/password-card"; +import { BffError } from "@/features/_shared/api/bff-fetch"; + +const bffFetch = vi.fn(); +vi.mock("@/features/_shared/api/bff-fetch", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, bffFetch: (...args: unknown[]) => bffFetch(...args) }; +}); + +const toastSuccess = vi.fn(); +const toastError = vi.fn(); +vi.mock("sonner", () => ({ + toast: { + success: (...args: unknown[]) => toastSuccess(...args), + error: (...args: unknown[]) => toastError(...args), + }, +})); + +afterEach(() => cleanup()); +beforeEach(() => vi.clearAllMocks()); + +const LONG_ENOUGH = "a-long-enough-password"; + +function renderCard(passwordSet: boolean | undefined) { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const invalidate = vi.spyOn(client, "invalidateQueries"); + render( + + + , + ); + return { invalidate }; +} + +function fill(label: string, value: string) { + fireEvent.change(screen.getByLabelText(label), { target: { value } }); +} + +describe("which form is drawn — the three values of password_set", () => { + // AC-5. `undefined` is NOT `false`: a backend that predates the field would + // otherwise get the set-password form for every operator, and every save + // would be a 400. + it("asks for the current password when the account has one", () => { + renderCard(true); + + expect(screen.getByLabelText("Current password")).toBeTruthy(); + expect(screen.getByRole("button", { name: "Change password" })).toBeTruthy(); + }); + + it("omits the current password when the account has none", () => { + renderCard(false); + + expect(screen.queryByLabelText("Current password")).toBeNull(); + expect(screen.getByRole("button", { name: "Set password" })).toBeTruthy(); + }); + + it("offers nothing at all when the field is absent", () => { + renderCard(undefined); + + expect(screen.queryByLabelText("New password")).toBeNull(); + expect(screen.getByText(/unavailable/i)).toBeTruthy(); + }); +}); + +describe("the length check", () => { + it("refuses a short password without calling the server", () => { + renderCard(false); + fill("New password", "short"); + fireEvent.click(screen.getByRole("button", { name: "Set password" })); + + expect(bffFetch).not.toHaveBeenCalled(); + expect(screen.getByRole("alert").textContent).toMatch(/at least 12 characters/i); + }); + + // The byte arithmetic itself is pinned once, in the domain policy test. What + // matters here is only that the guard runs before the request, which the + // short-password case above already proves. +}); + +describe("what is sent", () => { + it("omits current_password entirely on the set form", async () => { + bffFetch.mockResolvedValue(undefined); + renderCard(false); + fill("New password", LONG_ENOUGH); + fireEvent.click(screen.getByRole("button", { name: "Set password" })); + + await waitFor(() => expect(bffFetch).toHaveBeenCalled()); + const body = JSON.parse((bffFetch.mock.calls[0][1] as { body: string }).body); + expect(body).toEqual({ new_password: LONG_ENOUGH }); + expect("current_password" in body).toBe(false); + }); + + it("sends both fields on the change form", async () => { + bffFetch.mockResolvedValue(undefined); + renderCard(true); + fill("Current password", "old-password"); + fill("New password", LONG_ENOUGH); + fireEvent.click(screen.getByRole("button", { name: "Change password" })); + + await waitFor(() => expect(bffFetch).toHaveBeenCalled()); + const body = JSON.parse((bffFetch.mock.calls[0][1] as { body: string }).body); + expect(body).toEqual({ current_password: "old-password", new_password: LONG_ENOUGH }); + }); +}); + +describe("after a successful save", () => { + // AC-14. Both endpoints answer 204 with no body, so the cache cannot be + // seeded from the response — and `useMeQuery` holds its answer for 60s, which + // would leave the card drawing the form for the state the user just left. + it("invalidates the me query so password_set is refetched", async () => { + bffFetch.mockResolvedValue(undefined); + const { invalidate } = renderCard(false); + fill("New password", LONG_ENOUGH); + fireEvent.click(screen.getByRole("button", { name: "Set password" })); + + await waitFor(() => { + expect(invalidate).toHaveBeenCalledWith(expect.objectContaining({ queryKey: ["me"] })); + }); + }); +}); + +describe("failures the user must be able to act on", () => { + it("shows a wrong current password in place, without a redirect", async () => { + bffFetch.mockRejectedValue(new BffError(422, "That current password isn't right")); + renderCard(true); + fill("Current password", "wrong"); + fill("New password", LONG_ENOUGH); + fireEvent.click(screen.getByRole("button", { name: "Change password" })); + + await waitFor(() => { + expect(screen.getByRole("alert").textContent).toMatch(/current password isn't right/i); + }); + }); + + it("says a stale session changed nothing", async () => { + bffFetch.mockRejectedValue(new BffError(409, "session expired")); + renderCard(false); + fill("New password", LONG_ENOUGH); + fireEvent.click(screen.getByRole("button", { name: "Set password" })); + + await waitFor(() => { + expect(toastError).toHaveBeenCalledWith(expect.stringMatching(/session expired/i)); + }); + }); +}); + +describe("recovering from a wrong password_set — one flip, never a loop", () => { + // AC-8. `password_set` degrades to `false` when the backend's own read fails, + // so the card can draw the wrong form. The 400 that follows must lead + // somewhere. + it("flips to the change form when the account turns out to have a password", async () => { + bffFetch.mockRejectedValue(new BffError(400, "validation error: the current password is required")); + renderCard(false); + fill("New password", LONG_ENOUGH); + fireEvent.click(screen.getByRole("button", { name: "Set password" })); + + await waitFor(() => expect(screen.getByLabelText("Current password")).toBeTruthy()); + expect(screen.getByRole("alert").textContent).toMatch(/already has a password/i); + }); + + // The termination proof. A 400 is not evidence about `password_set` — the + // same status carries a length failure, and all three of the backend's 400s + // share one code — so an unbounded rule would oscillate set → change → set + // forever. + it("does not flip a second time", async () => { + bffFetch.mockRejectedValue(new BffError(400, "validation error")); + renderCard(false); + + fill("New password", LONG_ENOUGH); + fireEvent.click(screen.getByRole("button", { name: "Set password" })); + await waitFor(() => expect(screen.getByLabelText("Current password")).toBeTruthy()); + + fill("Current password", "something"); + fill("New password", LONG_ENOUGH); + fireEvent.click(screen.getByRole("button", { name: "Change password" })); + + await waitFor(() => expect(bffFetch).toHaveBeenCalledTimes(2)); + // Still the change form: the second 400 is a terminal error, not a flip + // back to where it started — and the error is SHOWN, not swallowed. + expect(screen.getByLabelText("Current password")).toBeTruthy(); + expect(screen.getByRole("alert")).toBeTruthy(); + }); + + it("does not flip on a length failure, which never reaches the server", () => { + renderCard(false); + fill("New password", "short"); + fireEvent.click(screen.getByRole("button", { name: "Set password" })); + + expect(screen.queryByLabelText("Current password")).toBeNull(); + }); +}); + +describe("the destructive consequence is stated before the submit", () => { + it("warns that changing a password signs other devices out", () => { + renderCard(true); + + expect(screen.getByText(/signs you out of your other devices/i)).toBeTruthy(); + }); +}); diff --git a/src/features/settings/__tests__/set-password-page.test.tsx b/src/features/settings/__tests__/set-password-page.test.tsx new file mode 100644 index 0000000..5bba3ec --- /dev/null +++ b/src/features/settings/__tests__/set-password-page.test.tsx @@ -0,0 +1,86 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { cleanup, render, screen, waitFor } from "@testing-library/react"; + +import { SetPasswordPage } from "@/features/settings/set-password-page"; + +const replace = vi.fn(); +vi.mock("next/navigation", () => ({ useRouter: () => ({ replace }) })); + +const useMeQuery = vi.fn(); +vi.mock("@/features/_shared/queries/use-me-query", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useMeQuery: () => useMeQuery() }; +}); + +vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +afterEach(() => cleanup()); +beforeEach(() => vi.clearAllMocks()); + +function renderPage() { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); +} + +describe("who this page is for", () => { + it("shows the form to an account with no password", () => { + useMeQuery.mockReturnValue({ isPending: false, isSuccess: true, data: { password_set: false } }); + renderPage(); + + expect(screen.getByLabelText("New password")).toBeTruthy(); + expect(replace).not.toHaveBeenCalled(); + }); + + // AC-9. The redirect is client-side by necessity (a page cannot call + // `readActiveSession`, which writes cookies), so this asserts the redirect + // fires and that nothing is submittable before it does — NOT the absence of a + // paint, which this mechanism cannot promise. + it("sends an account that already has a password to the profile", async () => { + useMeQuery.mockReturnValue({ isPending: false, isSuccess: true, data: { password_set: true } }); + renderPage(); + + await waitFor(() => expect(replace).toHaveBeenCalledWith("/settings/profile")); + expect(screen.queryByLabelText("New password")).toBeNull(); + }); + + // `undefined` is not `false`: the backend cannot say, so the card would offer + // nothing anyway, and the profile is where the rest of the account lives. + it("sends an unknown password state to the profile too", async () => { + useMeQuery.mockReturnValue({ isPending: false, isSuccess: true, data: {} }); + renderPage(); + + await waitFor(() => expect(replace).toHaveBeenCalledWith("/settings/profile")); + expect(screen.queryByLabelText("New password")).toBeNull(); + }); + + // An errored query leaves `password_set` undefined, which the card renders as + // "this deployment doesn't report password state" — blaming a version gap for + // a transient outage. The two must not share copy. + it("says the account could not be loaded, not that the feature is missing", () => { + useMeQuery.mockReturnValue({ + isPending: false, + isSuccess: false, + isError: true, + data: undefined, + }); + renderPage(); + + expect(screen.getByRole("alert").textContent).toMatch(/couldn't load your account/i); + expect(screen.queryByText(/doesn't report password state/i)).toBeNull(); + expect(replace).not.toHaveBeenCalled(); + }); + + it("shows no form while the answer is still loading", () => { + useMeQuery.mockReturnValue({ isPending: true, isSuccess: false, data: undefined }); + renderPage(); + + expect(screen.queryByLabelText("New password")).toBeNull(); + expect(replace).not.toHaveBeenCalled(); + }); +}); diff --git a/src/features/settings/password-card.tsx b/src/features/settings/password-card.tsx new file mode 100644 index 0000000..bac89f4 --- /dev/null +++ b/src/features/settings/password-card.tsx @@ -0,0 +1,173 @@ +"use client"; + +import { useState } from "react"; +import { toast } from "sonner"; + +import { isPasswordWithinPolicy } from "@/domain/auth/sign-in-method"; +import { BffError } from "@/features/_shared/api/bff-fetch"; +import { useChangePassword } from "@/features/_shared/queries/use-me-query"; +import { Button } from "@/shared/ui/shadcn/button"; +import { Input } from "@/shared/ui/shadcn/input"; +import { Label } from "@/shared/ui/shadcn/label"; + +/** + * Set or change the account password (RUK-289). + * + * Which form is drawn comes from `password_set`, which has THREE meaningful + * values (SPEC §2.4): + * + * - `true` — change: current password + new password. + * - `false` — set: new password only. Sending a current password for an + * account with none is a 400, so the field must not be there. + * - `undefined` — the deployed backend predates the field. Nothing is + * offered, because guessing either form makes every save a 400. + */ + +export interface PasswordCardProps { + /** `me.password_set`. `undefined` is "unknown", never "false". */ + passwordSet: boolean | undefined; +} + +export function PasswordCard({ passwordSet }: PasswordCardProps) { + const change = useChangePassword(); + const [current, setCurrent] = useState(""); + const [next, setNext] = useState(""); + const [error, setError] = useState(); + + /** + * Which shape the form is in. Seeded from `password_set` and allowed to flip + * ONCE — see `flipped`. + */ + const [asChange, setAsChange] = useState(passwordSet === true); + /** + * A `password_set` that disagrees with reality is possible: the backend + * answers `false` when its own read of the credential fails. The wrong guess + * is answered with a 400, and this flips the form to the other shape so the + * user is not stuck. + * + * Once, and only once. A 400 is NOT evidence about `password_set` — the same + * status also carries a length-policy failure, and all three of the backend's + * 400s share one code — so an unbounded rule would oscillate between the two + * forms forever. + */ + const [flipped, setFlipped] = useState(false); + + if (passwordSet === undefined) { + return ( +

+ Password management is unavailable — this deployment doesn't report password state yet. +

+ ); + } + + const onSubmit = (event: React.FormEvent) => { + event.preventDefault(); + if (change.isPending) return; + + // Checked before sending so a length failure is never attributed to the + // wrong thing by the flip rule below. Bytes, not characters: the backend + // measures with Go's `len()`. + if (!isPasswordWithinPolicy(next)) { + setError("Choose a longer password — at least 12 characters."); + return; + } + setError(undefined); + + change.mutate( + { ...(asChange ? { current_password: current } : {}), new_password: next }, + { + onSuccess: () => { + toast.success(asChange ? "Password changed" : "Password set"); + setCurrent(""); + setNext(""); + }, + onError: (mutationError) => { + if (!(mutationError instanceof BffError)) { + toast.error("Couldn't save your password. Try again."); + return; + } + + if (mutationError.status === 422) { + setError("That current password isn't right."); + return; + } + if (mutationError.status === 409) { + // Nothing the user typed was wrong, and nothing was changed. + toast.error("Your session expired before the change was saved. Sign in again."); + return; + } + if (mutationError.status === 400 && !flipped) { + // The password passed the local length check, so this 400 is the + // shape being wrong for this account's real state. Flip once. + setFlipped(true); + setAsChange((wasChange) => !wasChange); + setCurrent(""); + setError( + asChange + ? "This account has no password yet — set one below." + : "This account already has a password — enter your current one.", + ); + return; + } + setError(mutationError.message || "Couldn't save your password. Try again."); + }, + }, + ); + }; + + return ( +
+ {asChange ? ( + <> + + setCurrent(e.target.value)} + /> + + ) : null} + + + setNext(e.target.value)} + aria-describedby="new-password-hint" + /> + {/* + * "Characters" rather than bytes. The policy is 12 BYTES, which is not a + * unit to show an operator; the ASCII worst case can only under-promise, + * so a non-ASCII password shorter than the hint is still accepted. + */} +

+ At least 12 characters. {asChange ? "Changing it signs you out of your other devices." : null} +

+ + {error ? ( +

+ {error} +

+ ) : null} + +
+ + {!asChange ? ( + // The page is the same form on its own, for someone who came here to + // do this one thing. It is this card's only entry point. + + Open on its own page + + ) : null} +
+
+ ); +} diff --git a/src/features/settings/set-password-page.tsx b/src/features/settings/set-password-page.tsx new file mode 100644 index 0000000..e3e9615 --- /dev/null +++ b/src/features/settings/set-password-page.tsx @@ -0,0 +1,82 @@ +"use client"; + +import { useRouter } from "next/navigation"; +import { useEffect } from "react"; + +import { useMeQuery } from "@/features/_shared/queries/use-me-query"; +import { PasswordCard } from "@/features/settings/password-card"; +import { Skeleton } from "@/shared/ui/domain/skeleton"; + +/** + * A focused page for setting a first password (RUK-289), reached from the + * profile card. Renders the same form in a bare centered layout rather than + * inside `AppShell`, so there is one thing on the screen to do. + * + * **The redirect is client-side, and that is forced rather than preferred.** + * A server component cannot read the session here: `readActiveSession()` + * refreshes and writes cookies when the access token is near expiry, and Next + * permits a cookie write only inside a Server Action or Route Handler — a page + * render throws `ReadonlyRequestCookiesError`. Building it server-side would + * therefore 500 only inside the rotation window: green tests, intermittent + * production failure. + * + * The cost is a brief render before the redirect. It is acceptable because the + * page is reached deliberately from the profile card, so arriving in the wrong + * state is rare, and because the form is inert until submitted. + */ +export function SetPasswordPage() { + const router = useRouter(); + const meQuery = useMeQuery(); + const passwordSet = meQuery.data?.password_set; + + // `true` means they are not who this page is for; `undefined` means the + // backend cannot say, and the card would offer nothing anyway. Both belong on + // the profile, where the rest of the account lives. + const shouldLeave = meQuery.isSuccess && passwordSet !== false; + + useEffect(() => { + if (shouldLeave) { + router.replace("/settings/profile"); + } + }, [shouldLeave, router]); + + return ( +
+
+
+

Set a password

+

+ Add a password so you can sign in with your email address as well as your current method. +

+
+ + {meQuery.isPending || shouldLeave ? ( + // Shaped like the form it stands in for, not a fixed block. The page + // is `grid place-items-center`, so a height change on swap + // re-centres the whole card and visibly moves the heading above it — + // a placeholder of the wrong height guarantees that on every load. + // + // Also covers the moment between deciding to leave and the navigation + // landing, so the form never flashes at someone on their way out. + + ) : meQuery.isError ? ( + // Distinct from the card's own "this deployment doesn't report + // password state" copy. Both would otherwise render here — an errored + // query leaves `password_set` undefined — and blaming a version gap + // for a transient outage is the §3.6 conflation this change avoids + // everywhere else. +

+ Couldn't load your account just now. Reload to try again. +

+ ) : ( + + )} +
+
+ ); +} diff --git a/src/features/settings/user-settings-page.tsx b/src/features/settings/user-settings-page.tsx index 95275dd..4dca7c8 100644 --- a/src/features/settings/user-settings-page.tsx +++ b/src/features/settings/user-settings-page.tsx @@ -22,6 +22,7 @@ import { BrandIcon, type BrandProvider } from "@/shared/ui/icons/brand-icons"; import { useMeQuery } from "@/features/_shared/queries/use-me-query"; import { MessengerTagsFields } from "./messenger-tags-card"; import { TimezoneCard } from "./timezone-card"; +import { PasswordCard } from "./password-card"; import type { Role } from "@/domain/auth/permissions"; /** Role chips render admin-first, consistent with users-management. */ @@ -181,6 +182,10 @@ export function UserSettingsPage() { + + + +

This signs you out on this device only. Use the option below to sign out from all devices. diff --git a/src/server/auth/__tests__/backend-token-exchange-password.test.ts b/src/server/auth/__tests__/backend-token-exchange-password.test.ts new file mode 100644 index 0000000..43694e5 --- /dev/null +++ b/src/server/auth/__tests__/backend-token-exchange-password.test.ts @@ -0,0 +1,299 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + changeBackendPassword, + confirmPasswordReset, + requestPasswordResetCode, +} from "@/server/auth/backend-token-exchange"; + +/** + * The password HTTP client, executed rather than mocked. + * + * Every other suite in this change stubs this module, which left its 401 + * classification — the single most consequential branch in RUK-289 — provable + * only by reading it. An inverted classification tells an operator with a wrong + * password that their session expired, and an operator with a dead session that + * their password is wrong on a form with no password field. + * + * Assertions read the REQUEST off the fetch stub, not the arguments object, so + * they pin what actually goes on the wire. + */ + +const fetchMock = vi.fn(); + +beforeEach(() => { + vi.stubGlobal("fetch", fetchMock); + fetchMock.mockReset(); + process.env.MAINTMODE_API_BASE_URL = "http://backend.test/maintmode"; +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +function respond(status: number, body = "") { + return { + ok: status >= 200 && status < 300, + status, + statusText: `status ${status}`, + text: async () => body, + }; +} + +/** The JSON body of the Nth fetch call. */ +function sentBody(call = 0): Record { + return JSON.parse(fetchMock.mock.calls[call][1].body); +} + +describe("changeBackendPassword — the 401 classification", () => { + // The inversion of this pair is invisible to every other test in the change, + // and it is the failure SPEC §3.4 exists to prevent. + it("reads a 401 as a wrong password when a current password was sent", async () => { + fetchMock.mockResolvedValue(respond(401, '{"code":"unauthorized"}')); + + const outcome = await changeBackendPassword({ + accessToken: "access-1", + currentPassword: "wrong", + newPassword: "a-long-enough-password", + refreshToken: "refresh-1", + }); + + expect(outcome).toEqual({ ok: false, kind: "wrong-current-password" }); + }); + + it("reads a 401 as a stale session when none was sent", async () => { + fetchMock.mockResolvedValue(respond(401, '{"code":"unauthorized"}')); + + const outcome = await changeBackendPassword({ + accessToken: "access-1", + newPassword: "a-long-enough-password", + refreshToken: "stale-refresh", + }); + + // Nothing the user typed was wrong, and the backend changed nothing. + expect(outcome).toEqual({ ok: false, kind: "session-stale" }); + }); + + it("maps 204, 400 and everything else", async () => { + fetchMock.mockResolvedValueOnce(respond(204)); + await expect( + changeBackendPassword({ accessToken: "a", newPassword: "a-long-enough-password" }), + ).resolves.toEqual({ ok: true }); + + // A realistic envelope, not a hand-typed bare string. The previous fixture + // was satisfied identically by passing the WHOLE body through, which is how + // raw JSON reached the operator's form unnoticed. + fetchMock.mockResolvedValueOnce( + respond( + 400, + '{"code":"invalid request","message":"validation error: password does not meet the length policy"}', + ), + ); + await expect( + changeBackendPassword({ accessToken: "a", newPassword: "a-long-enough-password" }), + ).resolves.toEqual({ + ok: false, + kind: "rejected", + message: "validation error: password does not meet the length policy", + }); + + fetchMock.mockResolvedValueOnce(respond(503, "gateway down")); + await expect( + changeBackendPassword({ accessToken: "a", newPassword: "a-long-enough-password" }), + ).resolves.toEqual({ ok: false, kind: "unavailable" }); + }); + + // A 200 or 202 is not success here: the contract is 204 with an empty body, + // and treating any 2xx as done would report a password change the backend + // never made. + // A proxy's HTML error page must not be pasted into the form whole. + it("falls back to a sentence when a 400 is not the expected envelope", async () => { + fetchMock.mockResolvedValue(respond(400, "502 Bad Gateway")); + + await expect( + changeBackendPassword({ accessToken: "a", newPassword: "a-long-enough-password" }), + ).resolves.toEqual({ + ok: false, + kind: "rejected", + message: "That password wasn't accepted.", + }); + }); + + it("treats a 2xx that is not 204 as unavailable", async () => { + fetchMock.mockResolvedValue(respond(200, "{}")); + + await expect( + changeBackendPassword({ accessToken: "a", newPassword: "a-long-enough-password" }), + ).resolves.toEqual({ ok: false, kind: "unavailable" }); + }); + + it("treats a thrown fetch as unavailable rather than propagating", async () => { + fetchMock.mockRejectedValue(new Error("network down")); + + await expect( + changeBackendPassword({ accessToken: "a", newPassword: "a-long-enough-password" }), + ).resolves.toEqual({ ok: false, kind: "unavailable" }); + }); +}); + +describe("changeBackendPassword — what goes on the wire", () => { + it("sends the refresh token, which is what keeps the caller signed in", async () => { + fetchMock.mockResolvedValue(respond(204)); + + await changeBackendPassword({ + accessToken: "access-1", + currentPassword: "old-password", + newPassword: "a-long-enough-password", + refreshToken: "refresh-1", + }); + + // Omitting it makes the backend revoke every session including this one: + // the operator is signed out by their own success. + expect(sentBody()).toEqual({ + current_password: "old-password", + new_password: "a-long-enough-password", + refresh_token: "refresh-1", + }); + }); + + it("omits current_password as a KEY when the account has none", async () => { + fetchMock.mockResolvedValue(respond(204)); + + await changeBackendPassword({ + accessToken: "access-1", + newPassword: "a-long-enough-password", + refreshToken: "refresh-1", + }); + + // Absent, not empty: the backend rejects the field outright for an account + // with no password, so `""` would 400 every set-password call. + expect("current_password" in sentBody()).toBe(false); + }); + + it("omits refresh_token as a key when there is none to send", async () => { + fetchMock.mockResolvedValue(respond(204)); + + await changeBackendPassword({ accessToken: "a", newPassword: "a-long-enough-password" }); + + expect("refresh_token" in sentBody()).toBe(false); + }); + + it("carries the access token as a bearer credential", async () => { + fetchMock.mockResolvedValue(respond(204)); + + await changeBackendPassword({ accessToken: "access-1", newPassword: "a-long-enough-password" }); + + expect(fetchMock.mock.calls[0][1].headers.authorization).toBe("Bearer access-1"); + expect(String(fetchMock.mock.calls[0][0])).toContain("/api/v1/me/password"); + }); +}); + +describe("confirmPasswordReset", () => { + // Without the throw, EVERY reset failure resolves — the caller reports + // success, the user is told their password changed and is signed out, and the + // password is unchanged. + it("throws on a failure rather than resolving", async () => { + fetchMock.mockResolvedValue(respond(401, '{"code":"otp_session_mismatch"}')); + + await expect( + confirmPasswordReset({ + email: "op@example.test", + code: "123456", + sessionNonce: "nonce-1", + newPassword: "a-long-enough-password", + }), + ).rejects.toMatchObject({ status: 401, responseBody: '{"code":"otp_session_mismatch"}' }); + }); + + it("resolves on 204", async () => { + fetchMock.mockResolvedValue(respond(204)); + + await expect( + confirmPasswordReset({ + email: "op@example.test", + code: "123456", + sessionNonce: "nonce-1", + newPassword: "a-long-enough-password", + }), + ).resolves.toBeUndefined(); + }); + + it("sends the nonce under the key the backend reads", async () => { + fetchMock.mockResolvedValue(respond(204)); + + await confirmPasswordReset({ + email: "op@example.test", + code: "123456", + sessionNonce: "nonce-1", + newPassword: "a-long-enough-password", + }); + + // Named explicitly: a renamed or empty `session_nonce` is answered with the + // same opaque 401 as a wrong code, so nothing downstream could tell. + expect(sentBody()).toEqual({ + email: "op@example.test", + code: "123456", + session_nonce: "nonce-1", + new_password: "a-long-enough-password", + }); + expect(String(fetchMock.mock.calls[0][0])).toContain("/api/v1/password/reset/confirm"); + }); +}); + +describe("the request timeout covers the body read, not just the head", () => { + // `fetch` resolves on the response HEAD, so a helper that cleared its timeout + // on return would leave a stalled BODY hanging forever — which is the case + // the timeout exists for. A refactor that did exactly this passed all 1424 + // tests, because nothing covered it. + // + // No fake timers needed: the config floor is 100ms, and the assertion that + // matters is that the signal ABORTED. Without that line the test passes + // against the broken version, because the rejection then comes from shape + // validation rather than from the abort. + it("aborts a response whose body never arrives", async () => { + process.env.MAINTMODE_API_TIMEOUT_MS = "100"; + let aborted = false; + + fetchMock.mockImplementation(async (_url: string, init: { signal: AbortSignal }) => ({ + ok: true, + status: 202, + statusText: "Accepted", + text: () => + new Promise((_resolve, reject) => { + init.signal.addEventListener("abort", () => { + aborted = true; + reject(new Error("aborted")); + }); + }), + })); + + await expect(requestPasswordResetCode("op@example.test")).rejects.toThrow(); + expect(aborted).toBe(true); + + delete process.env.MAINTMODE_API_TIMEOUT_MS; + }); +}); + +describe("requestPasswordResetCode", () => { + it("returns the session nonce from a 202", async () => { + fetchMock.mockResolvedValue(respond(202, '{"session_nonce":"nonce-1"}')); + + await expect(requestPasswordResetCode("op@example.test")).resolves.toEqual({ + session_nonce: "nonce-1", + }); + expect(sentBody()).toEqual({ email: "op@example.test" }); + expect(String(fetchMock.mock.calls[0][0])).toContain("/api/v1/password/reset/request"); + }); + + it("rejects a 202 that carries no nonce, rather than binding undefined", async () => { + fetchMock.mockResolvedValue(respond(202, "{}")); + + await expect(requestPasswordResetCode("op@example.test")).rejects.toThrow(); + }); + + it("throws with the status on a rate limit", async () => { + fetchMock.mockResolvedValue(respond(429, "slow down")); + + await expect(requestPasswordResetCode("op@example.test")).rejects.toMatchObject({ status: 429 }); + }); +}); diff --git a/src/server/auth/__tests__/built-in-sign-in-callback.test.ts b/src/server/auth/__tests__/built-in-sign-in-callback.test.ts index 76658b4..e96ee27 100644 --- a/src/server/auth/__tests__/built-in-sign-in-callback.test.ts +++ b/src/server/auth/__tests__/built-in-sign-in-callback.test.ts @@ -31,7 +31,6 @@ vi.mock("@/server/auth/otp-nonce-cookie", () => ({ // The real implementation: normalization is part of the behaviour under test, // so stubbing it would hide the case-variant bug this file now covers. normalizeEmail: (email: string) => email.trim().toLowerCase(), - OTP_NONCE_COOKIE: "__Host-mm.otp_nonce", })); const { runBuiltInSignIn } = await import("@/server/auth/built-in-sign-in"); diff --git a/src/server/auth/__tests__/otp-nonce-cookie.test.ts b/src/server/auth/__tests__/otp-nonce-cookie.test.ts index c09d557..2e510a6 100644 --- a/src/server/auth/__tests__/otp-nonce-cookie.test.ts +++ b/src/server/auth/__tests__/otp-nonce-cookie.test.ts @@ -2,9 +2,13 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { OTP_NONCE_COOKIE, + PWRESET_NONCE_COOKIE, clearOtpBinding, + clearPasswordResetBinding, readOtpBinding, + readPasswordResetBinding, setOtpBinding, + setPasswordResetBinding, } from "@/server/auth/otp-nonce-cookie"; const store = { @@ -116,3 +120,67 @@ describe("otp-nonce-cookie — the encoding resists a hostile email address", () expect(read?.email).toBe(hostile); }); }); + +describe("two flows, two cookies (RUK-289)", () => { + beforeEach(() => { + store.set.mockReset(); + store.get.mockReset(); + store.delete.mockReset(); + }); + + // Asserted as literal strings against the real exported constants, NOT + // re-derived from whatever produces them and NOT mocked. Both names are + // deployed artifacts: renaming the sign-in one invalidates every binding a + // user is holding mid-flow at deploy time, and a test that recomputes the + // name would pass through exactly that rename. + it("pins both cookie names as deployed artifacts", () => { + expect(OTP_NONCE_COOKIE).toBe("__Host-mm.otp_nonce"); + expect(PWRESET_NONCE_COOKIE).toBe("__Host-mm.pwreset_nonce"); + }); + + // The bug this split exists for: with ONE cookie, requesting a password-reset + // code overwrites the sign-in binding, and the email equality check PASSES + // because the address is the same. The sign-in code is then verified against + // the reset nonce and the user is told a correct code is wrong. + // + // Note what this does NOT claim: the sign-in CODE does not survive either, + // because the backend keeps one OTP record per user and issuing a reset code + // consumes the live one (SPEC §1.5). Only the local binding is separated. + it("writing a reset binding leaves the sign-in binding untouched", async () => { + await setOtpBinding({ nonce: "signin-nonce", email: "op@example.test" }); + await setPasswordResetBinding({ nonce: "reset-nonce", email: "op@example.test" }); + + const written = store.set.mock.calls.map(([name]) => name); + expect(written).toEqual(["__Host-mm.otp_nonce", "__Host-mm.pwreset_nonce"]); + }); + + it("reads the reset binding from its own cookie", async () => { + store.get.mockImplementation((name: string) => + name === "__Host-mm.pwreset_nonce" + ? { value: encode({ nonce: "reset-nonce", email: "op@example.test" }) } + : undefined, + ); + + await expect(readPasswordResetBinding()).resolves.toEqual({ + nonce: "reset-nonce", + email: "op@example.test", + }); + // The sign-in reader must not fall back to the reset cookie. + await expect(readOtpBinding()).resolves.toBeUndefined(); + }); + + it("clearing one flow's binding does not clear the other's", async () => { + await clearPasswordResetBinding(); + + expect(store.delete).toHaveBeenCalledTimes(1); + expect(store.delete).toHaveBeenCalledWith("__Host-mm.pwreset_nonce"); + }); + + it("keeps the reset cookie's flags and TTL identical to sign-in's", async () => { + await setPasswordResetBinding({ nonce: "n", email: "op@example.test" }); + + const [, , opts] = store.set.mock.calls[0]; + expect(opts).toMatchObject({ httpOnly: true, sameSite: "lax", secure: true, path: "/" }); + expect(opts.maxAge).toBe(300); + }); +}); diff --git a/src/server/auth/__tests__/password-reset-actions.test.ts b/src/server/auth/__tests__/password-reset-actions.test.ts new file mode 100644 index 0000000..c0856ff --- /dev/null +++ b/src/server/auth/__tests__/password-reset-actions.test.ts @@ -0,0 +1,334 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { + abandonPasswordResetAction, + confirmPasswordResetAction, + requestPasswordResetAction, +} from "@/server/auth/password-reset-actions"; + +const requestPasswordResetCode = vi.fn(); +const confirmPasswordReset = vi.fn(); +const setPasswordResetBinding = vi.fn(); +const readPasswordResetBinding = vi.fn(); +const clearPasswordResetBinding = vi.fn(); +const signOut = vi.fn(); +const clearActiveSession = vi.fn(); + +vi.mock("@/server/auth/backend-token-exchange", () => ({ + requestPasswordResetCode: (...args: unknown[]) => requestPasswordResetCode(...args), + confirmPasswordReset: (...args: unknown[]) => confirmPasswordReset(...args), +})); + +vi.mock("@/server/auth/otp-nonce-cookie", async (importOriginal) => { + // `normalizeEmail` is the real one: the binding check compares against it, and + // a stubbed version would make the mismatch tests agree with themselves. + const actual = await importOriginal(); + return { + normalizeEmail: actual.normalizeEmail, + setPasswordResetBinding: (...args: unknown[]) => setPasswordResetBinding(...args), + readPasswordResetBinding: () => readPasswordResetBinding(), + clearPasswordResetBinding: () => clearPasswordResetBinding(), + }; +}); + +vi.mock("@/server/auth/auth-config", () => ({ signOut: (...args: unknown[]) => signOut(...args) })); +vi.mock("@/server/auth/session-token", () => ({ clearActiveSession: () => clearActiveSession() })); + +/** A backend failure as `postBackendJson` throws it. */ +function backendError(status: number, body = "") { + return Object.assign(new Error(`backend ${status}`), { status, responseBody: body }); +} + +beforeEach(() => { + vi.clearAllMocks(); + console.error = vi.fn(); +}); + +describe("requesting a reset code cannot become an account-existence oracle", () => { + // AC-1. The backend answers 202 for a registered address, an unknown one and + // a blocked one alike; this asserts the ACTION collapses them too, which is + // the only half of that promise the frontend owns. + it("answers identically for every address the backend accepts", async () => { + requestPasswordResetCode.mockResolvedValue({ session_nonce: "n" }); + + const registered = await requestPasswordResetAction("known@example.test"); + const unknown = await requestPasswordResetAction("nobody@example.test"); + const blocked = await requestPasswordResetAction("blocked@example.test"); + + expect(registered).toEqual({}); + expect(unknown).toEqual(registered); + expect(blocked).toEqual(registered); + }); + + it("binds the nonce to this browser", async () => { + requestPasswordResetCode.mockResolvedValue({ session_nonce: "nonce-1" }); + + await requestPasswordResetAction("op@example.test"); + + expect(setPasswordResetBinding).toHaveBeenCalledWith({ nonce: "nonce-1", email: "op@example.test" }); + }); + + it("rejects an empty address without calling the backend", async () => { + await expect(requestPasswordResetAction(" ")).resolves.toEqual({ error: "invalid_email" }); + expect(requestPasswordResetCode).not.toHaveBeenCalled(); + }); + + // AC-12. A rate limit is not a verdict on the address, and an outage is a + // fact about the service — neither may wear the uniform copy, which would + // tell every user their input was wrong. + it("separates a rate limit and an outage from the uniform answer", async () => { + requestPasswordResetCode.mockRejectedValueOnce(backendError(429)); + await expect(requestPasswordResetAction("op@example.test")).resolves.toEqual({ + error: "otp_rate_limited", + }); + + requestPasswordResetCode.mockRejectedValueOnce(backendError(503)); + await expect(requestPasswordResetAction("op@example.test")).resolves.toEqual({ + error: "password_reset_unavailable", + }); + + requestPasswordResetCode.mockRejectedValueOnce(backendError(404)); + await expect(requestPasswordResetAction("op@example.test")).resolves.toEqual({ + error: "password_reset_unavailable", + }); + }); + + // A rejected input is not an outage. Today this endpoint answers only 202 and + // 429, but reporting a future 4xx as "the service is down" would send a user + // whose address was refused off to wait for a recovery that never comes. + it("does not report a rejected request as an outage", async () => { + requestPasswordResetCode.mockRejectedValueOnce(backendError(400)); + + await expect(requestPasswordResetAction("op@example.test")).resolves.toEqual({ + error: "invalid_email", + }); + }); +}); + +describe("confirming a reset", () => { + beforeEach(() => { + readPasswordResetBinding.mockResolvedValue({ nonce: "nonce-1", email: "op@example.test" }); + }); + + // AC-3, at the action boundary. The backend hides a policy violation inside + // the same 401 as a wrong code, so a password that fails the policy must + // never reach it — the user would be told their correct code was wrong. + it("refuses a short password without spending an attempt", async () => { + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "short", + }); + + expect(result).toEqual({ error: "password_policy_violation" }); + expect(confirmPasswordReset).not.toHaveBeenCalled(); + // The binding survives: nothing was spent, and the code is still good. + expect(clearPasswordResetBinding).not.toHaveBeenCalled(); + }); + + // AC-4. Past the 204 the password IS changed and every session is dead. + it("tears the local session down on success", async () => { + confirmPasswordReset.mockResolvedValue(undefined); + + await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + expect(signOut).toHaveBeenCalledWith({ redirect: false }); + expect(clearActiveSession).toHaveBeenCalled(); + }); + + // AC-4, the half that matters more. The backend has already committed the + // change; a teardown that throws must not swallow the confirmation, or the + // user is left believing their password is unchanged when it is not. + it("still confirms when the teardown throws", async () => { + confirmPasswordReset.mockResolvedValue(undefined); + signOut.mockRejectedValueOnce(new Error("cookie store unavailable")); + + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + expect(result).toEqual({ done: true }); + }); + + it("sends the bound nonce, not anything the caller supplied", async () => { + confirmPasswordReset.mockResolvedValue(undefined); + + await confirmPasswordResetAction({ + email: "OP@Example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + expect(confirmPasswordReset).toHaveBeenCalledWith({ + email: "op@example.test", + code: "123456", + sessionNonce: "nonce-1", + newPassword: "a-long-enough-password", + }); + }); + + it("reports a lost binding with the reset flow's own code, not sign-in's", async () => { + readPasswordResetBinding.mockResolvedValue(undefined); + + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + // Not `otp_session_mismatch`: its copy returns the user to sign-in, which + // is not where someone mid-reset is trying to go. + expect(result).toEqual({ error: "password_reset_session_mismatch" }); + expect(confirmPasswordReset).not.toHaveBeenCalled(); + }); + + it("refuses when the bound address is not the one being confirmed", async () => { + readPasswordResetBinding.mockResolvedValue({ nonce: "n", email: "someone-else@example.test" }); + + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + expect(result).toEqual({ error: "password_reset_session_mismatch" }); + expect(confirmPasswordReset).not.toHaveBeenCalled(); + }); + + // The clear policy, which is the difference between a recoverable mistype and + // a destroyed code. + it("keeps the binding on a wrong code, so remaining attempts survive", async () => { + confirmPasswordReset.mockRejectedValueOnce( + backendError(401, '{"code":"unauthorized","message":"authentication failed"}'), + ); + + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "000000", + newPassword: "a-long-enough-password", + }); + + expect(result).toEqual({ error: "password_reset_failed" }); + expect(clearPasswordResetBinding).not.toHaveBeenCalled(); + }); + + it("clears the binding when the backend reports a session mismatch", async () => { + // Spaced exactly as a pretty-printing encoder would emit it: a substring + // match on `"code":"..."` would miss this and fail open into the generic + // collapse, showing sign-in recovery copy on a reset screen. + confirmPasswordReset.mockRejectedValueOnce( + backendError(401, '{ "code": "otp_session_mismatch", "message": "authentication failed" }'), + ); + + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + expect(result).toEqual({ error: "password_reset_session_mismatch" }); + expect(clearPasswordResetBinding).toHaveBeenCalled(); + }); + + // §3.6 requires the rate limit to keep its own copy on BOTH endpoints. The + // request path already had this; the confirm path did not, and folding a 429 + // into "unavailable" tells a throttled user the service is broken. + it("keeps a rate limit apart from an outage, and keeps the binding", async () => { + confirmPasswordReset.mockRejectedValueOnce(backendError(429)); + + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + expect(result).toEqual({ error: "otp_rate_limited" }); + expect(clearPasswordResetBinding).not.toHaveBeenCalled(); + }); + + // §3.4's rule, made executable. A substring implementation would match the + // code wherever it appears — including inside `message` — and would then + // clear a binding that still had attempts left, destroying a usable code. + // Every other fixture here has field and substring agreeing, so only this + // one separates the two implementations. + it("reads the code FIELD, not the message text", async () => { + confirmPasswordReset.mockRejectedValueOnce( + backendError( + 401, + '{"code":"unauthorized","message":"authentication failed for otp_session_mismatch reasons"}', + ), + ); + + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + expect(result).toEqual({ error: "password_reset_failed" }); + expect(clearPasswordResetBinding).not.toHaveBeenCalled(); + }); + + it("keeps an outage apart from a wrong code, and keeps the binding", async () => { + confirmPasswordReset.mockRejectedValueOnce(backendError(502)); + + const result = await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + expect(result).toEqual({ error: "password_reset_unavailable" }); + expect(clearPasswordResetBinding).not.toHaveBeenCalled(); + }); + + // The teardown error can be a BackendAuthError whose `responseBody` is the + // raw refresh response — a token pair. Logging the object would serialize it. + it("never logs credential material when the teardown fails", async () => { + confirmPasswordReset.mockResolvedValue(undefined); + signOut.mockRejectedValueOnce( + Object.assign(new Error("refresh failed"), { + status: 401, + responseBody: '{"access_token":"at_SECRET","refresh_token":"rt_SECRET"}', + }), + ); + + await confirmPasswordResetAction({ + email: "op@example.test", + code: "123456", + newPassword: "a-long-enough-password", + }); + + const logged = JSON.stringify((console.error as unknown as ReturnType).mock.calls); + expect(logged).not.toContain("rt_SECRET"); + expect(logged).not.toContain("access_token"); + }); + + it("never logs the address or the code", async () => { + confirmPasswordReset.mockRejectedValueOnce(backendError(401, "{}")); + + await confirmPasswordResetAction({ + email: "secret@example.test", + code: "424242", + newPassword: "a-long-enough-password", + }); + + const logged = JSON.stringify((console.error as unknown as ReturnType).mock.calls); + expect(logged).not.toContain("secret@example.test"); + expect(logged).not.toContain("424242"); + }); +}); + +describe("abandoning the flow", () => { + it("clears only the reset binding", async () => { + await abandonPasswordResetAction(); + + expect(clearPasswordResetBinding).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/server/auth/backend-token-exchange.ts b/src/server/auth/backend-token-exchange.ts index 2d42c6f..2bfd421 100644 --- a/src/server/auth/backend-token-exchange.ts +++ b/src/server/auth/backend-token-exchange.ts @@ -12,6 +12,9 @@ const ME_PATH = "/api/v1/me"; const OTP_REQUEST_PATH = "/api/v1/login/otp/request"; const OTP_VERIFY_PATH = "/api/v1/login/otp/verify"; const PASSWORD_LOGIN_PATH = "/api/v1/login/password"; +const PASSWORD_RESET_REQUEST_PATH = "/api/v1/password/reset/request"; +const PASSWORD_RESET_CONFIRM_PATH = "/api/v1/password/reset/confirm"; +const CHANGE_PASSWORD_PATH = "/api/v1/me/password"; /** * BFF-owned OAuth. @@ -149,6 +152,166 @@ export async function loginWithPassword(args: { ); } +/** + * Password reset, step one: ask the backend to mail a code. + * + * Shaped exactly like `requestOtpCode` because it IS the same mechanism — the + * backend routes both through one OTP issuer. It answers 202 with a session + * nonce for every outcome, including an address with no account and a malformed + * body (a placeholder nonce), so nothing here may branch on the result. + * + * Note the consequence the caller must not paper over: because the two flows + * share one OTP record per user, asking for a reset code consumes any live + * sign-in code. Separate cookies keep the two BINDINGS apart; they cannot keep + * the codes apart. + */ +export async function requestPasswordResetCode(email: string): Promise<{ session_nonce: string }> { + return postBackendJson<{ session_nonce: string }>( + PASSWORD_RESET_REQUEST_PATH, + { email }, + (value): value is { session_nonce: string } => + typeof value === "object" && + value !== null && + typeof (value as { session_nonce?: unknown }).session_nonce === "string", + ); +} + +/** + * Password reset, step two: redeem the code and install the new password. + * + * Answers **204 with an empty body** — no token pair — and revokes every + * session, so the caller signs in again afterwards. It cannot go through + * `postBackendJson`, which requires a JSON payload it can shape-check. + * + * Failures collapse into one 401 "authentication failed", with + * `otp_session_mismatch` as the sole distinguishable case. A password that + * breaks the length policy is INSIDE that collapse and is reported as a wrong + * code — which is why the client checks the length before calling this. + */ +export async function confirmPasswordReset(args: { + email: string; + code: string; + sessionNonce: string; + newPassword: string; +}): Promise { + return backendFetch( + PASSWORD_RESET_CONFIRM_PATH, + { + method: "POST", + headers: { accept: "application/json", "content-type": "application/json" }, + body: JSON.stringify({ + email: args.email, + code: args.code, + session_nonce: args.sessionNonce, + new_password: args.newPassword, + }), + }, + async (response) => { + if (!response.ok) { + throw new BackendAuthError(response.status, (await response.text()) || response.statusText); + } + }, + ); +} + +/** + * The outcome of a change-password call, classified WITHOUT reading any prose. + * + * The backend's three 400s all carry `code: "invalid request"` and differ only + * in free text, so branching on the message would be a contract that breaks on + * a reword. What the caller knows instead is what it sent, which is enough. + */ +export type ChangePasswordOutcome = + | { ok: true } + /** Wrong `current_password` — the caller sent one and the backend refused. */ + | { ok: false; kind: "wrong-current-password" } + /** The `refresh_token` was stale or foreign; NOTHING was changed. */ + | { ok: false; kind: "session-stale" } + /** A 400: the wrong shape for this account's state, or a policy violation. */ + | { ok: false; kind: "rejected"; message: string } + | { ok: false; kind: "unavailable" }; + +/** + * Sets the caller's own password. + * + * Deliberately NOT routed through `authenticatedBackendRequest`, for two + * reasons that both end in "the user is signed out and their password is + * unchanged": + * + * - it retries the mutation after refreshing on a 401, and this endpoint takes + * a `refresh_token` in the BODY, so the retry would carry a token the + * refresh just superseded; + * - it collapses every 401 into one error type, losing the difference between + * a wrong current password and a dead session — which is the difference + * between a message in the form and a redirect to /login. + * + * `refreshToken` names the session to keep alive. Omitting it revokes every + * session including the caller's, which is the honest fallback when the BFF has + * no live refresh token to offer. + */ +export async function changeBackendPassword(args: { + accessToken: string; + currentPassword?: string; + newPassword: string; + refreshToken?: string; +}): Promise { + try { + return await backendFetch( + CHANGE_PASSWORD_PATH, + { + method: "POST", + headers: { + accept: "application/json", + "content-type": "application/json", + authorization: `Bearer ${args.accessToken}`, + }, + body: JSON.stringify({ + // Sent only when there is one. The backend rejects the field outright + // for an account with no password, so an empty string is not the same + // as absent. + ...(args.currentPassword ? { current_password: args.currentPassword } : {}), + new_password: args.newPassword, + ...(args.refreshToken ? { refresh_token: args.refreshToken } : {}), + }), + }, + async (response): Promise => { + if (response.status === 204) { + return { ok: true }; + } + + const body = await response.text(); + + if (response.status === 401) { + // The one classification, and it reads what the REQUEST carried rather + // than what the response says. A 401 when a current password was sent is + // that password being wrong; a 401 when none was sent is the + // `refresh_token` having been superseded, and nothing the user typed. + return { ok: false, kind: args.currentPassword ? "wrong-current-password" : "session-stale" }; + } + if (response.status === 400) { + // The `message` FIELD, not the raw envelope. `body` is the whole + // response text, so passing it straight through put + // `{"code":"invalid request","message":…}` verbatim into the + // operator's form — and would paste a proxy's HTML error page in + // whole. Read for DISPLAY only: §3.4 forbids branching on backend + // prose, and the only behavioural branch remains the card's flip flag. + const parsed = safeJsonParse<{ message?: string }>(body); + return { + ok: false, + kind: "rejected", + message: parsed?.message ?? "That password wasn't accepted.", + }; + } + return { ok: false, kind: "unavailable" }; + }, + ); + } catch { + // Network failure, a malformed URL, or the abort above. This function + // reports rather than throws, so every one of them is `unavailable`. + return { ok: false, kind: "unavailable" }; + } +} + /** * Rotates the refresh token via `POST /api/v1/refresh`. Returns the new * `TokenPairResponse`. @@ -164,13 +327,9 @@ export async function refreshBackendToken(refreshToken: string): Promise { - const config = readMaintmodeBackendConfig(); - const target = resolveBackendUrl(config.authApiBaseUrl, LOGOUT_PATH); - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), config.requestTimeoutMs); - - try { - const response = await fetch(target, { + return backendFetch( + LOGOUT_PATH, + { method: "POST", headers: { accept: "application/json", @@ -178,15 +337,14 @@ export async function revokeBackendSession(accessToken: string, refreshToken: st authorization: `Bearer ${accessToken}`, }, body: JSON.stringify({ refresh_token: refreshToken }), - signal: controller.signal, - }); - if (!response.ok && response.status !== 204) { - const body = await response.text(); - throw new BackendAuthError(response.status, body || response.statusText); - } - } finally { - clearTimeout(timeout); - } + }, + async (response) => { + if (!response.ok && response.status !== 204) { + const body = await response.text(); + throw new BackendAuthError(response.status, body || response.statusText); + } + }, + ); } /** @@ -195,56 +353,80 @@ export async function revokeBackendSession(accessToken: string, refreshToken: st * (`Authorization: Bearer`); there is no body. */ export async function revokeAllBackendSessions(accessToken: string): Promise { - const config = readMaintmodeBackendConfig(); - const target = resolveBackendUrl(config.authApiBaseUrl, LOGOUT_ALL_PATH); - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), config.requestTimeoutMs); - - try { - const response = await fetch(target, { + return backendFetch( + LOGOUT_ALL_PATH, + { method: "POST", headers: { accept: "application/json", authorization: `Bearer ${accessToken}`, }, - signal: controller.signal, - }); - if (!response.ok && response.status !== 204) { - const body = await response.text(); - throw new BackendAuthError(response.status, body || response.statusText); - } - } finally { - clearTimeout(timeout); - } + }, + async (response) => { + if (!response.ok && response.status !== 204) { + const body = await response.text(); + throw new BackendAuthError(response.status, body || response.statusText); + } + }, + ); } /** * Loads the current user's profile via `GET /api/v1/me`. */ export async function fetchBackendMe(accessToken: string): Promise { - const config = readMaintmodeBackendConfig(); - const target = resolveBackendUrl(config.authApiBaseUrl, ME_PATH); - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), config.requestTimeoutMs); - - try { - const response = await fetch(target, { + return backendFetch( + ME_PATH, + { method: "GET", headers: { accept: "application/json", authorization: `Bearer ${accessToken}`, }, - signal: controller.signal, - }); - const body = await response.text(); - if (!response.ok) { - throw new BackendAuthError(response.status, body || response.statusText); - } - const parsed = safeJsonParse(body); - if (!parsed?.id || !parsed.email) { - throw new BackendAuthError(response.status, body, "Backend /me returned an unexpected payload"); - } - return parsed; + }, + async (response) => { + const body = await response.text(); + if (!response.ok) { + throw new BackendAuthError(response.status, body || response.statusText); + } + const parsed = safeJsonParse(body); + if (!parsed?.id || !parsed.email) { + throw new BackendAuthError(response.status, body, "Backend /me returned an unexpected payload"); + } + return parsed; + }, + ); +} + +/** + * Resolves `path` against the auth base URL and runs one `fetch` under the + * configured timeout. + * + * Owns the request scaffold ONLY — the URL and the abort timer, which were + * identical at six call sites. It deliberately does not touch the response: + * these endpoints disagree about what a response even is (a shape-checked JSON + * body, a bare 204, a 401 classified by what the REQUEST carried), and some + * throw where others return a discriminated result. Folding that in would erase + * the distinctions their callers exist to act on. + * + * `handleResponse` receives the response, and the timeout is cleared only once + * it settles. That is why this takes a callback rather than returning the + * `Response`: the timer has to outlive the body read. `fetch` resolves on the + * response HEAD, so clearing it at that point would leave a stalled `.text()` + * hanging forever — exactly the case the timeout exists for. + */ +async function backendFetch( + path: string, + init: Omit, + handleResponse: (response: Response) => Promise, +): Promise { + const config = readMaintmodeBackendConfig(); + const target = resolveBackendUrl(config.authApiBaseUrl, path); + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), config.requestTimeoutMs); + + try { + return await handleResponse(await fetch(target, { ...init, signal: controller.signal })); } finally { clearTimeout(timeout); } @@ -256,13 +438,9 @@ async function postBackendJson( isShapeValid: (parsed: TResponse | undefined) => boolean, extraHeaders?: Record, ): Promise { - const config = readMaintmodeBackendConfig(); - const target = resolveBackendUrl(config.authApiBaseUrl, path); - const controller = new AbortController(); - const timeout = setTimeout(() => controller.abort(), config.requestTimeoutMs); - - try { - const response = await fetch(target, { + return backendFetch( + path, + { method: "POST", headers: { // Spread extra headers first so the fixed accept/content-type below @@ -272,20 +450,19 @@ async function postBackendJson( "content-type": "application/json", }, body: JSON.stringify(body), - signal: controller.signal, - }); - const text = await response.text(); - if (!response.ok) { - throw new BackendAuthError(response.status, text || response.statusText); - } - const parsed = safeJsonParse(text); - if (!isShapeValid(parsed)) { - throw new BackendAuthError(response.status, text, `Backend ${path} returned an unexpected payload`); - } - return parsed as TResponse; - } finally { - clearTimeout(timeout); - } + }, + async (response) => { + const text = await response.text(); + if (!response.ok) { + throw new BackendAuthError(response.status, text || response.statusText); + } + const parsed = safeJsonParse(text); + if (!isShapeValid(parsed)) { + throw new BackendAuthError(response.status, text, `Backend ${path} returned an unexpected payload`); + } + return parsed as TResponse; + }, + ); } function safeJsonParse(text: string): T | undefined { diff --git a/src/server/auth/contracts.ts b/src/server/auth/contracts.ts index 1d1158a..eb96a52 100644 --- a/src/server/auth/contracts.ts +++ b/src/server/auth/contracts.ts @@ -90,5 +90,25 @@ export const AUTH_ERROR_CODES = { // separate so the copy does not tell someone to re-check a correct code and // send more requests into the limiter that is already refusing them. otpRateLimited: "otp_rate_limited", + // Password reset (RUK-289). The reset flow's OWN mismatch code, deliberately + // not a reuse of `otpSessionMismatch`: that one's copy sends the user back to + // sign-in, which is the wrong destination when they are mid-reset and have + // not chosen a new password yet. Leaks nothing, for the same reason its + // sign-in twin leaks nothing — it names a fact about this browser. + passwordResetSessionMismatch: "password_reset_session_mismatch", + // Every other confirm failure: wrong code, expired, attempts exhausted, and — + // because the backend deliberately hides it inside the same collapse — a + // password that breaks the length policy. The client checks length before + // sending precisely so a user never sees this code for that reason. + passwordResetFailed: "password_reset_failed", + // The new password failed the client-side policy check, so nothing was sent. + // Distinct from every server answer: no attempt was spent and the binding is + // still good, so the copy must not tell the user to request a new code. + passwordPolicyViolation: "password_policy_violation", + // The endpoint itself is unreachable or broken (404/5xx), which is a fact + // about the SERVICE and not about an account — so saying so plainly leaks + // nothing. Kept apart from the anti-enumeration collapse because folding it + // in tells every user their input was wrong during an outage. + passwordResetUnavailable: "password_reset_unavailable", } as const; export type AuthErrorCode = (typeof AUTH_ERROR_CODES)[keyof typeof AUTH_ERROR_CODES]; diff --git a/src/server/auth/otp-nonce-cookie.ts b/src/server/auth/otp-nonce-cookie.ts index f3aa578..1076613 100644 --- a/src/server/auth/otp-nonce-cookie.ts +++ b/src/server/auth/otp-nonce-cookie.ts @@ -29,11 +29,28 @@ import { cookies } from "next/headers"; * (the backend answers 202 for any well-formed one), so `${nonce}:${email}` * would let an address containing the delimiter control the parsed nonce. * - * One cookie, not one per flow: a second tab overwrites the first, and the - * first tab's code then fails with `otp_session_mismatch`, which renders the + * ONE COOKIE PER FLOW, not one overall (revised in RUK-289). Within a single + * flow a second tab still overwrites the first, and that remains deliberate: + * the first tab then fails with `otp_session_mismatch`, which renders the * honest "request a new code" state that exists for exactly this situation. + * + * Across flows it was a defect. Sign-in and password-reset both bind a nonce, + * and with one cookie a reset request overwrote the sign-in binding while the + * email equality check still PASSED — same address — so a sign-in code was + * verified against the reset nonce and the user was told a correct code was + * wrong. + * + * Separating the names fixes the binding, and only the binding. The backend + * keeps one OTP record per user, so requesting a reset code still consumes any + * live sign-in code server-side (SPEC §1.5): the sign-in tab fails either way, + * and the point of the split is that it now fails as "request a new code" + * rather than as "that code is wrong". + * + * Both names are deployed artifacts. Renaming either one invalidates every + * binding users hold mid-flow at deploy time. */ export const OTP_NONCE_COOKIE = "__Host-mm.otp_nonce"; +export const PWRESET_NONCE_COOKIE = "__Host-mm.pwreset_nonce"; /** * Equal to the backend's `otp_ttl` (5 min), deliberately not longer. A margin @@ -67,13 +84,13 @@ export function normalizeEmail(email: string): string { return email.trim().toLowerCase(); } -export async function setOtpBinding(binding: OtpBinding): Promise { +async function setBinding(name: string, binding: OtpBinding): Promise { const encoded = Buffer.from( JSON.stringify({ nonce: binding.nonce, email: normalizeEmail(binding.email) }), "utf8", ).toString("base64url"); const store = await cookies(); - store.set(OTP_NONCE_COOKIE, encoded, { + store.set(name, encoded, { httpOnly: true, sameSite: "lax", secure: true, @@ -82,6 +99,14 @@ export async function setOtpBinding(binding: OtpBinding): Promise { }); } +export function setOtpBinding(binding: OtpBinding): Promise { + return setBinding(OTP_NONCE_COOKIE, binding); +} + +export function setPasswordResetBinding(binding: OtpBinding): Promise { + return setBinding(PWRESET_NONCE_COOKIE, binding); +} + /** * Reads the binding, or `undefined` when there is none. * @@ -91,9 +116,9 @@ export async function setOtpBinding(binding: OtpBinding): Promise { * state — so a corrupted cookie can never surface as "wrong code" to someone * holding a correct one, and can never crash the sign-in callback. */ -export async function readOtpBinding(): Promise { +async function readBinding(name: string): Promise { const store = await cookies(); - const raw = store.get(OTP_NONCE_COOKIE)?.value; + const raw = store.get(name)?.value; if (!raw) { return undefined; } @@ -106,7 +131,23 @@ export async function readOtpBinding(): Promise { } } -export async function clearOtpBinding(): Promise { +export function readOtpBinding(): Promise { + return readBinding(OTP_NONCE_COOKIE); +} + +export function readPasswordResetBinding(): Promise { + return readBinding(PWRESET_NONCE_COOKIE); +} + +async function clearBinding(name: string): Promise { const store = await cookies(); - store.delete(OTP_NONCE_COOKIE); + store.delete(name); +} + +export function clearOtpBinding(): Promise { + return clearBinding(OTP_NONCE_COOKIE); +} + +export function clearPasswordResetBinding(): Promise { + return clearBinding(PWRESET_NONCE_COOKIE); } diff --git a/src/server/auth/password-reset-actions.ts b/src/server/auth/password-reset-actions.ts new file mode 100644 index 0000000..98b738c --- /dev/null +++ b/src/server/auth/password-reset-actions.ts @@ -0,0 +1,189 @@ +"use server"; + +import { signOut } from "@/server/auth/auth-config"; +import { confirmPasswordReset, requestPasswordResetCode } from "@/server/auth/backend-token-exchange"; +import { AUTH_ERROR_CODES } from "@/server/auth/contracts"; +import { + clearPasswordResetBinding, + normalizeEmail, + readPasswordResetBinding, + setPasswordResetBinding, +} from "@/server/auth/otp-nonce-cookie"; +import { clearActiveSession } from "@/server/auth/session-token"; +import { isPasswordWithinPolicy } from "@/domain/auth/sign-in-method"; + +/** + * Server actions behind the "forgot password" flow (RUK-289). + * + * Actions rather than BFF routes for the reason RUK-288 established: these + * write a cookie, and `/login` sits under `(public)`, which mounts neither + * React Query nor `sonner` — a `useMutation` there throws at runtime while + * `tsc` and the unit tests stay green. + */ + +export interface PasswordResetActionResult { + /** An `AUTH_ERROR_CODES` value the client renders in place, or undefined. */ + error?: string; + /** Set once the password is changed, so the sign-in form can confirm it. */ + done?: boolean; +} + +/** A backend failure's HTTP status, when it carried one. */ +function statusOf(error: unknown): number | undefined { + const status = (error as { status?: unknown } | null)?.status; + return typeof status === "number" ? status : undefined; +} + +/** + * Whether the backend's body named a specific code. + * + * Reads the `code` FIELD rather than substring-matching the raw text: a + * whitespace difference in the encoder (`"code": "..."`) would defeat a + * substring match, and it would fail open into the generic collapse — showing + * sign-in recovery copy on a reset screen. Never reads `message`, which is + * prose and is not a contract. + */ +function codeIs(error: unknown, code: string): boolean { + const body = (error as { responseBody?: unknown } | null)?.responseBody; + if (typeof body !== "string") return false; + try { + return (JSON.parse(body) as { code?: unknown }).code === code; + } catch { + return false; + } +} + +/** + * Step one: ask the backend to mail a reset code, and bind it to this browser. + * + * The backend answers 202 for every outcome — unknown address, blocked user, + * even a malformed body — so this reports success identically in all of them. + * Anything else would turn the screen into an account-existence oracle. + * + * What it cannot promise, and the copy must not either: if the user has a live + * code with attempts left, the backend issues no new one and still answers 202. + * "We sent a code if that address is registered" stays true; "check your inbox + * for a new code" would not. + */ +export async function requestPasswordResetAction(email: string): Promise { + const trimmed = email.trim(); + if (!trimmed) { + return { error: "invalid_email" }; + } + + try { + const { session_nonce: nonce } = await requestPasswordResetCode(trimmed); + await setPasswordResetBinding({ nonce, email: trimmed }); + return {}; + } catch (error) { + const status = statusOf(error); + // Logged because the user is shown one uniform state by design, which + // leaves an operator with nothing to diagnose from. Status only — never the + // address, which is the thing the uniform answer exists to protect. + console.error("[password-reset] request failed", { status }); + + if (status === 429) { + return { error: AUTH_ERROR_CODES.otpRateLimited }; + } + // A dead or missing endpoint is a fact about the service, not about an + // account, so saying so plainly leaks nothing — and folding it into the + // anti-enumeration copy would tell every user their input was wrong during + // an outage. + // + // Discriminated the same way the confirm path does it rather than treating + // every non-429 as an outage: today this endpoint answers 202 or 429 and + // nothing else, but a future 4xx would otherwise be reported as "the + // service is down" to a user whose input was simply rejected. + if (status === undefined || status >= 500 || status === 404) { + return { error: AUTH_ERROR_CODES.passwordResetUnavailable }; + } + return { error: "invalid_email" }; + } +} + +/** + * Step two: redeem the code, install the new password, and end the session. + * + * The length check runs BEFORE the request and is not optional politeness: the + * backend hides a policy violation inside the same 401 as a wrong code, so + * without it a user with a short password is told the code they just typed is + * wrong. It also spends no attempt, which is why its result must not clear the + * binding. + */ +export async function confirmPasswordResetAction(args: { + email: string; + code: string; + newPassword: string; +}): Promise { + if (!isPasswordWithinPolicy(args.newPassword)) { + return { error: AUTH_ERROR_CODES.passwordPolicyViolation }; + } + + const binding = await readPasswordResetBinding(); + if (!binding || binding.email !== normalizeEmail(args.email)) { + // No binding means this browser cannot prove it asked for the code, so the + // request would be refused anyway. Answered locally with the reset flow's + // own mismatch code — not sign-in's, whose copy sends the user somewhere + // they are not trying to go. + await clearPasswordResetBinding(); + return { error: AUTH_ERROR_CODES.passwordResetSessionMismatch }; + } + + try { + await confirmPasswordReset({ + email: binding.email, + code: args.code, + sessionNonce: binding.nonce, + newPassword: args.newPassword, + }); + } catch (error) { + const status = statusOf(error); + // Status only. A binding was necessarily present — the function returns + // early without one — so logging that fact would be a constant dressed as a + // variable. The address and the code are never logged: they are what the + // uniform 202 exists to protect. + console.error("[password-reset] confirm failed", { status }); + + if (status === 429) { + return { error: AUTH_ERROR_CODES.otpRateLimited }; + } + if (status === undefined || status >= 500 || status === 404) { + return { error: AUTH_ERROR_CODES.passwordResetUnavailable }; + } + if (codeIs(error, "otp_session_mismatch")) { + await clearPasswordResetBinding(); + return { error: AUTH_ERROR_CODES.passwordResetSessionMismatch }; + } + // Everything else is the deliberate collapse: wrong code, expired, + // attempts exhausted. The binding is KEPT — attempts may remain, and + // discarding a still-usable code is worse than a retry. + return { error: AUTH_ERROR_CODES.passwordResetFailed }; + } + + // Past this point the password IS changed and every session is revoked. The + // teardown below can fail; the confirmation cannot be conditional on it. + await clearPasswordResetBinding(); + + try { + // Without this the NextAuth cookie outlives the backend session: `proxy.ts` + // still reads a session, `/login` bounces the user to `/`, and every + // request 401s on a dead token. + await signOut({ redirect: false }); + await clearActiveSession(); + } catch (error) { + // Status only, like the two log lines above. Logging the error OBJECT here + // would render `BackendAuthError.responseBody` — the backend's raw response + // text — and the refresh path's body is a token pair, so a failed teardown + // would write a live refresh token into the application log. + console.error("[password-reset] post-reset session teardown failed", { + status: statusOf(error), + }); + } + + return { done: true }; +} + +/** Abandons the reset flow so the user can start again with another address. */ +export async function abandonPasswordResetAction(): Promise { + await clearPasswordResetBinding(); +} diff --git a/tests/contracts/contract-gaps.test.ts b/tests/contracts/contract-gaps.test.ts index f245b10..96b0e3a 100644 --- a/tests/contracts/contract-gaps.test.ts +++ b/tests/contracts/contract-gaps.test.ts @@ -353,6 +353,23 @@ describe("registry — refuted claims stay refuted", () => { // `null` is a VALUE — "not set". Absence would be the gap, and it is not. expect("timezone" in fixture("me.json")).toBe(true); }); + + it("`password_set` is NOT yet a key on /me (RUK-289 gap, still open)", () => { + // The mirror of the assertion above, and the only executable check the + // `password_set` registry row has. + // + // It is green today because the backend that serves this field is on an + // unmerged branch, so the recorded fixture predates it. It goes RED on the + // day someone re-records `me.json` against a backend that sends the field — + // which is exactly the day the gap closes, the registry row is owed + // deletion, and this assertion is owed deletion with it. + // + // Written this way round deliberately. The pass-through assertion in + // `me.contract.test.ts` cannot do this job: it compares the route's echo + // against the same fixture that fed the mock, so it is a tautology on key + // sets and stays green whatever the fixture contains. + expect("password_set" in fixture("me.json")).toBe(false); + }); }); describe("the registry file itself", () => { diff --git a/tests/contracts/me-password.contract.test.ts b/tests/contracts/me-password.contract.test.ts new file mode 100644 index 0000000..54bac27 --- /dev/null +++ b/tests/contracts/me-password.contract.test.ts @@ -0,0 +1,182 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * Contract test — `POST /api/me/password` → backend `POST /api/v1/me/password`. + * RUK-289, SPEC §1.3, §3.3, §3.4. + * + * **Why there is no recorded fixture here, and why that does not excuse the + * test.** `scripts/refresh-fixtures.mjs` issues GET only and refuses any + * non-2xx, while this seam is a POST whose success is `204` with an empty body + * and whose interesting shapes are the error envelopes the recorder rejects + * (SPEC §7.3). So the contract policy's four questions are answered against a + * stubbed `fetch` rather than a fixture: + * + * 1. are the fields forwarded — including the two whose ABSENCE is meaningful? + * 2. does the response reach the client unchanged (204 stays 204)? + * 3. does a backend error stay an error, rather than degrading into success? + * 4. is the expectation independent of the thing it checks — the assertions + * read the request off the `fetch` stub, not off the arguments passed in. + * + * The seam matters more than most: SPEC §7.3 lists "a wrong `current_password` + * moving off 401" as a drift no automated check would catch, and this is the + * check that catches it. It is here rather than only in the unit suite because + * `npm run test:contracts` is what a reviewer reads as "the contracts are + * green", and a route absent from it is a route that claim does not cover. + */ + +const readActiveSession = vi.fn(); +vi.mock("@/server/auth/session-token", () => ({ + readActiveSession: () => readActiveSession(), +})); +vi.mock("@/server/backend/security/csrf", () => ({ isSameOriginRequest: () => true })); + +const { POST } = await import("@/app/api/me/password/route"); + +const fetchMock = vi.fn(); + +beforeEach(() => { + vi.stubGlobal("fetch", fetchMock); + fetchMock.mockReset(); + process.env.MAINTMODE_API_BASE_URL = "http://backend.test/maintmode"; + readActiveSession.mockResolvedValue({ + accessToken: "access-1", + refreshToken: "refresh-1", + accessTokenExpiresAt: Date.now() + 600_000, + }); +}); + +afterEach(() => vi.unstubAllGlobals()); + +function backendAnswers(status: number, body = "") { + fetchMock.mockResolvedValue({ + ok: status >= 200 && status < 300, + status, + statusText: `status ${status}`, + text: async () => body, + }); +} + +function post(body: unknown) { + return new Request("https://app.test/api/me/password", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); +} + +/** What actually went on the wire, read from the stub rather than the input. */ +function wireRequest() { + const [url, init] = fetchMock.mock.calls[0]; + return { url: String(url), init, body: JSON.parse(init.body) as Record }; +} + +describe("me/password — request forwarding", () => { + it("posts to the auth base's password path with a bearer credential", async () => { + backendAnswers(204); + + await POST(post({ current_password: "old", new_password: "a-long-enough-password" })); + + const { url, init } = wireRequest(); + expect(url).toContain("/api/v1/me/password"); + expect(init.method).toBe("POST"); + expect(init.headers.authorization).toBe("Bearer access-1"); + }); + + it("forwards refresh_token, which is what keeps the caller signed in", async () => { + backendAnswers(204); + + await POST(post({ current_password: "old", new_password: "a-long-enough-password" })); + + // Dropping it makes the backend revoke every session including this one, so + // the operator is signed out by their own success (SPEC §1.3). + expect(wireRequest().body).toEqual({ + current_password: "old", + new_password: "a-long-enough-password", + refresh_token: "refresh-1", + }); + }); + + it("omits current_password as a KEY when the caller sent none", async () => { + backendAnswers(204); + + await POST(post({ new_password: "a-long-enough-password" })); + + // Absent and empty are different requests: the backend rejects the field + // outright for an account that has no password. + expect("current_password" in wireRequest().body).toBe(false); + }); +}); + +describe("me/password — response pass-through", () => { + it("hands 204 to the client as 204 with no body", async () => { + backendAnswers(204); + + const response = await POST(post({ new_password: "a-long-enough-password" })); + + expect(response.status).toBe(204); + await expect(response.text()).resolves.toBe(""); + }); +}); + +describe("me/password — a backend error stays an error", () => { + // The degradation this guards against is not an empty list but a false + // success: reporting 2xx for a password the backend never changed. + it("does not turn a backend failure into a success", async () => { + for (const status of [400, 401, 403, 500, 503]) { + fetchMock.mockReset(); + backendAnswers(status, '{"code":"unauthorized"}'); + + const response = await POST(post({ new_password: "a-long-enough-password" })); + + expect(response.status).not.toBe(204); + expect(response.status).toBeGreaterThanOrEqual(400); + } + }); + + // SPEC §7.3's drift #2, pinned. If the backend moves a wrong current password + // off 401, this fails — and without it the change would reach the generic + // mapper, which answers AUTH_REQUIRED and signs the operator out with no + // message on their most common mistake. + it("keeps a wrong current password renderable rather than signing the user out", async () => { + backendAnswers(401, '{"code":"unauthorized","message":"invalid credentials"}'); + + const response = await POST(post({ current_password: "wrong", new_password: "a-long-enough-password" })); + + expect(response.status).toBe(422); + await expect(response.json()).resolves.toMatchObject({ code: "WRONG_CURRENT_PASSWORD" }); + }); + + it("distinguishes a stale refresh token, which changed nothing", async () => { + backendAnswers(401, '{"code":"unauthorized"}'); + + const response = await POST(post({ new_password: "a-long-enough-password" })); + + expect(response.status).toBe(409); + await expect(response.json()).resolves.toMatchObject({ code: "SESSION_STALE" }); + }); + + it("surfaces the backend's message field on a 400, not its envelope", async () => { + backendAnswers( + 400, + '{"code":"invalid request","message":"validation error: password does not meet the length policy"}', + ); + + const response = await POST(post({ new_password: "a-long-enough-password" })); + + expect(response.status).toBe(400); + // The message, not the whole body: echoing the envelope would put raw JSON + // in the operator's form, and would paste a proxy's HTML page in whole. + // Read for display only — nothing branches on it. + await expect(response.json()).resolves.toMatchObject({ + error: "validation error: password does not meet the length policy", + }); + }); + + it("answers an unreachable backend as an outage", async () => { + fetchMock.mockRejectedValue(new Error("connect ECONNREFUSED")); + + const response = await POST(post({ new_password: "a-long-enough-password" })); + + expect(response.status).toBe(503); + }); +});