From 239b1325ff89f197b43d3e847e60589d3347fc70 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 14:12:02 -0500 Subject: [PATCH 1/4] test: cover enrollment invite delivery through adopter messaging Parity scenarios for POST /admin/enrollment/invites across Express, Fastify and Next.js: the external delivery header and delivery service token when messaging is set, one message per results[].delivery with the payloads stripped, a failed delivery reported as delivery_failed while the rest send, and plain passthrough without messaging. Fails until #183 lands. --- packages/fastify/tests/parity.test.js | 136 ++++++++++++++++++++++++++ packages/nextjs/tests/parity.test.js | 136 ++++++++++++++++++++++++++ 2 files changed, 272 insertions(+) diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index 6c4c922..2f57af1 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -2037,3 +2037,139 @@ describe("fastify and express rotate a recovery session after enrollment", () => expect(viaAdapter.result).toEqual(viaExpressResult.result); }); }); + +// The API hands each invite back for the adopter's own messaging to send (#183). +describe("fastify and express deliver enrollment invites through adopter messaging", () => { + const originalFetch = global.fetch; + afterEach(() => { + global.fetch = originalFetch; + }); + + const invite = (to) => ({ + kind: "enrollment_invite_email", + to, + signInUrl: "https://app.example.com/login", + }); + + const EXTERNAL_RESPONSE = { + sent: 3, + skipped: 1, + remaining: 4, + results: [ + { userId: "u-1", status: "sent", delivery: invite("one@example.com") }, + { userId: "u-2", status: "skipped", reason: "already_enrolled" }, + { userId: "u-3", status: "sent", delivery: invite("three@example.com") }, + { userId: "u-4", status: "sent", delivery: invite("four@example.com") }, + ], + }; + + function invitesUpstream(body) { + return jest.fn(async (url) => + String(url).endsWith("/.well-known/jwks.json") + ? upstream(200, { keys: [jwk] }) + : upstream(200, body), + ); + } + + async function run(via, { body = EXTERNAL_RESPONSE, send, messaging = true } = {}) { + global.fetch = invitesUpstream(body); + const sendEnrollmentInviteEmail = jest.fn(send ?? (async () => {})); + const result = await via({ + method: "post", + path: "/admin/enrollment/invites", + cookie: accessCookie(), + payload: { organizationId: "org-1" }, + options: messaging ? { messaging: { handlers: { sendEnrollmentInviteEmail } } } : {}, + }); + const [, init] = global.fetch.mock.calls.find(([url]) => + String(url).endsWith("/admin/enrollment/invites"), + ); + return { result, init, sendEnrollmentInviteEmail }; + } + + async function both(scenario) { + const other = await run(viaFastify, scenario); + const expressRun = await run(viaExpress, scenario); + return { other, express: expressRun }; + } + + it("asks for external delivery with the delivery service token", async () => { + const runs = await both(); + + for (const { init } of [runs.other, runs.express]) { + expect(init.headers["x-seamless-auth-delivery-mode"]).toBe("external"); + expect(init.headers.Authorization).toBe(`Bearer ${ACCESS_TOKEN}`); + const serviceToken = jwt.decode( + init.headers["x-seamless-service-token"].replace(/^Bearer /, ""), + ); + expect(serviceToken.sub).toBe("seamless-auth-external-delivery"); + } + }); + + it("sends each invite and keeps addresses and links out of the response", async () => { + const runs = await both(); + + for (const { result, sendEnrollmentInviteEmail } of [runs.other, runs.express]) { + expect(sendEnrollmentInviteEmail.mock.calls.map(([input]) => input)).toEqual([ + { to: "one@example.com", signInUrl: "https://app.example.com/login" }, + { to: "three@example.com", signInUrl: "https://app.example.com/login" }, + { to: "four@example.com", signInUrl: "https://app.example.com/login" }, + ]); + expect(result).toEqual({ + status: 200, + body: { + sent: 3, + skipped: 1, + remaining: 4, + results: [ + { userId: "u-1", status: "sent" }, + { userId: "u-2", status: "skipped", reason: "already_enrolled" }, + { userId: "u-3", status: "sent" }, + { userId: "u-4", status: "sent" }, + ], + }, + cookies: [], + }); + } + expect(runs.other.result).toEqual(runs.express.result); + }); + + it("reports an invite that could not be sent and sends the rest", async () => { + const runs = await both({ + send: async ({ to }) => { + if (to === "three@example.com") throw new Error("smtp down"); + }, + }); + + for (const { result, sendEnrollmentInviteEmail } of [runs.other, runs.express]) { + expect(sendEnrollmentInviteEmail).toHaveBeenCalledTimes(3); + expect(result.status).toBe(200); + expect(result.body).toEqual({ + sent: 2, + skipped: 2, + remaining: 4, + results: [ + { userId: "u-1", status: "sent" }, + { userId: "u-2", status: "skipped", reason: "already_enrolled" }, + { userId: "u-3", status: "skipped", reason: "delivery_failed" }, + { userId: "u-4", status: "sent" }, + ], + }); + } + expect(runs.other.result).toEqual(runs.express.result); + }); + + it("leaves delivery to the auth API without messaging", async () => { + const body = { + sent: 1, + skipped: 0, + results: [{ userId: "u-1", status: "sent" }], + }; + const runs = await both({ body, messaging: false }); + + for (const { result, init } of [runs.other, runs.express]) { + expect(init.headers["x-seamless-auth-delivery-mode"]).toBeUndefined(); + expect(result).toEqual({ status: 200, body, cookies: [] }); + } + }); +}); diff --git a/packages/nextjs/tests/parity.test.js b/packages/nextjs/tests/parity.test.js index 859356a..3d48470 100644 --- a/packages/nextjs/tests/parity.test.js +++ b/packages/nextjs/tests/parity.test.js @@ -2239,3 +2239,139 @@ describe("next.js and express route guards agree", () => { expect(await res.json()).toEqual({ params: { id: "t-1" } }); }); }); + +// The API hands each invite back for the adopter's own messaging to send (#183). +describe("next and express deliver enrollment invites through adopter messaging", () => { + const originalFetch = global.fetch; + afterEach(() => { + global.fetch = originalFetch; + }); + + const invite = (to) => ({ + kind: "enrollment_invite_email", + to, + signInUrl: "https://app.example.com/login", + }); + + const EXTERNAL_RESPONSE = { + sent: 3, + skipped: 1, + remaining: 4, + results: [ + { userId: "u-1", status: "sent", delivery: invite("one@example.com") }, + { userId: "u-2", status: "skipped", reason: "already_enrolled" }, + { userId: "u-3", status: "sent", delivery: invite("three@example.com") }, + { userId: "u-4", status: "sent", delivery: invite("four@example.com") }, + ], + }; + + function invitesUpstream(body) { + return jest.fn(async (url) => + String(url).endsWith("/.well-known/jwks.json") + ? upstream(200, { keys: [jwk] }) + : upstream(200, body), + ); + } + + async function run(via, { body = EXTERNAL_RESPONSE, send, messaging = true } = {}) { + global.fetch = invitesUpstream(body); + const sendEnrollmentInviteEmail = jest.fn(send ?? (async () => {})); + const result = await via({ + method: "post", + path: "/admin/enrollment/invites", + cookie: accessCookie(), + payload: { organizationId: "org-1" }, + options: messaging ? { messaging: { handlers: { sendEnrollmentInviteEmail } } } : {}, + }); + const [, init] = global.fetch.mock.calls.find(([url]) => + String(url).endsWith("/admin/enrollment/invites"), + ); + return { result, init, sendEnrollmentInviteEmail }; + } + + async function both(scenario) { + const other = await run(viaNext, scenario); + const expressRun = await run(viaExpress, scenario); + return { other, express: expressRun }; + } + + it("asks for external delivery with the delivery service token", async () => { + const runs = await both(); + + for (const { init } of [runs.other, runs.express]) { + expect(init.headers["x-seamless-auth-delivery-mode"]).toBe("external"); + expect(init.headers.Authorization).toBe(`Bearer ${ACCESS_TOKEN}`); + const serviceToken = jwt.decode( + init.headers["x-seamless-service-token"].replace(/^Bearer /, ""), + ); + expect(serviceToken.sub).toBe("seamless-auth-external-delivery"); + } + }); + + it("sends each invite and keeps addresses and links out of the response", async () => { + const runs = await both(); + + for (const { result, sendEnrollmentInviteEmail } of [runs.other, runs.express]) { + expect(sendEnrollmentInviteEmail.mock.calls.map(([input]) => input)).toEqual([ + { to: "one@example.com", signInUrl: "https://app.example.com/login" }, + { to: "three@example.com", signInUrl: "https://app.example.com/login" }, + { to: "four@example.com", signInUrl: "https://app.example.com/login" }, + ]); + expect(result).toEqual({ + status: 200, + body: { + sent: 3, + skipped: 1, + remaining: 4, + results: [ + { userId: "u-1", status: "sent" }, + { userId: "u-2", status: "skipped", reason: "already_enrolled" }, + { userId: "u-3", status: "sent" }, + { userId: "u-4", status: "sent" }, + ], + }, + cookies: [], + }); + } + expect(runs.other.result).toEqual(runs.express.result); + }); + + it("reports an invite that could not be sent and sends the rest", async () => { + const runs = await both({ + send: async ({ to }) => { + if (to === "three@example.com") throw new Error("smtp down"); + }, + }); + + for (const { result, sendEnrollmentInviteEmail } of [runs.other, runs.express]) { + expect(sendEnrollmentInviteEmail).toHaveBeenCalledTimes(3); + expect(result.status).toBe(200); + expect(result.body).toEqual({ + sent: 2, + skipped: 2, + remaining: 4, + results: [ + { userId: "u-1", status: "sent" }, + { userId: "u-2", status: "skipped", reason: "already_enrolled" }, + { userId: "u-3", status: "skipped", reason: "delivery_failed" }, + { userId: "u-4", status: "sent" }, + ], + }); + } + expect(runs.other.result).toEqual(runs.express.result); + }); + + it("leaves delivery to the auth API without messaging", async () => { + const body = { + sent: 1, + skipped: 0, + results: [{ userId: "u-1", status: "sent" }], + }; + const runs = await both({ body, messaging: false }); + + for (const { result, init } of [runs.other, runs.express]) { + expect(init.headers["x-seamless-auth-delivery-mode"]).toBeUndefined(); + expect(result).toEqual({ status: 200, body, cookies: [] }); + } + }); +}); From 4163c4ade7deefbea647043342323313dfc4884a Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 14:12:35 -0500 Subject: [PATCH 2/4] feat: deliver enrollment invites through adopter messaging With messaging configured, POST /admin/enrollment/invites now requests external delivery with the external delivery service token and sends each results[].delivery through deliverAuthMessage. The payloads are stripped from the response. A recipient whose delivery throws is reported with the API's delivery_failed skip reason and the counts are adjusted, while the other recipients are still sent. The route is served from the manifest in every adapter, so the handling lives in core's handleManifestRoute. Closes #183 --- .../enrollment-invites-external-delivery.md | 10 +++ packages/core/src/deliverAuthMessage.ts | 70 +++++++++++++++++++ packages/core/src/manifestProxy.ts | 15 +++- packages/core/tests/manifestProxy.test.js | 1 + 4 files changed, 94 insertions(+), 2 deletions(-) create mode 100644 .changeset/enrollment-invites-external-delivery.md diff --git a/.changeset/enrollment-invites-external-delivery.md b/.changeset/enrollment-invites-external-delivery.md new file mode 100644 index 0000000..0b7ee8e --- /dev/null +++ b/.changeset/enrollment-invites-external-delivery.md @@ -0,0 +1,10 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": minor +"@seamless-auth/fastify": minor +"@seamless-auth/nextjs": minor +--- + +Deliver enrollment invites through adopter messaging (#183). With `messaging` configured, `POST /admin/enrollment/invites` now asks the auth API for external delivery (`x-seamless-auth-delivery-mode: external`, with the `seamless-auth-external-delivery` service token) and sends each invite in `results[].delivery` through `deliverAuthMessage`, so `handlers.sendEnrollmentInviteEmail`, `overrides.enrollmentInviteEmail` or the email transport send it instead of the auth API's mail system. Without `messaging` the route is unchanged. + +The response no longer carries `results[].delivery`, so recipient addresses and sign-in links are not echoed to the browser. A recipient whose delivery throws is reported as `{ "status": "skipped", "reason": "delivery_failed" }` and `sent` and `skipped` are adjusted; the other recipients are still sent and the route answers 200. The auth API has already recorded that recipient as invited, so repeating the call within a day reports them as `recently_invited`. diff --git a/packages/core/src/deliverAuthMessage.ts b/packages/core/src/deliverAuthMessage.ts index 751dbb2..5890e3d 100644 --- a/packages/core/src/deliverAuthMessage.ts +++ b/packages/core/src/deliverAuthMessage.ts @@ -370,3 +370,73 @@ export async function applyExternalDelivery( return body; } + +interface EnrollmentInviteResult { + userId: string; + status: "sent" | "skipped"; + reason?: string; + delivery?: AuthDeliveryInstruction; +} + +/** + * Sends each invite the auth API handed back from `POST /admin/enrollment/invites` and + * drops the payloads, which carry addresses and links the browser has no use for. + * + * One recipient's failed delivery does not fail the batch: it is reported with the API's + * own `delivery_failed` skip reason and the counts follow. The API has already stamped + * that user as invited, so an immediate retry reports them as `recently_invited`. + */ +export async function deliverEnrollmentInvites( + messaging: SeamlessAuthMessagingOptions | undefined, + body: unknown, +): Promise { + if ( + !body || + typeof body !== "object" || + !Array.isArray((body as { results?: unknown }).results) + ) { + return body; + } + + const results: EnrollmentInviteResult[] = []; + let failed = 0; + + for (const entry of (body as { results: EnrollmentInviteResult[] }) + .results) { + if (!entry || typeof entry !== "object" || !("delivery" in entry)) { + results.push(entry); + continue; + } + + const { delivery, ...result } = entry; + + if (result.status !== "sent") { + results.push(result); + continue; + } + + try { + await deliverAuthMessage(messaging, delivery); + results.push(result); + } catch (error) { + getSeamlessLogger().error( + `[SeamlessAuth] An enrollment invite could not be sent: ${ + error instanceof Error ? error.name : "unknown error" + }.`, + ); + failed += 1; + results.push({ ...result, status: "skipped", reason: "delivery_failed" }); + } + } + + const counts = body as { sent?: unknown; skipped?: unknown }; + + return { + ...body, + ...(typeof counts.sent === "number" ? { sent: counts.sent - failed } : {}), + ...(typeof counts.skipped === "number" + ? { skipped: counts.skipped + failed } + : {}), + results, + }; +} diff --git a/packages/core/src/manifestProxy.ts b/packages/core/src/manifestProxy.ts index d435093..bc87b75 100644 --- a/packages/core/src/manifestProxy.ts +++ b/packages/core/src/manifestProxy.ts @@ -12,6 +12,7 @@ import { applyExternalDelivery, deliverableNoticeKinds, deliverAuthMessage, + deliverEnrollmentInvites, } from "./deliverAuthMessage.js"; import type { AuthDeliveryInstruction } from "./authMessaging.js"; import { @@ -78,6 +79,12 @@ function cookieNameFor(held: AdapterHeld, opts: ManifestProxyOptions) { }[held]; } +// Its payloads come back per recipient in `results[].delivery`, which the manifest's +// `delivery` flag (one top-level payload) cannot describe. +function isEnrollmentInvites(route: AdapterManifestRoute) { + return route.method === "POST" && route.path === "/admin/enrollment/invites"; +} + function isObject(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } @@ -238,7 +245,9 @@ export async function handleManifestRoute( ? `Bearer ${extractBearerToken(input.authorization)}` : `Bearer ${input.cookiePayload?.token}`; - const externalDelivery = Boolean(route.delivery && opts.messaging); + const inviteDelivery = isEnrollmentInvites(route) && Boolean(opts.messaging); + const externalDelivery = + inviteDelivery || Boolean(route.delivery && opts.messaging); // Without a kind it can send, the request goes out as before and the API sends the // notices itself. const noticeKinds = route.notices ? deliverableNoticeKinds(opts.messaging) : []; @@ -289,7 +298,9 @@ export async function handleManifestRoute( return { status: upstream.status, ...readPassthroughFailure(data) }; } - if (externalDelivery) { + if (inviteDelivery) { + data = await deliverEnrollmentInvites(opts.messaging, data); + } else if (externalDelivery) { data = await applyExternalDelivery(opts.messaging, data); } diff --git a/packages/core/tests/manifestProxy.test.js b/packages/core/tests/manifestProxy.test.js index ff97e10..e12697c 100644 --- a/packages/core/tests/manifestProxy.test.js +++ b/packages/core/tests/manifestProxy.test.js @@ -16,6 +16,7 @@ jest.unstable_mockModule("../dist/deliverAuthMessage.js", () => ({ applyExternalDelivery: applyExternalDeliveryMock, deliverAuthMessage: deliverAuthMessageMock, deliverableNoticeKinds: deliverableNoticeKindsMock, + deliverEnrollmentInvites: jest.fn(), })); const { handleManifestRoute } = await import("../dist/manifestProxy.js"); From dc5e8a7904ad602670d28d2c58d49e2225b7126e Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 14:15:31 -0500 Subject: [PATCH 3/4] test: reproduce a transient refresh failure signing a browser out Parity scenarios for the silent refresh and cookie-transport POST /refresh in Express, Fastify and Next.js. A transport error, a non-401 answer, a 200 without a session and a rotated session that fails verification all clear the session cookies today (POST /refresh answers 500 for the last two). The auth API answering 401 and an unreadable refresh cookie still clear them. The existing forged-token scenarios now expect 502 with the cookies kept. A Next.js scenario pins that one caller aborting its request does not cancel a rotation another caller shares; it passes already, since no request's abort signal reaches the upstream call. Refs fells-code/seamless-auth-go#22 --- packages/fastify/tests/parity.test.js | 136 ++++++++++++++++- packages/nextjs/tests/parity.test.js | 204 +++++++++++++++++++++++++- 2 files changed, 336 insertions(+), 4 deletions(-) diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index 2f57af1..7518fd0 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -801,11 +801,14 @@ describe("fastify and express adapters agree", () => { global.fetch = refreshUpstream(forged); const expressResult = await viaExpress(scenario); - expect(fastify.status).toBe(401); + // A rotated session that fails verification is not the auth API refusing + // the token, so the cookies stay. + expect(fastify.status).toBe(502); + expect(fastify.body).toEqual({ error: "upstream_unavailable" }); + expect(fastify.cookies).toEqual([]); expect(fastify.status).toBe(expressResult.status); expect(fastify.body).toEqual(expressResult.body); expect(fastify.cookies).toEqual(expressResult.cookies); - expect(fastify.cookies.some((c) => c.includes("="))).toBe(false); expect(fastifyCalls.some((url) => url.endsWith("/users/me"))).toBe(false); } finally { spy.mockRestore(); @@ -2173,3 +2176,132 @@ describe("fastify and express deliver enrollment invites through adopter messagi } }); }); + +// A refresh that fails ends the session only when the auth API refused the +// token or the refresh cookie cannot be read. After a network failure or any +// other answer the token may still be good, or rotated with the answer lost, so +// the cookies stay for the next attempt (seamless-auth-go#22). +describe("fastify and express keep the session through a refresh that fails transiently", () => { + const originalFetch = global.fetch; + let errorSpy; + beforeEach(() => { + errorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + afterEach(() => { + global.fetch = originalFetch; + errorSpy.mockRestore(); + }); + + let run = 0; + const refreshCookie = (secret = COOKIE_SECRET) => { + run += 1; + const value = jwt.sign( + { sub: "user-123", refreshToken: `transient-${run}` }, + secret, + { algorithm: "HS256", expiresIn: "3600s" }, + ); + return `seamless-refresh=${value}`; + }; + + function failingUpstream(answer) { + return jest.fn(async (url) => { + const href = String(url); + if (href.endsWith("/.well-known/jwks.json")) return upstream(200, { keys: [jwk] }); + if (href.endsWith("/refresh")) return answer(); + return upstream(200, { user: { id: "u1" } }); + }); + } + + async function both(scenario, answer) { + global.fetch = failingUpstream(answer); + const other = await viaFastify({ ...scenario, cookie: scenario.cookie() }); + const otherCalls = global.fetch.mock.calls.map(([url]) => new URL(String(url)).pathname); + + global.fetch = failingUpstream(answer); + const expressResult = await viaExpress({ ...scenario, cookie: scenario.cookie() }); + const expressCalls = global.fetch.mock.calls.map(([url]) => new URL(String(url)).pathname); + + return { other, express: expressResult, otherCalls, expressCalls }; + } + + const names = (cookies) => cookies.map((cookie) => cookie.split("; ")[0]); + + const SILENT = { method: "get", path: "/users/me", cookie: () => refreshCookie() }; + const ROUTE = { method: "post", path: "/refresh", cookie: () => refreshCookie() }; + + const forgedSession = async () => + upstream(200, { + sub: "user-123", + token: await accessToken( + { sub: "user-123", typ: "access" }, + (await generateKeyPair("RS256")).privateKey, + ), + refreshToken: "rotated", + ttl: 300, + refreshTtl: 3600, + }); + + const KEPT = [ + [ + "the auth API cannot be reached", + () => Promise.reject(new TypeError("fetch failed")), + 502, + { error: "upstream_unavailable" }, + ], + ["the auth API answers 503", () => upstream(503, { error: "maintenance" }), 503, { error: "maintenance" }], + ["the auth API answers 500", () => upstream(500, { error: "internal" }), 500, { error: "internal" }], + ["the auth API answers 429", () => upstream(429, { error: "rate_limited" }), 429, { error: "rate_limited" }], + ["the rotated session fails verification", forgedSession, 502, { error: "upstream_unavailable" }], + ["the auth API answers 200 without a session", () => upstream(200, { message: "ok" }), 502, { error: "upstream_unavailable" }], + ]; + + describe.each([ + ["a silent refresh", SILENT], + ["POST /refresh", ROUTE], + ])("%s", (_flow, scenario) => { + it.each(KEPT)("keeps the cookies when %s", async (_label, answer, status, body) => { + const result = await both(scenario, answer); + + expect(result.other).toEqual({ status, body, cookies: [] }); + expect(result.express).toEqual(result.other); + expect(result.otherCalls).not.toContain("/users/me"); + }); + }); + + it("clears the session when the auth API refuses a silent refresh", async () => { + const result = await both(SILENT, () => upstream(401, { error: "refresh_token_reused" })); + + expect(result.other.status).toBe(401); + expect(result.other.body).toEqual({ error: "Refresh failed" }); + expect(names(result.other.cookies)).toEqual( + expect.arrayContaining(["seamless-access=", "seamless-refresh="]), + ); + expect(result.express).toEqual(result.other); + }); + + it("clears the session when the auth API refuses POST /refresh", async () => { + const result = await both(ROUTE, () => upstream(401, { error: "refresh_token_reused" })); + + expect(result.other.status).toBe(401); + expect(result.other.body).toEqual({ error: "invalid_refresh_token" }); + expect(names(result.other.cookies)).toEqual([ + "seamless-access=", + "seamless-refresh=", + ]); + expect(result.express).toEqual(result.other); + }); + + it.each([ + ["a silent refresh", { ...SILENT, cookie: () => refreshCookie("another-secret-another-secret-another") }], + ["POST /refresh", { ...ROUTE, cookie: () => refreshCookie("another-secret-another-secret-another") }], + ])("clears the session when %s holds an unreadable refresh cookie", async (_label, scenario) => { + const result = await both(scenario, () => upstream(200, {})); + + expect(result.other.status).toBe(401); + expect(names(result.other.cookies)).toEqual( + expect.arrayContaining(["seamless-access=", "seamless-refresh="]), + ); + expect(result.otherCalls).not.toContain("/refresh"); + expect(result.express).toEqual(result.other); + }); +}); diff --git a/packages/nextjs/tests/parity.test.js b/packages/nextjs/tests/parity.test.js index 3d48470..6f8f939 100644 --- a/packages/nextjs/tests/parity.test.js +++ b/packages/nextjs/tests/parity.test.js @@ -776,11 +776,14 @@ describe("next.js and express adapters agree", () => { global.fetch = refreshUpstream(forged); const expressResult = await viaExpress(scenario); - expect(next.status).toBe(401); + // A rotated session that fails verification is not the auth API refusing + // the token, so the cookies stay. + expect(next.status).toBe(502); + expect(next.body).toEqual({ error: "upstream_unavailable" }); + expect(next.cookies).toEqual([]); expect(next.status).toBe(expressResult.status); expect(next.body).toEqual(expressResult.body); expect(next.cookies).toEqual(expressResult.cookies); - expect(next.cookies.some((c) => c.includes("="))).toBe(false); expect(nextCalls.some((url) => url.endsWith("/users/me"))).toBe(false); } finally { spy.mockRestore(); @@ -2375,3 +2378,200 @@ describe("next and express deliver enrollment invites through adopter messaging" } }); }); + +// A refresh that fails ends the session only when the auth API refused the +// token or the refresh cookie cannot be read. After a network failure or any +// other answer the token may still be good, or rotated with the answer lost, so +// the cookies stay for the next attempt (seamless-auth-go#22). +describe("next and express keep the session through a refresh that fails transiently", () => { + const originalFetch = global.fetch; + let errorSpy; + beforeEach(() => { + errorSpy = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + afterEach(() => { + global.fetch = originalFetch; + errorSpy.mockRestore(); + }); + + let run = 0; + const refreshCookie = (secret = COOKIE_SECRET) => { + run += 1; + const value = jwt.sign( + { sub: "user-123", refreshToken: `transient-${run}` }, + secret, + { algorithm: "HS256", expiresIn: "3600s" }, + ); + return `seamless-refresh=${value}`; + }; + + function failingUpstream(answer) { + return jest.fn(async (url) => { + const href = String(url); + if (href.endsWith("/.well-known/jwks.json")) return upstream(200, { keys: [jwk] }); + if (href.endsWith("/refresh")) return answer(); + return upstream(200, { user: { id: "u1" } }); + }); + } + + async function both(scenario, answer) { + global.fetch = failingUpstream(answer); + const other = await viaNext({ ...scenario, cookie: scenario.cookie() }); + const otherCalls = global.fetch.mock.calls.map(([url]) => new URL(String(url)).pathname); + + global.fetch = failingUpstream(answer); + const expressResult = await viaExpress({ ...scenario, cookie: scenario.cookie() }); + const expressCalls = global.fetch.mock.calls.map(([url]) => new URL(String(url)).pathname); + + return { other, express: expressResult, otherCalls, expressCalls }; + } + + const names = (cookies) => cookies.map((cookie) => cookie.split("; ")[0]); + + const SILENT = { method: "get", path: "/users/me", cookie: () => refreshCookie() }; + const ROUTE = { method: "post", path: "/refresh", cookie: () => refreshCookie() }; + + const forgedSession = async () => + upstream(200, { + sub: "user-123", + token: await accessToken( + { sub: "user-123", typ: "access" }, + (await generateKeyPair("RS256")).privateKey, + ), + refreshToken: "rotated", + ttl: 300, + refreshTtl: 3600, + }); + + const KEPT = [ + [ + "the auth API cannot be reached", + () => Promise.reject(new TypeError("fetch failed")), + 502, + { error: "upstream_unavailable" }, + ], + ["the auth API answers 503", () => upstream(503, { error: "maintenance" }), 503, { error: "maintenance" }], + ["the auth API answers 500", () => upstream(500, { error: "internal" }), 500, { error: "internal" }], + ["the auth API answers 429", () => upstream(429, { error: "rate_limited" }), 429, { error: "rate_limited" }], + ["the rotated session fails verification", forgedSession, 502, { error: "upstream_unavailable" }], + ["the auth API answers 200 without a session", () => upstream(200, { message: "ok" }), 502, { error: "upstream_unavailable" }], + ]; + + describe.each([ + ["a silent refresh", SILENT], + ["POST /refresh", ROUTE], + ])("%s", (_flow, scenario) => { + it.each(KEPT)("keeps the cookies when %s", async (_label, answer, status, body) => { + const result = await both(scenario, answer); + + expect(result.other).toEqual({ status, body, cookies: [] }); + expect(result.express).toEqual(result.other); + expect(result.otherCalls).not.toContain("/users/me"); + }); + }); + + it("clears the session when the auth API refuses a silent refresh", async () => { + const result = await both(SILENT, () => upstream(401, { error: "refresh_token_reused" })); + + expect(result.other.status).toBe(401); + expect(result.other.body).toEqual({ error: "Refresh failed" }); + expect(names(result.other.cookies)).toEqual( + expect.arrayContaining(["seamless-access=", "seamless-refresh="]), + ); + expect(result.express).toEqual(result.other); + }); + + it("clears the session when the auth API refuses POST /refresh", async () => { + const result = await both(ROUTE, () => upstream(401, { error: "refresh_token_reused" })); + + expect(result.other.status).toBe(401); + expect(result.other.body).toEqual({ error: "invalid_refresh_token" }); + expect(names(result.other.cookies)).toEqual([ + "seamless-access=", + "seamless-refresh=", + ]); + expect(result.express).toEqual(result.other); + }); + + it.each([ + ["a silent refresh", { ...SILENT, cookie: () => refreshCookie("another-secret-another-secret-another") }], + ["POST /refresh", { ...ROUTE, cookie: () => refreshCookie("another-secret-another-secret-another") }], + ])("clears the session when %s holds an unreadable refresh cookie", async (_label, scenario) => { + const result = await both(scenario, () => upstream(200, {})); + + expect(result.other.status).toBe(401); + expect(names(result.other.cookies)).toEqual( + expect.arrayContaining(["seamless-access=", "seamless-refresh="]), + ); + expect(result.otherCalls).not.toContain("/refresh"); + expect(result.express).toEqual(result.other); + }); +}); + +// Nothing passes a request's abort signal to the upstream call, so a caller +// that goes away cannot cancel a rotation other callers share +// (seamless-auth-go#22). +describe("next.js shares a rotation whose first caller went away", () => { + const originalFetch = global.fetch; + afterEach(() => { + global.fetch = originalFetch; + }); + + it("hands the rotated session to the callers still waiting", async () => { + let releaseRefresh; + const refreshAnswered = new Promise((resolve) => { + releaseRefresh = resolve; + }); + let refreshStarted; + const started = new Promise((resolve) => { + refreshStarted = resolve; + }); + const refreshSignals = []; + + global.fetch = jest.fn(async (url, init) => { + const href = String(url); + if (href.endsWith("/.well-known/jwks.json")) return upstream(200, { keys: [jwk] }); + if (href.endsWith("/refresh")) { + refreshSignals.push(init?.signal); + refreshStarted(); + await refreshAnswered; + return upstream(200, { + sub: "user-123", + token: REFRESHED_ACCESS_TOKEN, + refreshToken: "after-abort", + roles: ["admin"], + ttl: 300, + refreshTtl: 3600, + }); + } + return upstream(200, { user: { id: "u1" } }); + }); + + const handlers = createSeamlessAuthHandler(OPTIONS); + const cookie = `seamless-refresh=${signed({ sub: "user-123", refreshToken: "shared-abort" }, "3600s")}`; + const leaving = new AbortController(); + const call = (signal) => + handlers.GET( + new Request("http://localhost/auth/users/me", { + headers: { cookie }, + ...(signal ? { signal } : {}), + }), + ); + + const first = call(leaving.signal); + await started; + const second = call(); + leaving.abort(); + releaseRefresh(); + + const [firstRes, secondRes] = await Promise.all([first, second]); + + expect(refreshSignals).toHaveLength(1); + expect(refreshSignals[0]?.aborted).toBe(false); + expect(secondRes.status).toBe(200); + expect(firstRes.status).toBe(200); + expect( + secondRes.headers.getSetCookie().some((c) => c.startsWith("seamless-refresh=") && !c.startsWith("seamless-refresh=;")), + ).toBe(true); + }); +}); From a23743a27a1c8a97dd0f8e47a19a01b27b97de1c Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 14:20:40 -0500 Subject: [PATCH 4/4] fix: keep the session cookies when a refresh fails transiently The silent refresh and cookie-transport POST /refresh cleared the session cookies on any failure. They are now cleared only when the auth API answers the refresh with 401 or the refresh cookie cannot be read. A transport error answers 502 upstream_unavailable, any other non-2xx answer passes through, and a rotated session that fails verification (or a 2xx without one) answers 502, all with the cookies kept. This matches the Go adapter's fix for fells-code/seamless-auth-go#22. Core gains refreshCookieSession, which reports how a cookie rotation ended; refreshAccessToken keeps its signature. In-flight sharing and the 5 second completed-rotation window are unchanged. Tests that pinned the old answers are updated, and the cookie security tests now serve a verifiable rotated token, since they had been reading the attributes of cookies cleared by a failed verification. --- .changeset/refresh-failure-keeps-session.md | 16 ++++ packages/core/src/applyResult.ts | 16 +--- packages/core/src/ensureCookies.ts | 48 +++++++--- packages/core/src/handlers/refresh.ts | 30 ++++-- packages/core/src/refreshAccessToken.ts | 91 ++++++++++++++----- packages/core/src/result.ts | 9 ++ packages/core/tests/ensureCookies.test.js | 14 ++- packages/core/tests/pathParam.test.js | 1 + .../express/src/middleware/ensureCookies.ts | 3 +- packages/express/tests/cookieSecurity.test.js | 21 ++++- .../tests/ensureCookies.middleware.test.js | 14 ++- packages/fastify/src/hooks/ensureCookies.ts | 3 +- packages/fastify/tests/issuer.parity.test.js | 8 +- packages/nextjs/src/handler.ts | 3 +- packages/nextjs/tests/issuer.parity.test.js | 8 +- 15 files changed, 212 insertions(+), 73 deletions(-) create mode 100644 .changeset/refresh-failure-keeps-session.md diff --git a/.changeset/refresh-failure-keeps-session.md b/.changeset/refresh-failure-keeps-session.md new file mode 100644 index 0000000..e824908 --- /dev/null +++ b/.changeset/refresh-failure-keeps-session.md @@ -0,0 +1,16 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": minor +"@seamless-auth/fastify": minor +"@seamless-auth/nextjs": minor +--- + +Keep the session cookies when a refresh fails without the auth API refusing it, matching the Go adapter (fells-code/seamless-auth-go#22). The silent refresh on the auth routes and cookie-transport `POST /refresh` used to clear the access and refresh cookies on any failure, so a network blip or an auth API restart signed the browser out. + +The cookies are now cleared only when the auth API answers the refresh with 401, or the refresh cookie cannot be read. Those keep their current answers (`401 {"error":"Refresh failed"}` on a silent refresh, `401 {"error":"invalid_refresh_token"}` on `POST /refresh`). Otherwise the cookies stay and: + +- the auth API cannot be reached: `502 {"error":"upstream_unavailable"}`; +- the auth API answers any other non-2xx status: that status and body pass through; +- the rotated session fails verification, or a 2xx carries no session: `502 {"error":"upstream_unavailable"}`. `POST /refresh` answered these with 500 before. + +Core adds `refreshCookieSession`, which reports how a cookie rotation ended, and `failureBody`. `refreshAccessToken` keeps its signature. In-flight sharing and the 5 second window for a completed cookie rotation are unchanged, and only a successful rotation is kept. No request's abort signal reaches the upstream call, so a caller that goes away does not cancel a rotation other callers share. diff --git a/packages/core/src/applyResult.ts b/packages/core/src/applyResult.ts index 7e1aa38..efb1d96 100644 --- a/packages/core/src/applyResult.ts +++ b/packages/core/src/applyResult.ts @@ -1,7 +1,7 @@ import jwt from "jsonwebtoken"; import type { CookiePayload } from "./ensureCookies.js"; -import type { ResultFailure } from "./result.js"; +import { failureBody, type ResultFailure } from "./result.js"; import type { AuthTransport } from "./transport.js"; export type CookieSameSite = "lax" | "none" | "strict"; @@ -244,18 +244,8 @@ export function applyResult( applyCookies(result, adapter, opts); } - if (result.errorBody !== undefined) { - adapter.send(result.status, result.errorBody); - return; - } - - if (result.errorCode !== undefined) { - adapter.send( - result.status, - result.details === undefined - ? { error: result.errorCode } - : { error: result.errorCode, details: result.details }, - ); + if (result.errorBody !== undefined || result.errorCode !== undefined) { + adapter.send(result.status, failureBody(result)); return; } diff --git a/packages/core/src/ensureCookies.ts b/packages/core/src/ensureCookies.ts index c2b68e0..b00925d 100644 --- a/packages/core/src/ensureCookies.ts +++ b/packages/core/src/ensureCookies.ts @@ -1,7 +1,9 @@ import { holdsAccessToken } from "./guards.js"; import { verifyCookieJwt } from "./verifyCookieJwt.js"; import type { ResultFailure } from "./result.js"; -import { refreshAccessToken } from "./refreshAccessToken.js"; +import { refreshCookieSession } from "./refreshAccessToken.js"; +import { UPSTREAM_UNAVAILABLE_ERROR_CODE } from "./authFetch.js"; +import { readPassthroughFailure } from "./upstreamError.js"; import { assertSecrets } from "./validateSecrets.js"; import { type AdapterManifest, @@ -284,7 +286,7 @@ async function refreshRequiredCookie( return null; } - const refreshed = await refreshAccessToken(refreshCookie, { + const outcome = await refreshCookieSession(refreshCookie, { authServerUrl: opts.authServerUrl, cookieSecret: opts.cookieSecret, serviceSecret: opts.serviceSecret, @@ -295,19 +297,37 @@ async function refreshRequiredCookie( forwardedUserAgent: opts.forwardedUserAgent, }); - const failure: EnsureCookiesResult = { + if (outcome.type === "rejected") { + return { + type: "error", + status: 401, + errorCode: "Refresh failed", + clearCookies: [ + cookieName, + opts.registrationCookieName, + opts.refreshCookieName, + ], + }; + } + + // The token may still be good, or rotated with the answer lost, so the + // cookies stay for the next attempt. + if (outcome.type === "answered") { + return { + type: "error", + status: outcome.status, + ...readPassthroughFailure(outcome.body), + }; + } + + const unavailable: EnsureCookiesResult = { type: "error", - status: 401, - errorCode: "Refresh failed", - clearCookies: [ - cookieName, - opts.registrationCookieName, - opts.refreshCookieName, - ], + status: 502, + errorCode: UPSTREAM_UNAVAILABLE_ERROR_CODE, }; - if (!refreshed?.token) { - return failure; + if (outcome.type === "unavailable") { + return unavailable; } // The cookie signs whatever goes into it with the adopter's own secret, and @@ -318,7 +338,7 @@ async function refreshRequiredCookie( let setCookies: CookieInstruction[]; try { setCookies = await issueSessionCookies( - refreshed as UpstreamSessionResponse, + outcome.session as UpstreamSessionResponse, { authServerUrl: opts.authServerUrl, audience: opts.accessTokenAudience || opts.authServerUrl, @@ -329,7 +349,7 @@ async function refreshRequiredCookie( }, ); } catch { - return failure; + return unavailable; } const access = setCookies[0].value; diff --git a/packages/core/src/handlers/refresh.ts b/packages/core/src/handlers/refresh.ts index 94e8475..8957be1 100644 --- a/packages/core/src/handlers/refresh.ts +++ b/packages/core/src/handlers/refresh.ts @@ -1,9 +1,11 @@ import type { AuthServerIssuerOption } from "../authServerIssuer.js"; import { extractBearerToken } from "../verifyAccessToken.js"; import { - refreshAccessToken, refreshBearerSession, + refreshCookieSession, } from "../refreshAccessToken.js"; +import { UPSTREAM_UNAVAILABLE_ERROR_CODE } from "../authFetch.js"; +import { readPassthroughFailure } from "../upstreamError.js"; import { SERVICE_TOKEN_AUDIENCE, SERVICE_TOKEN_ISSUER } from "../apiContract.js"; import { sessionResult, type UpstreamSessionResponse } from "../upstreamSession.js"; import type { AuthTransport } from "../transport.js"; @@ -98,7 +100,7 @@ export async function refreshHandler( return { status: 401, errorCode: "refresh token required" }; } - const rotated = await refreshAccessToken(input.refreshCookie, { + const rotated = await refreshCookieSession(input.refreshCookie, { authServerUrl: opts.authServerUrl, cookieSecret: opts.cookieSecret, serviceSecret: opts.serviceSecret, @@ -110,7 +112,7 @@ export async function refreshHandler( ...forwarding, }); - if (!rotated) { + if (rotated.type === "rejected") { return { status: 401, errorCode: "invalid_refresh_token", @@ -118,8 +120,22 @@ export async function refreshHandler( }; } - return { - status: 200, - ...(await sessionResult(rotated as UpstreamSessionResponse, session)), - }; + // Short of a refusal the token may still be good, or rotated with the answer + // lost, so the cookies stay for the next attempt. + if (rotated.type === "answered") { + return { status: rotated.status, ...readPassthroughFailure(rotated.body) }; + } + + if (rotated.type === "unavailable") { + return { status: 502, errorCode: UPSTREAM_UNAVAILABLE_ERROR_CODE }; + } + + try { + return { + status: 200, + ...(await sessionResult(rotated.session as UpstreamSessionResponse, session)), + }; + } catch { + return { status: 502, errorCode: UPSTREAM_UNAVAILABLE_ERROR_CODE }; + } } diff --git a/packages/core/src/refreshAccessToken.ts b/packages/core/src/refreshAccessToken.ts index 98ece2f..c7657bb 100644 --- a/packages/core/src/refreshAccessToken.ts +++ b/packages/core/src/refreshAccessToken.ts @@ -29,10 +29,24 @@ type RefreshAccessTokenResult = { refreshTtl: number; }; -const inFlightRefreshes = new Map< - string, - Promise ->(); +/** + * How a cookie-transport rotation ended. Only `rejected` ends the session: the + * auth API refused the token, or the refresh cookie cannot be read. After a + * network failure or any other answer the token may still be good, or rotated + * with the answer lost, so the caller keeps the cookies for the next attempt. + */ +export type RefreshCookieOutcome = + | { type: "session"; session: RefreshAccessTokenResult } + | { type: "rejected" } + /** The auth API's own non-2xx answer. A transport failure arrives here as 502 `upstream_unavailable`. */ + | { type: "answered"; status: number; body: unknown } + /** A 2xx that carried no session. */ + | { type: "unavailable" }; + +// Shared by callers racing one token. Nothing here is tied to the request that +// started the rotation, so a caller going away cannot cancel it for the others: +// authFetch's only abort signal is its own timeout. +const inFlightRefreshes = new Map>(); /** * Runs one refresh per key at a time. The auth API rotates refresh tokens and @@ -45,13 +59,13 @@ const inFlightRefreshes = new Map< * the replay. The refresh cookie is the one exception, see * `completedCookieRefreshes`. */ -async function dedupeRefresh( +async function dedupeRefresh( key: string, - run: () => Promise, -): Promise { + run: () => Promise, +): Promise { const existingRefresh = inFlightRefreshes.get(key); if (existingRefresh) { - return existingRefresh as Promise; + return existingRefresh as Promise; } const refreshPromise = run(); @@ -103,45 +117,61 @@ function pruneCompletedCookieRefreshes(now: number): void { async function dedupeCookieRefresh( refreshCookie: string, - run: () => Promise, -): Promise { + run: () => Promise, +): Promise { const key = createHash("sha256").update(refreshCookie).digest("base64url"); const completed = completedCookieRefreshes.get(key); if (completed) { - if (completed.expiresAt > Date.now()) return completed.result; + if (completed.expiresAt > Date.now()) { + return { type: "session", session: completed.result }; + } completedCookieRefreshes.delete(key); } - const result = await dedupeRefresh(`cookie:${key}`, run); + const outcome = await dedupeRefresh(`cookie:${key}`, run); // Every caller sharing the in-flight rotation reaches this; only the first // insert matters, and a later one must not extend the window. - if (result && !completedCookieRefreshes.has(key)) { + if (outcome.type === "session" && !completedCookieRefreshes.has(key)) { const now = Date.now(); pruneCompletedCookieRefreshes(now); completedCookieRefreshes.set(key, { - result, + result: outcome.session, expiresAt: now + COMPLETED_COOKIE_REFRESH_TTL_MS, }); } - return result; + return outcome; } -export async function refreshAccessToken( +/** + * Rotates the session held in a signed refresh cookie and says how it ended, so + * the caller can tell a refusal (clear the session) from a failure to complete + * the rotation (keep it). + */ +export async function refreshCookieSession( refreshCookie: string, opts: RefreshAccessTokenOptions, -): Promise { +): Promise { assertSecrets(opts, { allowPlaceholder: true }); return dedupeCookieRefresh(refreshCookie, async () => { const payload = verifyRefreshCookie(refreshCookie, opts.cookieSecret); - if (!payload) return null; + if (!payload) return { type: "rejected" }; return rotate(payload, opts); }); } +/** {@link refreshCookieSession} reduced to the session, or `null` for any failure. */ +export async function refreshAccessToken( + refreshCookie: string, + opts: RefreshAccessTokenOptions, +): Promise { + const outcome = await refreshCookieSession(refreshCookie, opts); + return outcome.type === "session" ? outcome.session : null; +} + /** * Rotates a refresh token this request already holds unsigned, such as one a * silent refresh earlier in the same request just issued. The cookie carrying @@ -153,15 +183,16 @@ export async function refreshSessionToken( ): Promise { assertSecrets(opts, { allowPlaceholder: true }); - return dedupeRefresh(`token:${payload.refreshToken}`, () => + const outcome = await dedupeRefresh(`token:${payload.refreshToken}`, () => rotate(payload, opts), ); + return outcome.type === "session" ? outcome.session : null; } async function rotate( payload: { sub: string; refreshToken: string }, opts: RefreshAccessTokenOptions, -): Promise { +): Promise { const serviceToken = createServiceToken({ subject: payload.sub, issuer: opts.issuer, @@ -179,9 +210,25 @@ async function rotate( forwardedUserAgent: opts.forwardedUserAgent, }); - if (!response.ok) return null; + const body: unknown = await response.json(); - return response.json(); + if (response.status === 401) return { type: "rejected" }; + if (!response.ok) { + return { type: "answered", status: response.status, body }; + } + + return isSession(body) + ? { type: "session", session: body } + : { type: "unavailable" }; +} + +function isSession(body: unknown): body is RefreshAccessTokenResult { + return ( + typeof body === "object" && + body !== null && + typeof (body as { token?: unknown }).token === "string" && + (body as { token: string }).token !== "" + ); } class UpstreamRefreshFailure extends Error { diff --git a/packages/core/src/result.ts b/packages/core/src/result.ts index 9a8489d..e4848d8 100644 --- a/packages/core/src/result.ts +++ b/packages/core/src/result.ts @@ -20,3 +20,12 @@ export interface ResultFailure { errorBody?: unknown; details?: unknown; } + +/** The body a failure goes out as: the auth API's own, else `{ error, details }`. */ +export function failureBody(result: ResultFailure): unknown { + if (result.errorBody !== undefined) return result.errorBody; + + return result.details === undefined + ? { error: result.errorCode } + : { error: result.errorCode, details: result.details }; +} diff --git a/packages/core/tests/ensureCookies.test.js b/packages/core/tests/ensureCookies.test.js index ad57bd2..5d1e2b9 100644 --- a/packages/core/tests/ensureCookies.test.js +++ b/packages/core/tests/ensureCookies.test.js @@ -8,8 +8,13 @@ jest.unstable_mockModule("../dist/verifyCookieJwt.js", () => ({ verifyCookieJwt: verifyCookieJwtMock, })); +// A null session stands for the auth API refusing the token. jest.unstable_mockModule("../dist/refreshAccessToken.js", () => ({ refreshAccessToken: refreshAccessTokenMock, + refreshCookieSession: async (...args) => { + const session = await refreshAccessTokenMock(...args); + return session ? { type: "session", session } : { type: "rejected" }; + }, })); jest.unstable_mockModule("../dist/verifySignedAuthResponse.js", () => ({ @@ -310,11 +315,13 @@ describe("ensureCookies", () => { const result = await silentRefresh(); + // Not a refusal from the auth API, so the session cookies stay. expect(result.type).toBe("error"); - expect(result.status).toBe(401); + expect(result.status).toBe(502); + expect(result.errorCode).toBe("upstream_unavailable"); expect(result.setCookies).toBeUndefined(); expect(result.user).toBeUndefined(); - expect(result.clearCookies).toEqual(["access", "registration", "refresh"]); + expect(result.clearCookies).toBeUndefined(); }); it("issues no cookie when the token names a different subject than the body", async () => { @@ -322,8 +329,9 @@ describe("ensureCookies", () => { const result = await silentRefresh(); - expect(result.status).toBe(401); + expect(result.status).toBe(502); expect(result.setCookies).toBeUndefined(); + expect(result.clearCookies).toBeUndefined(); }); it("verifies against the access token audience, not the service token's", async () => { diff --git a/packages/core/tests/pathParam.test.js b/packages/core/tests/pathParam.test.js index c442473..8eacb85 100644 --- a/packages/core/tests/pathParam.test.js +++ b/packages/core/tests/pathParam.test.js @@ -4,6 +4,7 @@ const authFetchMock = jest.fn(); jest.unstable_mockModule("../dist/authFetch.js", () => ({ authFetch: authFetchMock, + UPSTREAM_UNAVAILABLE_ERROR_CODE: "upstream_unavailable", })); const { diff --git a/packages/express/src/middleware/ensureCookies.ts b/packages/express/src/middleware/ensureCookies.ts index 0c17fb6..3053266 100644 --- a/packages/express/src/middleware/ensureCookies.ts +++ b/packages/express/src/middleware/ensureCookies.ts @@ -4,6 +4,7 @@ import { ensureCookies, type EnsureCookiesOptions, EnsureCookiesResult, + failureBody, refreshAfterRecoveryEnrollment, } from "@seamless-auth/core"; @@ -126,6 +127,6 @@ function applyMiddlewareResult( } if (result.type === "error") { - res.status(result.status ?? 401).json({ error: result.errorCode }); + res.status(result.status ?? 401).json(failureBody(result)); } } diff --git a/packages/express/tests/cookieSecurity.test.js b/packages/express/tests/cookieSecurity.test.js index 7e303ea..4f8b807 100644 --- a/packages/express/tests/cookieSecurity.test.js +++ b/packages/express/tests/cookieSecurity.test.js @@ -1,3 +1,4 @@ +import { generateKeyPairSync } from "node:crypto"; import { jest } from "@jest/globals"; import cookieParser from "cookie-parser"; import express from "express"; @@ -18,6 +19,21 @@ function createJsonResponse(status, body) { }; } +// The rotated token has to verify for the access cookie to be set at all. +const { privateKey, publicKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); +const jwk = { ...publicKey.export({ format: "jwk" }), alg: "RS256", kid: "k1", use: "sig" }; +const REFRESHED_ACCESS_TOKEN = jwt.sign( + { sub: "user-123", typ: "access", sid: "session-123" }, + privateKey, + { + algorithm: "RS256", + keyid: "k1", + issuer: "https://auth.example.com", + audience: "https://auth.example.com", + expiresIn: "5m", + }, +); + function createRefreshCookie(subject = "user-123") { const token = jwt.sign( { sub: subject, refreshToken: "opaque-refresh-token" }, @@ -84,10 +100,13 @@ describe("cookie security policy (#64)", () => { beforeEach(() => { global.fetch = jest.fn(async (url) => { + if (String(url).endsWith("/.well-known/jwks.json")) { + return createJsonResponse(200, { keys: [jwk] }); + } if (String(url).endsWith("/refresh")) { return createJsonResponse(200, { sub: "user-123", - token: "new-access", + token: REFRESHED_ACCESS_TOKEN, refreshToken: "new-refresh", roles: ["user"], email: "user@example.com", diff --git a/packages/express/tests/ensureCookies.middleware.test.js b/packages/express/tests/ensureCookies.middleware.test.js index b3c27af..918ef84 100644 --- a/packages/express/tests/ensureCookies.middleware.test.js +++ b/packages/express/tests/ensureCookies.middleware.test.js @@ -128,8 +128,11 @@ describe("createEnsureCookiesMiddleware silent refresh", () => { const res = await refreshWithTokenFor("https://app.example.com", {}, "opaque-2"); spy.mockRestore(); - expect(res.status).toBe(401); - expect(res.headers["set-cookie"].join(";")).not.toMatch(/access=ey/); + // The rotation completed but its token does not verify, which is not the + // auth API refusing the session, so the cookies stay. + expect(res.status).toBe(502); + expect(res.body).toEqual({ error: "upstream_unavailable" }); + expect(res.headers["set-cookie"]).toBeUndefined(); }); // On the local Docker stack the auth server signs as http://auth:5312 while @@ -156,7 +159,10 @@ describe("createEnsureCookiesMiddleware silent refresh", () => { ); spy.mockRestore(); - expect(res.status).toBe(401); - expect(res.headers["set-cookie"].join(";")).not.toMatch(/access=ey/); + // The rotation completed but its token does not verify, which is not the + // auth API refusing the session, so the cookies stay. + expect(res.status).toBe(502); + expect(res.body).toEqual({ error: "upstream_unavailable" }); + expect(res.headers["set-cookie"]).toBeUndefined(); }); }); diff --git a/packages/fastify/src/hooks/ensureCookies.ts b/packages/fastify/src/hooks/ensureCookies.ts index 5b79366..1df690d 100644 --- a/packages/fastify/src/hooks/ensureCookies.ts +++ b/packages/fastify/src/hooks/ensureCookies.ts @@ -5,6 +5,7 @@ import { assertSecrets, ensureCookies, type EnsureCookiesOptions, + failureBody, refreshAfterRecoveryEnrollment, SERVICE_TOKEN_AUDIENCE, SERVICE_TOKEN_ISSUER, @@ -98,7 +99,7 @@ export function createEnsureCookiesHook( if (result.type === "error") { return reply .status(result.status ?? 401) - .send({ error: result.errorCode }); + .send(failureBody(result)); } // Fastify keeps one Set-Cookie per name, so the rotated cookies replace the diff --git a/packages/fastify/tests/issuer.parity.test.js b/packages/fastify/tests/issuer.parity.test.js index a138792..e1745e0 100644 --- a/packages/fastify/tests/issuer.parity.test.js +++ b/packages/fastify/tests/issuer.parity.test.js @@ -290,10 +290,12 @@ describe("authServerIssuer", () => { await mockUpstream(AUTH); const misconfigured = await run(silentRefresh(), DOCKER_OPTIONS); - // The 401 clears the session cookies, so their names still appear. + // A rotated session that fails verification is not the auth API refusing + // the token, so the answer is 502 and the cookies stay. for (const res of [unconfigured, misconfigured]) { - expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Refresh failed" }); + expect(res.status).toBe(502); + expect(res.body).toEqual({ error: "upstream_unavailable" }); + expect(res.cookies).toEqual([]); } }); }); diff --git a/packages/nextjs/src/handler.ts b/packages/nextjs/src/handler.ts index 7306549..efd16eb 100644 --- a/packages/nextjs/src/handler.ts +++ b/packages/nextjs/src/handler.ts @@ -13,6 +13,7 @@ import { type EnsureCookiesOptions, handleManifestRoute, InvalidPathParamError, + failureBody, matchManifestRoute, redactSensitiveText, refreshAfterRecoveryEnrollment, @@ -244,7 +245,7 @@ async function dispatch( } if (result.type === "error") { - return collector.json(result.status ?? 401, { error: result.errorCode }); + return collector.json(result.status ?? 401, failureBody(result)); } refreshRecovery = (status) => diff --git a/packages/nextjs/tests/issuer.parity.test.js b/packages/nextjs/tests/issuer.parity.test.js index 3fad36d..c750333 100644 --- a/packages/nextjs/tests/issuer.parity.test.js +++ b/packages/nextjs/tests/issuer.parity.test.js @@ -290,10 +290,12 @@ describe("authServerIssuer", () => { await mockUpstream(AUTH); const misconfigured = await run(silentRefresh(), DOCKER_OPTIONS); - // The 401 clears the session cookies, so their names still appear. + // A rotated session that fails verification is not the auth API refusing + // the token, so the answer is 502 and the cookies stay. for (const res of [unconfigured, misconfigured]) { - expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Refresh failed" }); + expect(res.status).toBe(502); + expect(res.body).toEqual({ error: "upstream_unavailable" }); + expect(res.cookies).toEqual([]); } }); });