diff --git a/.changeset/admin-step-up-authenticator-app.md b/.changeset/admin-step-up-authenticator-app.md new file mode 100644 index 0000000..083ad54 --- /dev/null +++ b/.changeset/admin-step-up-authenticator-app.md @@ -0,0 +1,7 @@ +--- +"seamless-cli": minor +--- + +Admin writes step up with an authenticator app code, which the auth server requires from v0.20 (fells-code/seamless-auth-api#490). When an instance answers `403 step_up_required`, the CLI asks for the code, verifies it with `POST /totp/verify-mfa` on the same session, and retries the write once. This covers `config set`, `config apply`, `config oauth-providers add|update|remove`, `users delete`, `users prepare-device-replacement`, the `org` and `org members` writes, and every batch of `migrate`. A wrong code can be re-entered up to three times; a lockout, a rate limit, and an account with no authenticator app enrolled each get their own message instead of the old "requires an admin role" one. Without a terminal, `SEAMLESS_TOTP_CODE` supplies the code for the first step-up of a run. + +Breaking: `users prepare-device-replacement` now requires `--proofing ` and `--evidence-ref ` (with an optional `--approver `), because the auth server rejects a device replacement that does not record how the user's identity was established. diff --git a/AGENTS.md b/AGENTS.md index 56c30ee..ba603fc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -202,6 +202,12 @@ Templates are not in this repo — they live in the `seamless-templates` monorep `.loose()`d: a key the schema does not know is kept (and reaches `--json` output) rather than stripped, or rejected by a strict schema such as `ApiUserSchema`. A response missing a required field fails with the path that tripped it. See [src/core/admin.ts](src/core/admin.ts). +- **Admin writes step up in the client, not in each wrapper.** From auth server v0.20 an admin write + answers `403 step_up_required` until the session has a fresh step-up. `createAuthClient` gives + every instance client a handler ([src/core/stepUp.ts](src/core/stepUp.ts)) that verifies an + authenticator app code on the same session and retries the request once, so a new admin call + needs nothing extra. Wrappers still map a 403 with `forbidden(res)`, which tells a missing + step-up apart from a missing admin role. - **Shared test fixtures** live in `src/**/*.fixtures.ts` (for example [src/core/admin.fixtures.ts](src/core/admin.fixtures.ts)). That suffix is excluded from the build and from coverage; a `*.test.ts` helper would instead run as an empty test file. diff --git a/README.md b/README.md index 2130349..29e758f 100644 --- a/README.md +++ b/README.md @@ -628,7 +628,7 @@ Manage users and organizations from the terminal (requires an admin role). seamless users list --limit 50 --offset 0 seamless users delete # confirms first seamless users credentials # registered credentials for a user -seamless users prepare-device-replacement # admin-assisted recovery +seamless users prepare-device-replacement --proofing in_person --evidence-ref CASE-42 seamless org list seamless org create "Acme Inc" --slug acme @@ -643,6 +643,34 @@ seamless org members remove A non-admin user receives a clear permission error rather than a stack trace. +`prepare-device-replacement` clears a user's credentials so they can enroll a new device, which is +the step social engineering aims at, so the instance records how their identity was established. +`--proofing` is `in_person` or `remote_exception`, `--evidence-ref` is a ticket or case number for +the audit trail (never personal data), and `--approver` names who approved a remote exception. + +### Step-up for admin writes + +From auth server v0.20, every admin write needs a fresh step-up on the session: `config set`, +`config apply`, `config oauth-providers add|update|remove`, `users delete`, +`users prepare-device-replacement`, the `org` and `org members` writes, and `migrate`. When the +instance asks for one, the CLI prompts for the code from your authenticator app, verifies it on the +same session (`POST /totp/verify-mfa`), and retries the write once. + +```text +This action needs a fresh step-up on your session. +◆ Enter the code from your authenticator app +``` + +- A wrong or already used code can be re-entered, up to three attempts. +- A locked account or a rate limit stops the command and says how long to wait. +- An account with no authenticator app enrolled cannot step up from the CLI. Enroll one from the + admin dashboard or your app, or make the change from the dashboard. +- The step-up lasts five minutes. A long `migrate` run that outlasts it asks for a new code before + the next batch, and one that cannot get one stops with a report of the batches already applied. + +Without a terminal, set `SEAMLESS_TOTP_CODE` to a current code. It is used for the first step-up of +the run only: a code is single use, so a second one needs a terminal. + ### Token storage and the keychain The refresh token is the durable secret, so sessions are stored in the operating system keychain, @@ -702,6 +730,8 @@ The prompts that gather input rather than confirm an action are answered by the commands already take, for example `seamless login --identifier you@example.com` and `seamless profile add prod --instance-url https://auth.example.com`. The one-time code at login is the exception: it only exists after the code is sent, so that step genuinely needs a terminal. +The step-up code for an admin write is answered by `SEAMLESS_TOTP_CODE` (see +[Step-up for admin writes](#step-up-for-admin-writes)). --- diff --git a/src/commands/helpTopics.ts b/src/commands/helpTopics.ts index de5e2ab..41a4331 100644 --- a/src/commands/helpTopics.ts +++ b/src/commands/helpTopics.ts @@ -434,6 +434,12 @@ session (or --all) prompts for confirmation and then clears local tokens. heading: "config ", body: `Read and write the instance system configuration (requires an admin role). +Writes need a fresh step-up on your session (auth server v0.20 and later). +The CLI asks for a code from your authenticator app, verifies it, and +retries the write once. Without a terminal, set SEAMLESS_TOTP_CODE to a +current code; it is used once per run. An account with no authenticator app +enrolled must enroll one (dashboard or app) or use the dashboard. + config get [key] [--json] • Print the whole config or a single key @@ -478,6 +484,12 @@ config oauth-providers heading: "users ", body: `Admin user management (requires an admin role). +Writes need a fresh step-up on your session (auth server v0.20 and later). +The CLI asks for a code from your authenticator app, verifies it, and +retries the write once. Without a terminal, set SEAMLESS_TOTP_CODE to a +current code; it is used once per run. An account with no authenticator app +enrolled must enroll one (dashboard or app) or use the dashboard. + users list [--limit ] [--offset ] [--json] • List users, 50 at a time by default • --limit is 1 to 100, --offset is 0 or more; total counts every user, not @@ -486,8 +498,14 @@ users delete [--force] • Delete a user (asks for confirmation) users credentials [--json] • Show a user's registered credentials -users prepare-device-replacement [--force] [--keep-sessions] [--keep-passkeys] [--keep-totp] - • Admin-assisted account recovery (needs an elevated session) +users prepare-device-replacement --proofing --evidence-ref + [--approver ] [--force] [--keep-sessions] [--keep-passkeys] [--keep-totp] + • Admin-assisted account recovery: revokes sessions, removes passkeys and + disables TOTP unless kept + • --proofing is how you established the user's identity: in_person, or + remote_exception under an approved exception + • --evidence-ref is a ticket or case number for the audit trail (no + personal data); --approver names who approved a remote exception --force • Skips the confirmation on delete and prepare-device-replacement (--yes and @@ -507,6 +525,9 @@ users prepare-device-replacement [--force] [--keep-sessions] [--keep-passke heading: "migrate csv ", body: `Import users from a CSV export into the instance (requires an admin role). Runs as a dry run unless --apply is passed, and writes a report either way. +Like every admin write, it needs a fresh step-up (see seamless help users). +A long run that outlasts the step-up window asks for a new code, and one +that cannot get it stops with a report of the batches already applied. Columns are found by header name: email (required), externalId (or id, employee id), phone, roles, and organizations (or department). Multi-value @@ -580,6 +601,12 @@ and guests are skipped and listed in the report. "org , org members ", body: `Admin organization management (requires an admin role). +Writes need a fresh step-up on your session (auth server v0.20 and later). +The CLI asks for a code from your authenticator app, verifies it, and +retries the write once. Without a terminal, set SEAMLESS_TOTP_CODE to a +current code; it is used once per run. An account with no authenticator app +enrolled must enroll one (dashboard or app) or use the dashboard. + org list [--limit ] [--offset ] [--search ] [--json] • Lists 50 at a time; --limit is 1 to 100, --offset is 0 or more • --search matches the name and slug, and total counts every match rather diff --git a/src/commands/users.test.ts b/src/commands/users.test.ts index 53e8bce..f21ad51 100644 --- a/src/commands/users.test.ts +++ b/src/commands/users.test.ts @@ -313,6 +313,8 @@ describe("runUsers credentials", () => { }); }); +const PROOF = ["--proofing", "in_person", "--evidence-ref", "CASE-42"]; + describe("runUsers prepare-device-replacement", () => { it("prints usage and exits 1 when no id is given", async () => { await expect( @@ -325,14 +327,14 @@ describe("runUsers prepare-device-replacement", () => { it("cancels when the prompt is cancelled", async () => { vi.mocked(isCancel).mockReturnValue(true); - await runUsers(["prepare-device-replacement", "u1"]); + await runUsers(["prepare-device-replacement", "u1", ...PROOF]); expect(logs()).toContain("Cancelled."); expect(vi.mocked(prepareDeviceReplacement)).not.toHaveBeenCalled(); }); it("cancels when the user declines", async () => { vi.mocked(confirm).mockResolvedValue(false); - await runUsers(["prepare-device-replacement", "u1"]); + await runUsers(["prepare-device-replacement", "u1", ...PROOF]); expect(logs()).toContain("Cancelled."); expect(vi.mocked(prepareDeviceReplacement)).not.toHaveBeenCalled(); }); @@ -344,7 +346,7 @@ describe("runUsers prepare-device-replacement", () => { removedCredentials: 1, disabledTotpCredentials: 1, }); - await runUsers(["prepare-device-replacement", "u1"]); + await runUsers(["prepare-device-replacement", "u1", ...PROOF]); expect(vi.mocked(confirm)).toHaveBeenCalledWith({ message: "Prepare device replacement for u1? This will revoke all sessions, remove passkeys, disable TOTP.", @@ -354,6 +356,7 @@ describe("runUsers prepare-device-replacement", () => { revokeSessions: true, removePasskeys: true, disableTotp: true, + proofing: { method: "in_person", evidenceRef: "CASE-42" }, }); expect(logs()).toContain("Prepared device replacement for u1."); expect(logs()).toContain("Revoked sessions: 2, removed credentials: 1, disabled TOTP: 1"); @@ -369,6 +372,7 @@ describe("runUsers prepare-device-replacement", () => { await runUsers([ "prepare-device-replacement", "u1", + ...PROOF, "--keep-sessions", "--keep-passkeys", "--keep-totp", @@ -381,9 +385,53 @@ describe("runUsers prepare-device-replacement", () => { revokeSessions: false, removePasskeys: false, disableTotp: false, + proofing: { method: "in_person", evidenceRef: "CASE-42" }, }); expect(logs()).toContain("Revoked sessions: 0, removed credentials: 0, disabled TOTP: 0"); }); + it("records a remote exception with its approver", async () => { + vi.mocked(prepareDeviceReplacement).mockResolvedValue({ + userId: "u1", + revokedSessions: 0, + removedCredentials: 0, + disabledTotpCredentials: 0, + }); + await runUsers([ + "prepare-device-replacement", + "u1", + "--proofing=remote_exception", + "--evidence-ref=INC-7", + "--approver", + "security-lead", + "--force", + ]); + expect(vi.mocked(prepareDeviceReplacement)).toHaveBeenCalledWith(fakeClient, "u1", { + revokeSessions: true, + removePasskeys: true, + disableTotp: true, + proofing: { + method: "remote_exception", + evidenceRef: "INC-7", + approver: "security-lead", + }, + }); + }); + + it.each([ + ["no proofing at all", ["u1"], "--proofing is required"], + ["an unknown proofing method", ["u1", "--proofing", "video", "--evidence-ref", "C-1"], "--proofing is required"], + ["no evidence reference", ["u1", "--proofing", "in_person"], "--evidence-ref is required"], + ["a blank evidence reference", ["u1", "--proofing", "in_person", "--evidence-ref", " "], "--evidence-ref is required"], + ["a blank approver", ["u1", ...["--proofing", "in_person", "--evidence-ref", "C-1"], "--approver", " "], "--approver must be"], + ])("refuses %s before asking anything", async (_label, args, message) => { + await expect( + runUsers(["prepare-device-replacement", ...args]), + ).rejects.toBeInstanceOf(ExitError); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining(message)); + expect(logs()).toContain("--proofing --evidence-ref "); + expect(vi.mocked(confirm)).not.toHaveBeenCalled(); + expect(vi.mocked(prepareDeviceReplacement)).not.toHaveBeenCalled(); + }); }); describe("users --force", () => { @@ -402,7 +450,7 @@ describe("users --force", () => { it("prepares a device replacement without confirming when forced", async () => { vi.mocked(prepareDeviceReplacement).mockResolvedValue({} as never); - await runUsers(["prepare-device-replacement", "u1", "--force"]); + await runUsers(["prepare-device-replacement", "u1", ...PROOF, "--force"]); expect(vi.mocked(confirm)).not.toHaveBeenCalled(); expect(vi.mocked(prepareDeviceReplacement)).toHaveBeenCalled(); diff --git a/src/commands/users.ts b/src/commands/users.ts index bd93582..2d4cf31 100644 --- a/src/commands/users.ts +++ b/src/commands/users.ts @@ -1,3 +1,4 @@ +import { RecoveryProofingSchema } from "@seamless-auth/types"; import kleur from "kleur"; import { extractFlag } from "../core/args.js"; import { createAuthClient, type AuthClient } from "../core/authClient.js"; @@ -126,22 +127,51 @@ async function usersCredentials( } } +const DEVICE_REPLACEMENT_USAGE = + "seamless users prepare-device-replacement --proofing --evidence-ref [--approver ]"; + +// The instance records how the user's identity was established before it clears +// their credentials, so the request cannot be sent without it. +const PROOFING_ERRORS: Record = { + method: + "--proofing is required: in_person when you verified the user face to face, remote_exception when you did it remotely under an approved exception.", + evidenceRef: + "--evidence-ref is required (up to 200 characters): a ticket or case number that records the proofing. It is written to the audit trail, so do not put personal data in it.", + approver: "--approver must be 1 to 200 characters.", +}; + async function usersPrepareDeviceReplacement( client: AuthClient, rest: string[], ): Promise { - const id = rest.find((arg) => !arg.startsWith("--")); + const method = extractFlag(rest, "proofing"); + const evidence = extractFlag(method.rest, "evidence-ref"); + const approver = extractFlag(evidence.rest, "approver"); + const operands = approver.rest; + + const id = operands.find((arg) => !arg.startsWith("--")); if (!id) { - console.error( - kleur.red("Usage: seamless users prepare-device-replacement "), - ); + console.error(kleur.red(`Usage: ${DEVICE_REPLACEMENT_USAGE}`)); + process.exit(1); + } + + const proofing = RecoveryProofingSchema.safeParse({ + method: method.value, + evidenceRef: evidence.value, + ...(approver.value !== undefined ? { approver: approver.value } : {}), + }); + if (!proofing.success) { + const field = String(proofing.error.issues[0].path[0]); + console.error(kleur.red(PROOFING_ERRORS[field] ?? PROOFING_ERRORS.method)); + console.log(`Usage: ${DEVICE_REPLACEMENT_USAGE}`); process.exit(1); } const opts = { - revokeSessions: !rest.includes("--keep-sessions"), - removePasskeys: !rest.includes("--keep-passkeys"), - disableTotp: !rest.includes("--keep-totp"), + revokeSessions: !operands.includes("--keep-sessions"), + removePasskeys: !operands.includes("--keep-passkeys"), + disableTotp: !operands.includes("--keep-totp"), + proofing: proofing.data, }; const actions = [ @@ -152,7 +182,7 @@ async function usersPrepareDeviceReplacement( const proceed = await confirmDestructive({ message: `Prepare device replacement for ${id}? This will ${actions.join(", ")}.`, - force: hasForceFlag(rest), + force: hasForceFlag(operands), remedy: "Pass --force to prepare the replacement without confirming.", }); if (!proceed) { diff --git a/src/core/admin.test.ts b/src/core/admin.test.ts index 0a040cc..bc7ca80 100644 --- a/src/core/admin.test.ts +++ b/src/core/admin.test.ts @@ -24,6 +24,7 @@ import { apiUser, apiUserDetail, } from "./admin.fixtures.js"; +import { STEP_UP_REQUIRED_MESSAGE, StepUpError } from "./stepUp.js"; function response(status: number, data: T | null): ApiResponse { return { ok: status >= 200 && status < 300, status, data, headers: new Headers() }; @@ -136,21 +137,54 @@ describe("users", () => { expect(detail.credentials).toHaveLength(2); }); - it("explains the step-up requirement for device replacement", async () => { - const { client, calls } = fakeClient(() => response(401, { error: "step up" })); + const proofing = { method: "in_person" as const, evidenceRef: "CASE-42" }; + + it("sends the device replacement options with the proofing record", async () => { + const counts = { + userId: "u1", + revokedSessions: 0, + removedCredentials: 0, + disabledTotpCredentials: 0, + }; + const { client, calls } = fakeClient(() => response(200, counts)); + await prepareDeviceReplacement(client, "u1", { + revokeSessions: true, + removePasskeys: true, + disableTotp: true, + proofing, + }); + expect(calls[0]).toEqual({ + method: "POST", + path: "/admin/users/u1/recovery/device-replacement", + body: { + revokeSessions: true, + removePasskeys: true, + disableTotp: true, + proofing, + }, + }); + }); + + it("reports a missing user on device replacement", async () => { + const { client } = fakeClient(() => response(404, { error: "User not found" })); await expect( prepareDeviceReplacement(client, "u1", { revokeSessions: true, removePasskeys: true, disableTotp: true, + proofing, }), - ).rejects.toThrow(/step-up/i); - expect(calls[0].path).toBe("/admin/users/u1/recovery/device-replacement"); - expect(calls[0].body).toEqual({ - revokeSessions: true, - removePasskeys: true, - disableTotp: true, - }); + ).rejects.toThrow("No user found with id u1 (404: User not found)."); + }); + + it("maps a step_up_required 403 to a StepUpError, not the admin-role message", async () => { + const { client } = fakeClient(() => + response(403, { error: "step_up_required", fresh: false, maxAgeSeconds: 300 }), + ); + const err = await deleteUser(client, "u1").catch((e: unknown) => e); + expect(err).toBeInstanceOf(StepUpError); + expect(err).toBeInstanceOf(PermissionError); + expect((err as Error).message).toBe(STEP_UP_REQUIRED_MESSAGE); }); it("maps 403 to a PermissionError", async () => { @@ -170,6 +204,7 @@ describe("users", () => { revokeSessions: true, removePasskeys: false, disableTotp: false, + proofing, }); expect(result).toEqual(counts); }); @@ -181,6 +216,7 @@ describe("users", () => { revokeSessions: false, removePasskeys: false, disableTotp: false, + proofing, }), ).rejects.toThrow(/Could not prepare device replacement/); }); diff --git a/src/core/admin.ts b/src/core/admin.ts index 32a3944..defb22c 100644 --- a/src/core/admin.ts +++ b/src/core/admin.ts @@ -12,6 +12,7 @@ import { UsersListResponseSchema, type AddOrganizationMemberRequest, type CreateOrganizationRequest, + type DeviceReplacementRecoveryRequest, type DeviceReplacementRecoveryResponse, type UpdateOrganizationMemberRequest, type UpdateOrganizationRequest, @@ -19,7 +20,8 @@ import { import { z } from "zod"; import type { AuthClient } from "./authClient.js"; import type { ApiResponse } from "./http.js"; -import { PermissionError } from "./systemConfig.js"; +import { PermissionError } from "./permissionError.js"; +import { forbidden } from "./stepUp.js"; export { PermissionError }; @@ -116,7 +118,7 @@ async function call( } res = await client.request(path, init); } - if (res.status === 403) throw new PermissionError(); + if (res.status === 403) throw forbidden(res); return res; } @@ -166,31 +168,20 @@ export async function getUserDetail( return parse(UserDetailSchema, res.data, "a user"); } -export interface DeviceReplacementOptions { - revokeSessions: boolean; - removePasskeys: boolean; - disableTotp: boolean; -} +export type DeviceReplacementOptions = DeviceReplacementRecoveryRequest; export async function prepareDeviceReplacement( client: AuthClient, id: string, opts: DeviceReplacementOptions, ): Promise { - const res = await client.request( + const res = await call( + client, + "POST", `/admin/users/${encodeURIComponent(id)}/recovery/device-replacement`, - { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(opts), - }, + opts, ); if (res.status === 404) throw apiError(res, `No user found with id ${id}`); - if (res.status === 401 || res.status === 403) { - throw new PermissionError( - "Admin-assisted device replacement requires an elevated step-up admin session, which the CLI cannot establish yet. Use the dashboard for this action.", - ); - } if (!res.ok) throw apiError(res, "Could not prepare device replacement"); return parse( DeviceReplacementRecoveryResponseSchema, diff --git a/src/core/authClient.test.ts b/src/core/authClient.test.ts index a2ab3a3..e656719 100644 --- a/src/core/authClient.test.ts +++ b/src/core/authClient.test.ts @@ -586,3 +586,160 @@ describe("a headless session", () => { ); }); }); + +describe("step-up on admin writes", () => { + const stepUpRequired = () => + json({ error: "step_up_required", fresh: false, maxAgeSeconds: 300 }, 403); + + beforeEach(async () => { + await saveTokens(profile, { accessToken: "access", refreshToken: "refresh" }); + }); + + it("steps up the same session and retries the request once", async () => { + const calls = mockFetch([stepUpRequired, () => json({ success: true })]); + const stepUp = vi.fn(async () => {}); + + const client = await createAuthClient({ stepUp }); + const res = await client.request("/admin/users", { + method: "DELETE", + body: JSON.stringify({ userId: "u1" }), + }); + + expect(res.status).toBe(200); + expect(stepUp).toHaveBeenCalledWith(client); + expect(calls).toHaveLength(2); + expect(calls[1].init.method).toBe("DELETE"); + expect(calls[1].init.body).toBe(JSON.stringify({ userId: "u1" })); + expect(authHeader(calls[1])).toBe("Bearer access"); + }); + + it("returns a second refusal to the caller instead of looping", async () => { + const calls = mockFetch([stepUpRequired]); + const stepUp = vi.fn(async () => {}); + + const client = await createAuthClient({ stepUp }); + const res = await client.post("/admin/organizations", { name: "Acme" }); + + expect(res.status).toBe(403); + expect(stepUp).toHaveBeenCalledTimes(1); + expect(calls).toHaveLength(2); + }); + + it("steps up again when a later request finds the window lapsed", async () => { + mockFetch([ + stepUpRequired, + () => json({ ok: 1 }), + () => json({ ok: 2 }), + stepUpRequired, + () => json({ ok: 3 }), + ]); + const stepUp = vi.fn(async () => {}); + + const client = await createAuthClient({ stepUp }); + for (let i = 0; i < 3; i++) { + expect((await client.post("/admin/users/import", {})).ok).toBe(true); + } + + expect(stepUp).toHaveBeenCalledTimes(2); + }); + + it("shares one step-up between concurrent refusals", async () => { + mockFetch([stepUpRequired, stepUpRequired, () => json({ ok: true })]); + let release!: () => void; + const stepUp = vi.fn( + () => + new Promise((resolve) => { + release = resolve; + }), + ); + + const client = await createAuthClient({ stepUp }); + const both = Promise.all([client.post("/a"), client.post("/b")]); + await vi.waitFor(() => expect(stepUp).toHaveBeenCalled()); + release(); + const results = await both; + + expect(stepUp).toHaveBeenCalledTimes(1); + expect(results.every((r) => r.ok)).toBe(true); + }); + + it("does not retry when the step-up fails", async () => { + const calls = mockFetch([stepUpRequired]); + const stepUp = vi.fn(async () => { + throw new Error("no code"); + }); + + const client = await createAuthClient({ stepUp }); + await expect(client.post("/admin/organizations", {})).rejects.toThrow("no code"); + expect(calls).toHaveLength(1); + }); + + it("leaves a 403 for a missing admin role alone", async () => { + const calls = mockFetch([() => json({ error: "Forbidden" }, 403)]); + const stepUp = vi.fn(async () => {}); + + const client = await createAuthClient({ stepUp }); + const res = await client.get("/admin/users"); + + expect(res.status).toBe(403); + expect(stepUp).not.toHaveBeenCalled(); + expect(calls).toHaveLength(1); + }); + + it("returns a rejected code without refreshing or clearing the session", async () => { + const calls = mockFetch([() => json({ error: "totp_verification_failed" }, 401)]); + + const client = await createAuthClient({ stepUp: vi.fn() }); + const res = await client.post("/totp/verify-mfa", { code: "123456" }); + + expect(res.status).toBe(401); + expect(calls).toHaveLength(1); + expect((await getTokens(profile))?.refreshToken).toBe("refresh"); + }); + + it("steps up with the authenticator app by default", async () => { + vi.stubEnv("SEAMLESS_TOTP_CODE", "123456"); + const calls = mockFetch([ + stepUpRequired, + () => json({ enabled: true, verifiedAt: null, lastUsedAt: null }), + () => + json({ + message: "Success", + fresh: true, + method: "totp", + verifiedAt: "2026-10-28T00:00:00.000Z", + expiresAt: "2026-10-28T00:05:00.000Z", + maxAgeSeconds: 300, + }), + () => json({ success: true }), + ]); + + const client = await createAuthClient(); + const res = await client.post("/admin/organizations", { name: "Acme" }); + vi.unstubAllEnvs(); + + expect(res.ok).toBe(true); + expect(calls.map((c) => c.url)).toEqual([ + "https://auth.example.com/admin/organizations", + "https://auth.example.com/totp/status", + "https://auth.example.com/totp/verify-mfa", + "https://auth.example.com/admin/organizations", + ]); + expect(calls[2].init.body).toBe(JSON.stringify({ code: "123456" })); + }); + + it("never steps up a portal session", async () => { + process.env.SEAMLESS_PORTAL_AUTH_URL = "https://portal.example.com"; + const portal = { name: PORTAL_PROFILE_NAME, instanceUrl: "https://portal.example.com" }; + savePortalSession({ instanceUrl: portal.instanceUrl }); + await saveTokens(portal, { accessToken: "pa", refreshToken: "pr" }); + const calls = mockFetch([stepUpRequired]); + + const client = await createPortalClient(); + const res = await client.post("/anything"); + delete process.env.SEAMLESS_PORTAL_AUTH_URL; + + expect(res.status).toBe(403); + expect(calls).toHaveLength(1); + }); +}); diff --git a/src/core/authClient.ts b/src/core/authClient.ts index bfd7551..99b3420 100644 --- a/src/core/authClient.ts +++ b/src/core/authClient.ts @@ -12,6 +12,12 @@ import { type TokenBundle, } from "./keychain.js"; import { apiRequest, joinUrl, type ApiResponse } from "./http.js"; +import { + authenticatorAppStepUp, + isFactorRejection, + isStepUpRequired, + type StepUpHandler, +} from "./stepUp.js"; export class ReauthRequiredError extends Error { constructor(message: string) { @@ -102,7 +108,7 @@ export function tokensFromRefreshResponse( // profile. Portal calls use createPortalClient instead: the two sessions are // different accounts on different hosts and must never be interchanged. export async function createAuthClient( - opts: { profileFlag?: string } = {}, + opts: { profileFlag?: string; stepUp?: StepUpHandler } = {}, ): Promise { const profile = getActiveProfile(opts); if (!profile) { @@ -111,10 +117,14 @@ export async function createAuthClient( ); } - return createClientForProfile(profile, { - label: `profile "${profile.name}"`, - reauthCommand: `seamless profile login ${profile.name}`, - }); + return createClientForProfile( + profile, + { + label: `profile "${profile.name}"`, + reauthCommand: `seamless profile login ${profile.name}`, + }, + opts.stepUp ?? authenticatorAppStepUp(), + ); } // A client for the Seamless portal, used for control-plane calls @@ -143,6 +153,7 @@ interface ReauthCopy { async function createClientForProfile( profile: Profile, copy: ReauthCopy, + stepUp?: StepUpHandler, ): Promise { const tokens = await getTokens(profile); if (!tokens || !tokens.refreshToken) { @@ -220,9 +231,19 @@ async function createClientForProfile( return inFlight; }; - const authedFetch = async ( + let stepUpInFlight: Promise | null = null; + const runStepUp = (handler: StepUpHandler): Promise => { + if (!stepUpInFlight) { + stepUpInFlight = handler(client).finally(() => { + stepUpInFlight = null; + }); + } + return stepUpInFlight; + }; + + const send = async ( path: string, - init: RequestInit = {}, + init: RequestInit, ): Promise> => { const build = (): RequestInit => ({ ...init, @@ -236,7 +257,7 @@ async function createClientForProfile( const sentWith = session.accessToken; let res = await apiRequest(url, build()); - if (res.status === 401 && session.refreshToken) { + if (res.status === 401 && session.refreshToken && !isFactorRejection(res)) { // Only refresh if nothing else already did while this request was in flight. // Otherwise a burst of 401s against one stale token would each rotate again. if (session.accessToken === sentWith) { @@ -258,7 +279,19 @@ async function createClientForProfile( return res; }; - return { + // A write the instance refuses for want of a fresh step-up is retried once after + // the handler steps up the same session. A second refusal goes back to the caller. + const authedFetch = async ( + path: string, + init: RequestInit = {}, + ): Promise> => { + const res = await send(path, init); + if (!stepUp || !isStepUpRequired(res)) return res; + await runStepUp(stepUp); + return send(path, init); + }; + + const client: AuthClient = { profile, request: authedFetch, get: (path, init = {}) => authedFetch(path, { ...init, method: "GET" }), @@ -279,4 +312,5 @@ async function createClientForProfile( }); }, }; + return client; } diff --git a/src/core/migrate.test.ts b/src/core/migrate.test.ts index a350b4e..dfacf48 100644 --- a/src/core/migrate.test.ts +++ b/src/core/migrate.test.ts @@ -15,6 +15,8 @@ import { writeReport, type PreparedRow, } from "./migrate.js"; +import { PermissionError } from "./permissionError.js"; +import { STEP_UP_REQUIRED_MESSAGE, StepUpError } from "./stepUp.js"; const tmp: string[] = []; function tmpDir() { @@ -186,6 +188,57 @@ describe("runImport", () => { expect(err.outcomes).toHaveLength(200); expect(err.message).toMatch(/after 200 of 250 rows/); }); + + const stepUpRequired = { + ok: false, + status: 403, + data: { error: "step_up_required", fresh: false, maxAgeSeconds: 300 }, + }; + + it("reports a missing step-up on the first batch as a StepUpError", async () => { + const c = client([stepUpRequired]); + const err = await runImport(c, "csv", rowsOf(10), true).catch((e) => e); + + expect(err).toBeInstanceOf(StepUpError); + expect(err.message).toBe(STEP_UP_REQUIRED_MESSAGE); + }); + + it("keeps earlier batches when a later one still lacks a step-up", async () => { + const c = client([okBatch(200), stepUpRequired]); + const err = await runImport(c, "csv", rowsOf(250), false).catch((e) => e); + + expect(err).toBeInstanceOf(ImportBatchError); + expect(err.outcomes).toHaveLength(200); + expect(err.message).toBe( + `${STEP_UP_REQUIRED_MESSAGE} Stopped after 200 of 250 rows.`, + ); + }); + + it("keeps earlier batches when the step-up for a later one fails", async () => { + const c = client([okBatch(200)]); + c.request.mockRejectedValueOnce(new StepUpError("Too many code attempts.")); + const err = await runImport(c, "csv", rowsOf(250), false).catch((e) => e); + + expect(err).toBeInstanceOf(ImportBatchError); + expect(err.outcomes).toHaveLength(200); + expect(err.message).toBe("Too many code attempts. Stopped after 200 of 250 rows."); + }); + + it("passes a failure on the first batch through unchanged", async () => { + const c = client([]); + const cause = new StepUpError("Too many code attempts."); + c.request.mockRejectedValueOnce(cause); + + await expect(runImport(c, "csv", rowsOf(10), false)).rejects.toBe(cause); + }); + + it("maps a missing admin role to a PermissionError", async () => { + const c = client([{ ok: false, status: 403, data: { error: "Forbidden" } }]); + const err = await runImport(c, "csv", rowsOf(10), false).catch((e) => e); + + expect(err).toBeInstanceOf(PermissionError); + expect(err).not.toBeInstanceOf(StepUpError); + }); }); describe("reports", () => { diff --git a/src/core/migrate.ts b/src/core/migrate.ts index 043e584..8bf1952 100644 --- a/src/core/migrate.ts +++ b/src/core/migrate.ts @@ -8,10 +8,13 @@ import { type UserImportResult, type UserImportRow, } from "@seamless-auth/types"; -import { AdminApiError, PermissionError } from "./admin.js"; +import { AdminApiError } from "./admin.js"; import type { AuthClient } from "./authClient.js"; import type { CsvRecord } from "./csv.js"; import { toCsv } from "./csv.js"; +import { errorMessage } from "./errors.js"; +import type { ApiResponse } from "./http.js"; +import { forbidden } from "./stepUp.js"; export type MappedField = | "externalId" @@ -246,13 +249,28 @@ export async function runImport( for (let start = 0; start < rows.length; start += USER_IMPORT_MAX_ROWS) { const batch = rows.slice(start, start + USER_IMPORT_MAX_ROWS); - const res = await client.request("/admin/users/import", { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ source, dryRun, users: batch.map((r) => r.user) }), - }); + // A step-up can lapse between batches, and one that fails part way through + // must still leave a report of the rows that were applied. + const stopped = (err: unknown): unknown => + outcomes.length === 0 + ? err + : new ImportBatchError( + `${errorMessage(err)} Stopped after ${outcomes.length} of ${rows.length} rows.`, + outcomes, + ); + + let res: ApiResponse; + try { + res = await client.request("/admin/users/import", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ source, dryRun, users: batch.map((r) => r.user) }), + }); + } catch (err) { + throw stopped(err); + } - if (res.status === 403) throw new PermissionError(); + if (res.status === 403) throw stopped(forbidden(res)); if (res.status === 404 && outcomes.length === 0) { throw new AdminApiError( "This instance does not support user import. Upgrade the auth server to a release with POST /admin/users/import.", diff --git a/src/core/permissionError.ts b/src/core/permissionError.ts new file mode 100644 index 0000000..0e8ded8 --- /dev/null +++ b/src/core/permissionError.ts @@ -0,0 +1,8 @@ +export class PermissionError extends Error { + constructor( + message = "You do not have permission for this action. It requires an admin role on the instance.", + ) { + super(message); + this.name = "PermissionError"; + } +} diff --git a/src/core/stepUp.test.ts b/src/core/stepUp.test.ts new file mode 100644 index 0000000..8cb17fa --- /dev/null +++ b/src/core/stepUp.test.ts @@ -0,0 +1,361 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@clack/prompts", () => { + const CANCEL = Symbol("cancel"); + return { CANCEL, text: vi.fn(), isCancel: (v: unknown) => v === CANCEL }; +}); + +vi.mock("./tty.js", () => ({ isInteractive: vi.fn(() => true) })); + +import { CANCEL, text } from "@clack/prompts"; +import type { AuthClient } from "./authClient.js"; +import { CancelledError } from "./cancel.js"; +import type { ApiResponse } from "./http.js"; +import { PermissionError } from "./permissionError.js"; +import { + authenticatorAppStepUp, + forbidden, + isFactorRejection, + isStepUpRequired, + MAX_CODE_ATTEMPTS, + NO_AUTHENTICATOR_MESSAGE, + STEP_UP_REQUIRED_MESSAGE, + StepUpError, + TOTP_CODE_ENV, +} from "./stepUp.js"; +import { isInteractive } from "./tty.js"; + +function response( + status: number, + data: unknown, + headers: Record = {}, +): ApiResponse { + return { + ok: status >= 200 && status < 300, + status, + data, + headers: new Headers(headers), + }; +} + +const enrolled = response(200, { enabled: true, verifiedAt: null, lastUsedAt: null }); +const verified = response(200, { + message: "Success", + fresh: true, + method: "totp", + verifiedAt: "2026-10-28T00:00:00.000Z", + expiresAt: "2026-10-28T00:05:00.000Z", + maxAgeSeconds: 300, +}); +const wrongCode = response(401, { error: "totp_verification_failed" }); + +function fakeClient( + status: ApiResponse, + verifies: ApiResponse[], +): { client: AuthClient; codes: string[] } { + const codes: string[] = []; + let i = 0; + const client: AuthClient = { + profile: { name: "default", instanceUrl: "https://auth.example.com" }, + get: vi.fn(async (path: string) => { + expect(path).toBe("/totp/status"); + return status; + }) as AuthClient["get"], + post: vi.fn(async (path: string, body?: unknown) => { + expect(path).toBe("/totp/verify-mfa"); + codes.push((body as { code: string }).code); + return verifies[Math.min(i++, verifies.length - 1)]; + }) as AuthClient["post"], + request: vi.fn() as AuthClient["request"], + }; + return { client, codes }; +} + +let stderr: string[]; + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(text).mockReset(); + vi.mocked(isInteractive).mockReturnValue(true); + stderr = []; + vi.spyOn(process.stderr, "write").mockImplementation((chunk) => { + stderr.push(String(chunk)); + return true; + }); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +function answers(...values: unknown[]): void { + for (const value of values) vi.mocked(text).mockResolvedValueOnce(value as never); +} + +describe("response classification", () => { + it("tells step_up_required apart from a missing admin role", () => { + const stepUp = response(403, { error: "step_up_required", fresh: false }); + const role = response(403, { error: "Forbidden" }); + + expect(isStepUpRequired(stepUp)).toBe(true); + expect(isStepUpRequired(role)).toBe(false); + expect(isStepUpRequired(response(403, null))).toBe(false); + expect(isStepUpRequired(response(401, { error: "step_up_required" }))).toBe(false); + + expect(forbidden(stepUp)).toBeInstanceOf(StepUpError); + expect(forbidden(stepUp).message).toBe(STEP_UP_REQUIRED_MESSAGE); + const plain = forbidden(role); + expect(plain).toBeInstanceOf(PermissionError); + expect(plain).not.toBeInstanceOf(StepUpError); + expect(plain.message).toMatch(/requires an admin role/); + }); + + it("treats only a rejected code as a factor rejection", () => { + expect(isFactorRejection(wrongCode)).toBe(true); + expect(isFactorRejection(response(401, { error: "unauthorized" }))).toBe(false); + expect(isFactorRejection(response(400, { error: "totp_verification_failed" }))).toBe(false); + }); +}); + +describe("authenticatorAppStepUp: prompting", () => { + it("prompts for a code, sends it, and returns once verified", async () => { + answers("123456"); + const { client, codes } = fakeClient(enrolled, [verified]); + + await authenticatorAppStepUp({})(client); + + expect(codes).toEqual(["123456"]); + expect(stderr.join("")).toContain("needs a fresh step-up"); + const options = vi.mocked(text).mock.calls[0][0]; + expect(options.message).toBe("Enter the code from your authenticator app"); + expect(options.output).toBe(process.stderr); + }); + + it("strips the spaces an app shows inside a code", async () => { + answers(" 123 456 "); + const { client, codes } = fakeClient(enrolled, [verified]); + + await authenticatorAppStepUp({})(client); + + expect(codes).toEqual(["123456"]); + }); + + it("validates the shape of a code before sending it", async () => { + answers("123456"); + const { client } = fakeClient(enrolled, [verified]); + await authenticatorAppStepUp({})(client); + + const validate = vi.mocked(text).mock.calls[0][0].validate!; + expect(validate("123456")).toBeUndefined(); + expect(validate("123 456")).toBeUndefined(); + expect(validate("12345")).toMatch(/6 digits/); + expect(validate("abcdef")).toMatch(/6 digits/); + expect(validate(undefined)).toMatch(/6 digits/); + }); + + it("re-prompts after a wrong code and succeeds on a later attempt", async () => { + answers("111111", "222222"); + const { client, codes } = fakeClient(enrolled, [wrongCode, verified]); + + await authenticatorAppStepUp({})(client); + + expect(codes).toEqual(["111111", "222222"]); + expect(vi.mocked(text).mock.calls[1][0].message).toBe( + "Enter a new code from your authenticator app", + ); + expect(stderr.join("")).toContain("That code was not accepted"); + }); + + it(`gives up after ${MAX_CODE_ATTEMPTS} wrong codes`, async () => { + answers("111111", "222222", "333333", "444444"); + const { client, codes } = fakeClient(enrolled, [wrongCode]); + + const err = await authenticatorAppStepUp({})(client).catch((e: unknown) => e); + + expect(err).toBeInstanceOf(StepUpError); + expect((err as Error).message).toMatch(/None of the 3 codes was accepted/); + expect(codes).toHaveLength(MAX_CODE_ATTEMPTS); + }); + + it("reads a cancelled prompt as a cancellation", async () => { + answers(CANCEL); + const { client, codes } = fakeClient(enrolled, [verified]); + + await expect(authenticatorAppStepUp({})(client)).rejects.toBeInstanceOf( + CancelledError, + ); + expect(codes).toEqual([]); + }); + + it("fails without a terminal and names the environment variable", async () => { + vi.mocked(isInteractive).mockReturnValue(false); + const { client, codes } = fakeClient(enrolled, [verified]); + + const err = await authenticatorAppStepUp({})(client).catch((e: unknown) => e); + + expect(err).toBeInstanceOf(StepUpError); + expect((err as Error).message).toBe(STEP_UP_REQUIRED_MESSAGE); + expect((err as Error).message).toContain(TOTP_CODE_ENV); + expect(text).not.toHaveBeenCalled(); + expect(codes).toEqual([]); + }); +}); + +describe("authenticatorAppStepUp: enrollment", () => { + it("stops before asking when no authenticator app is enrolled", async () => { + const { client, codes } = fakeClient( + response(200, { enabled: false, verifiedAt: null, lastUsedAt: null }), + [verified], + ); + + const err = await authenticatorAppStepUp({})(client).catch((e: unknown) => e); + + expect(err).toBeInstanceOf(StepUpError); + expect((err as Error).message).toBe(NO_AUTHENTICATOR_MESSAGE); + expect(text).not.toHaveBeenCalled(); + expect(codes).toEqual([]); + }); + + it.each([ + ["an unreadable status", response(200, { something: "else" })], + ["a failed status call", response(500, { error: "boom" })], + ])("goes on to verify after %s", async (_label, status) => { + answers("123456"); + const { client, codes } = fakeClient(status, [verified]); + + await authenticatorAppStepUp({})(client); + + expect(codes).toEqual(["123456"]); + }); +}); + +describe("authenticatorAppStepUp: the environment code", () => { + it(`uses ${TOTP_CODE_ENV} without prompting`, async () => { + vi.mocked(isInteractive).mockReturnValue(false); + const { client, codes } = fakeClient(enrolled, [verified]); + + await authenticatorAppStepUp({ [TOTP_CODE_ENV]: " 654 321 " })(client); + + expect(codes).toEqual(["654321"]); + expect(text).not.toHaveBeenCalled(); + }); + + it("uses the environment code once, then needs a terminal", async () => { + vi.mocked(isInteractive).mockReturnValue(false); + const { client } = fakeClient(enrolled, [verified]); + const stepUp = authenticatorAppStepUp({ [TOTP_CODE_ENV]: "654321" }); + + await stepUp(client); + await expect(stepUp(client)).rejects.toThrow(STEP_UP_REQUIRED_MESSAGE); + }); + + it("names the variable when its code is rejected without a terminal", async () => { + vi.mocked(isInteractive).mockReturnValue(false); + const { client, codes } = fakeClient(enrolled, [wrongCode]); + + const err = await authenticatorAppStepUp({ [TOTP_CODE_ENV]: "654321" })( + client, + ).catch((e: unknown) => e); + + expect(err).toBeInstanceOf(StepUpError); + expect((err as Error).message).toMatch( + new RegExp(`The code in ${TOTP_CODE_ENV} was not accepted`), + ); + expect(codes).toEqual(["654321"]); + }); + + it("falls back to the prompt when the environment code is rejected in a terminal", async () => { + answers("222222"); + const { client, codes } = fakeClient(enrolled, [wrongCode, verified]); + + await authenticatorAppStepUp({ [TOTP_CODE_ENV]: "111111" })(client); + + expect(codes).toEqual(["111111", "222222"]); + }); + + it("refuses a malformed environment code without sending it", async () => { + const { client, codes } = fakeClient(enrolled, [verified]); + + await expect( + authenticatorAppStepUp({ [TOTP_CODE_ENV]: "abc" })(client), + ).rejects.toThrow(`${TOTP_CODE_ENV} is not a code from an authenticator app`); + expect(codes).toEqual([]); + }); + + it("ignores a blank environment variable", async () => { + answers("123456"); + const { client, codes } = fakeClient(enrolled, [verified]); + + await authenticatorAppStepUp({ [TOTP_CODE_ENV]: " " })(client); + + expect(codes).toEqual(["123456"]); + expect(text).toHaveBeenCalledTimes(1); + }); + + it("reads process.env by default", async () => { + vi.mocked(isInteractive).mockReturnValue(false); + vi.stubEnv(TOTP_CODE_ENV, "987654"); + const { client, codes } = fakeClient(enrolled, [verified]); + + await authenticatorAppStepUp()(client); + + expect(codes).toEqual(["987654"]); + vi.unstubAllEnvs(); + }); +}); + +describe("authenticatorAppStepUp: verify errors", () => { + async function failWith(res: ApiResponse): Promise { + answers("123456"); + const { client } = fakeClient(enrolled, [res]); + return authenticatorAppStepUp({})(client).catch((e: unknown) => e as Error); + } + + it.each([ + [1, "1 second"], + [45, "45 seconds"], + [60, "1 minute"], + [61, "2 minutes"], + [900, "15 minutes"], + ])("reports a lockout of %i seconds as %s", async (seconds, wait) => { + const err = await failWith( + response(423, { + error: "account_locked", + message: "Too many failed authentication attempts. Try again later.", + retryAfterSeconds: seconds, + }), + ); + expect(err).toBeInstanceOf(StepUpError); + expect(err.message).toBe( + `Your account is locked after too many failed attempts. Try again in ${wait}.`, + ); + }); + + it("reports a lockout with no wait as a few minutes", async () => { + const err = await failWith(response(423, { error: "account_locked" })); + expect(err.message).toMatch(/Try again in a few minutes\./); + }); + + it("reports a rate limit with its Retry-After header", async () => { + const err = await failWith(response(429, null, { "retry-after": "120" })); + expect(err).toBeInstanceOf(StepUpError); + expect(err.message).toBe("Too many code attempts. Wait 2 minutes and run the command again."); + }); + + it.each([ + ["no header", {}], + ["a blank header", { "retry-after": " " }], + ["a date header", { "retry-after": "Wed, 21 Oct 2026 07:28:00 GMT" }], + ])("reports a rate limit with %s as a few minutes", async (_label, headers) => { + const err = await failWith(response(429, { error: "Too many requests" }, headers)); + expect(err.message).toBe( + "Too many code attempts. Wait a few minutes and run the command again.", + ); + }); + + it("reports any other failure with its status", async () => { + const err = await failWith(response(500, { error: "boom" })); + expect(err).toBeInstanceOf(StepUpError); + expect(err.message).toBe("Could not verify the authenticator app code (500)."); + }); +}); diff --git a/src/core/stepUp.ts b/src/core/stepUp.ts new file mode 100644 index 0000000..4b5e1a6 --- /dev/null +++ b/src/core/stepUp.ts @@ -0,0 +1,192 @@ +import { isCancel, text } from "@clack/prompts"; +import { TotpStatusSchema, TotpVerifyRequestSchema } from "@seamless-auth/types"; +import kleur from "kleur"; +import type { AuthClient } from "./authClient.js"; +import { CancelledError } from "./cancel.js"; +import type { ApiResponse } from "./http.js"; +import { PermissionError } from "./permissionError.js"; +import { isInteractive } from "./tty.js"; + +export const TOTP_CODE_ENV = "SEAMLESS_TOTP_CODE"; + +export const MAX_CODE_ATTEMPTS = 3; + +export const STEP_UP_REQUIRED_MESSAGE = `This action needs a recent step-up: a code from your authenticator app, verified on this session within the last few minutes. Run the command in an interactive terminal to enter one, or set ${TOTP_CODE_ENV} to a current code.`; + +export const NO_AUTHENTICATOR_MESSAGE = + "This action needs a step-up with an authenticator app, and your account has none enrolled. Enroll one from the admin dashboard or your app, then run this again, or make this change from the dashboard."; + +// Raised for every step-up failure. It is a PermissionError so the commands that +// already report one print its message rather than a stack trace. +export class StepUpError extends PermissionError { + constructor(message: string) { + super(message); + this.name = "StepUpError"; + } +} + +export type StepUpHandler = (client: AuthClient) => Promise; + +function errorCode(data: unknown): string | undefined { + if (data && typeof data === "object") { + const error = (data as Record).error; + if (typeof error === "string") return error; + } + return undefined; +} + +// A missing admin role is also a 403, so only the error code tells the two apart. +export function isStepUpRequired(res: Pick): boolean { + return res.status === 403 && errorCode(res.data) === "step_up_required"; +} + +// A 401 that rejects a submitted code is an answer about the code, not about the +// access token, so it must not be treated as an expired session. +export function isFactorRejection(res: Pick): boolean { + return res.status === 401 && errorCode(res.data) === "totp_verification_failed"; +} + +export function forbidden(res: Pick): PermissionError { + return isStepUpRequired(res) + ? new StepUpError(STEP_UP_REQUIRED_MESSAGE) + : new PermissionError(); +} + +function describeWait(seconds: number | undefined): string { + if (seconds === undefined || !Number.isFinite(seconds) || seconds <= 0) { + return "a few minutes"; + } + if (seconds < 60) { + const s = Math.ceil(seconds); + return `${s} second${s === 1 ? "" : "s"}`; + } + const minutes = Math.ceil(seconds / 60); + return `${minutes} minute${minutes === 1 ? "" : "s"}`; +} + +function retryAfterSeconds(res: ApiResponse): number | undefined { + if (res.data && typeof res.data === "object") { + const value = (res.data as Record).retryAfterSeconds; + if (typeof value === "number") return value; + } + const header = res.headers.get("retry-after"); + return header === null || header.trim() === "" ? undefined : Number(header); +} + +// Authenticator apps often show a code as "123 456". +function normalizeCode(raw: string): string { + return raw.replace(/\s+/g, ""); +} + +const codeShape = TotpVerifyRequestSchema.shape.code; + +function isWellFormed(code: string): boolean { + return codeShape.safeParse(code).success; +} + +async function requireEnrolledAuthenticator(client: AuthClient): Promise { + const res = await client.get("/totp/status"); + // Only a definite "not enrolled" stops here. Anything else is left to the verify + // call, which reports its own failures. + if (!res.ok) return; + const parsed = TotpStatusSchema.loose().safeParse(res.data); + if (parsed.success && !parsed.data.enabled) { + throw new StepUpError(NO_AUTHENTICATOR_MESSAGE); + } +} + +/** + * Steps up the session with a code from the user's authenticator app, read from + * SEAMLESS_TOTP_CODE once and otherwise from a prompt. + * + * The environment code is used at most once per process. A code is single use, so + * a second step-up in the same run (a long migrate outlasting the window) needs a + * new one, which only a terminal can supply. + */ +export function authenticatorAppStepUp( + env: NodeJS.ProcessEnv = process.env, +): StepUpHandler { + let envCode = env[TOTP_CODE_ENV]?.trim() || undefined; + + const nextCode = async ( + attempt: number, + ): Promise<{ code: string; fromEnv: boolean }> => { + if (envCode !== undefined) { + const code = normalizeCode(envCode); + envCode = undefined; + if (!isWellFormed(code)) { + throw new StepUpError( + `${TOTP_CODE_ENV} is not a code from an authenticator app. Set it to the 6 digits the app shows.`, + ); + } + return { code, fromEnv: true }; + } + + if (!isInteractive()) throw new StepUpError(STEP_UP_REQUIRED_MESSAGE); + + if (attempt === 1) { + process.stderr.write( + `${kleur.yellow("This action needs a fresh step-up on your session.")}\n`, + ); + } + const answer = await text({ + message: + attempt === 1 + ? "Enter the code from your authenticator app" + : "Enter a new code from your authenticator app", + placeholder: "123456", + validate: (value) => + isWellFormed(normalizeCode(value ?? "")) + ? undefined + : "Enter the 6 digits your authenticator app shows", + // Keeps stdout clean for commands printing --json. + output: process.stderr, + }); + if (isCancel(answer)) throw new CancelledError(); + return { code: normalizeCode(answer), fromEnv: false }; + }; + + return async (client) => { + await requireEnrolledAuthenticator(client); + + for (let attempt = 1; ; attempt++) { + const { code, fromEnv } = await nextCode(attempt); + const res = await client.post("/totp/verify-mfa", { code }); + + if (res.ok) return; + + if (isFactorRejection(res)) { + if (attempt >= MAX_CODE_ATTEMPTS) { + throw new StepUpError( + `None of the ${MAX_CODE_ATTEMPTS} codes was accepted. Check that your device clock is correct and that the code is from this instance's entry in your authenticator app, then run the command again.`, + ); + } + if (fromEnv && !isInteractive()) { + throw new StepUpError( + `The code in ${TOTP_CODE_ENV} was not accepted. It may have expired or already been used; set a current one and run the command again.`, + ); + } + process.stderr.write( + `${kleur.red("That code was not accepted. Codes are single use and change every 30 seconds.")}\n`, + ); + continue; + } + + if (res.status === 423) { + throw new StepUpError( + `Your account is locked after too many failed attempts. Try again in ${describeWait(retryAfterSeconds(res))}.`, + ); + } + + if (res.status === 429) { + throw new StepUpError( + `Too many code attempts. Wait ${describeWait(retryAfterSeconds(res))} and run the command again.`, + ); + } + + throw new StepUpError( + `Could not verify the authenticator app code (${res.status}).`, + ); + } + }; +} diff --git a/src/core/systemConfig.test.ts b/src/core/systemConfig.test.ts index aa96b13..9952780 100644 --- a/src/core/systemConfig.test.ts +++ b/src/core/systemConfig.test.ts @@ -22,6 +22,7 @@ import { updateOAuthProvider, WRITABLE_KEYS, } from "./systemConfig.js"; +import { STEP_UP_REQUIRED_MESSAGE, StepUpError } from "./stepUp.js"; function response(status: number, data: T | null): ApiResponse { return { ok: status >= 200 && status < 300, status, data, headers: new Headers() }; @@ -150,6 +151,20 @@ describe("patchSystemConfig", () => { ).rejects.toBeInstanceOf(PermissionError); }); + it.each([ + ["a config patch", (c: AuthClient) => patchSystemConfig(c, { app_name: "x" })], + ["adding an OAuth provider", (c: AuthClient) => createOAuthProvider(c, { id: "google" })], + ["updating an OAuth provider", (c: AuthClient) => updateOAuthProvider(c, "google", {})], + ["removing an OAuth provider", (c: AuthClient) => deleteOAuthProvider(c, "google")], + ])("reports a missing step-up on %s as a StepUpError", async (_label, run) => { + const { client } = fakeClient(() => + response(403, { error: "step_up_required", fresh: false, maxAgeSeconds: 300 }), + ); + const err = await run(client).catch((e: unknown) => e); + expect(err).toBeInstanceOf(StepUpError); + expect((err as Error).message).toBe(STEP_UP_REQUIRED_MESSAGE); + }); + it("throws a ConfigApiError on other failures", async () => { const { client } = fakeClient(() => response(500, { error: "boom" })); await expect( diff --git a/src/core/systemConfig.ts b/src/core/systemConfig.ts index f350648..af76e3b 100644 --- a/src/core/systemConfig.ts +++ b/src/core/systemConfig.ts @@ -10,7 +10,11 @@ import { } from "@seamless-auth/types"; import { z } from "zod"; import type { AuthClient } from "./authClient.js"; +import { PermissionError } from "./permissionError.js"; import { scrubTokens } from "./redact.js"; +import { forbidden } from "./stepUp.js"; + +export { PermissionError }; // What an instance stores, typed with the shared schema's input side but every key // optional and others allowed: the instance may be older or newer than this CLI's @@ -67,15 +71,6 @@ const WRITABLE = new Set(WRITABLE_KEYS); const KNOWN = new Set(Object.keys(SystemConfigSchema.shape)); -export class PermissionError extends Error { - constructor( - message = "You do not have permission for this action. It requires an admin role on the instance.", - ) { - super(message); - this.name = "PermissionError"; - } -} - export class ConfigApiError extends Error { constructor(message: string) { super(message); @@ -89,7 +84,7 @@ export async function getSystemConfig( client: AuthClient, ): Promise { const res = await client.get("/system-config/admin"); - if (res.status === 403) throw new PermissionError(); + if (res.status === 403) throw forbidden(res); if (!res.ok) { throw new ConfigApiError(`Could not read system config (${res.status}).`); } @@ -104,7 +99,7 @@ export async function getSystemConfig( export async function getRoles(client: AuthClient): Promise { const res = await client.get("/system-config/roles"); - if (res.status === 403) throw new PermissionError(); + if (res.status === 403) throw forbidden(res); if (!res.ok) { throw new ConfigApiError(`Could not read roles (${res.status}).`); } @@ -132,7 +127,7 @@ export async function patchSystemConfig( body: JSON.stringify(patch), }); - if (res.status === 403) throw new PermissionError(); + if (res.status === 403) throw forbidden(res); if (res.status === 400) { const reason = res.data?.error ?? "Invalid configuration"; const details = res.data?.details @@ -174,7 +169,7 @@ function providerMutationError( action: string, id?: unknown, ): never { - if (res.status === 403) throw new PermissionError(); + if (res.status === 403) throw forbidden(res); const label = id ? ` "${String(id)}"` : ""; if (res.status === 404) { @@ -199,7 +194,7 @@ export async function listOAuthProviders( client: AuthClient, ): Promise { const res = await client.get(OAUTH_PROVIDERS_PATH); - if (res.status === 403) throw new PermissionError(); + if (res.status === 403) throw forbidden(res); if (!res.ok) { throw new ConfigApiError(`Could not list OAuth providers (${res.status}).`); }