From cc31117a0c5618aa67e992737ed2b0997cb0ee76 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sun, 1 Nov 2026 18:26:14 -0500 Subject: [PATCH] fix(core): align upstream calls with the other adapters Request OTP codes and magic links with POST, time out calls to the auth API after 15 seconds and answer 502 upstream_unavailable when it cannot be reached, keep the authServerUrl path prefix when fetching the key set, and verify the registration token before setting its cookie. Also bumps @seamless-auth/types to ^0.35.0 in core and nextjs, which the types-current CI check requires. Closes #261 --- .changeset/upstream-call-parity.md | 15 ++ packages/core/README.md | 4 +- packages/core/package.json | 2 +- packages/core/src/authFetch.ts | 55 +++++-- packages/core/src/handlers/register.ts | 20 ++- .../src/handlers/requestMagicLinkHandler.ts | 2 +- .../core/src/handlers/requestOtpHandler.ts | 2 +- packages/core/src/jwks.ts | 3 +- packages/core/tests/authFetch.test.js | 41 ++++++ packages/core/tests/jwks.test.js | 39 +++++ packages/core/tests/registerHandler.test.js | 137 +++++++++++------- .../tests/requestMagicLinkHandler.test.js | 11 ++ packages/core/tests/requestOtpHandler.test.js | 4 +- packages/express/src/handlers/register.ts | 4 +- packages/express/tests/loginOtpRoutes.test.js | 2 +- packages/express/tests/logoutRoutes.test.js | 2 +- .../express/tests/messagingDelivery.test.js | 2 +- .../express/tests/routeErrorHandling.test.js | 39 ++++- packages/fastify/src/routes/authRoutes.ts | 4 +- packages/fastify/tests/parity.test.js | 25 +++- packages/nextjs/package.json | 2 +- packages/nextjs/src/routes/authRoutes.ts | 4 +- packages/nextjs/tests/handler.test.js | 30 +++- packages/nextjs/tests/parity.test.js | 25 +++- pnpm-lock.yaml | 14 +- 25 files changed, 383 insertions(+), 105 deletions(-) create mode 100644 .changeset/upstream-call-parity.md create mode 100644 packages/core/tests/jwks.test.js diff --git a/.changeset/upstream-call-parity.md b/.changeset/upstream-call-parity.md new file mode 100644 index 0000000..a629ed3 --- /dev/null +++ b/.changeset/upstream-call-parity.md @@ -0,0 +1,15 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Calls to the auth API now behave the same as in the Go, Python and Rust adapters. + +- OTP generation and magic link requests use `POST`. The `GET` routes they called are deprecated in the auth API and missing from its adapter manifest. +- Every call times out after 15 seconds (`timeoutMs` on `authFetch`). An auth API that cannot be reached or does not answer in time gets `502 {"error":"upstream_unavailable"}` instead of a hung request or `500 internal_error`. +- The key set is fetched from `authServerUrl` with its path kept, so an auth API mounted under a prefix such as `https://example.com/auth-api` is verified against its own keys. +- `/registration/register` verifies the registration token (issuer, audience, `typ` `ephemeral`, and a `sub` matching the body) before it sets the registration cookie or returns the token to a bearer client, and answers `502 {"error":"invalid_upstream_session"}` when it fails. + +Breaking for code that calls `registerHandler` from `@seamless-auth/core` directly: `RegisterOptions` now requires `audience` and accepts `authServerIssuer`, like `LoginOptions`. The express, fastify and nextjs adapters pass both already. diff --git a/packages/core/README.md b/packages/core/README.md index 25e77c3..6cbbfe1 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -130,7 +130,9 @@ remain for direct imports. - `checkProxyIdentity(...)` – checks a request carries the session a proxied route requires - `buildQueryString(...)` / `buildUpstreamUrl(...)` – build an upstream URL - `signSessionCookie(...)` / `resolveCookieSameSite(...)` – cookie format and policy -- `authFetch(...)` – calls the auth API with the adapter's headers and a tolerant `json()` +- `authFetch(...)` – calls the auth API with the adapter's headers and a tolerant `json()`. A call + that cannot reach the API or takes longer than `timeoutMs` (15 seconds by default) resolves to + `502 {"error":"upstream_unavailable"}` **The adapter manifest** diff --git a/packages/core/package.json b/packages/core/package.json index 01cae96..cf46330 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -51,7 +51,7 @@ "test:watch": "pnpm run build && NODE_OPTIONS=--experimental-vm-modules jest --watch" }, "dependencies": { - "@seamless-auth/types": "^0.34.0", + "@seamless-auth/types": "^0.35.0", "@types/jsonwebtoken": "^9.0.10", "jose": "^6.1.3", "jsonwebtoken": "^9.0.2" diff --git a/packages/core/src/authFetch.ts b/packages/core/src/authFetch.ts index 91bfa48..437f1b5 100644 --- a/packages/core/src/authFetch.ts +++ b/packages/core/src/authFetch.ts @@ -3,6 +3,7 @@ * Licensed under the Apache License, Version 2.0 */ import { getSeamlessLogger } from "./logger.js"; +import { redactSensitiveText } from "./redaction.js"; export interface AuthFetchOptions { method: "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; @@ -12,8 +13,16 @@ export interface AuthFetchOptions { serviceAuthorization?: string; forwardedClientIp?: string; forwardedUserAgent?: string; + /** Defaults to {@link DEFAULT_UPSTREAM_TIMEOUT_MS}. */ + timeoutMs?: number; } +/** How long a call to the auth API may take before it answers 502, matching the other adapters. */ +export const DEFAULT_UPSTREAM_TIMEOUT_MS = 15_000; + +/** The error code a request gets when the auth API could not be reached or timed out. */ +export const UPSTREAM_UNAVAILABLE_ERROR_CODE = "upstream_unavailable"; + export async function authFetch( url: string, options: AuthFetchOptions, @@ -35,28 +44,50 @@ export async function authFetch( // Following a redirect would resend the service token and the forwarded // client headers to wherever a misconfigured proxy in front of the API points. - const response = await fetch(url, { - method: options.method, - headers, - body: options.body ? JSON.stringify(options.body) : undefined, - redirect: "manual", - }); + let response: Response; + try { + response = await fetch(url, { + method: options.method, + headers, + body: options.body ? JSON.stringify(options.body) : undefined, + redirect: "manual", + signal: AbortSignal.timeout( + options.timeoutMs ?? DEFAULT_UPSTREAM_TIMEOUT_MS, + ), + }); + } catch (err) { + getSeamlessLogger().error( + `[SeamlessAuth] The auth API at ${originOf(url)} could not be reached for a ${options.method} (${describeFetchError(err)}).`, + ); + return jsonError(502, UPSTREAM_UNAVAILABLE_ERROR_CODE); + } if (isRedirect(response)) { getSeamlessLogger().error( `[SeamlessAuth] The auth API at ${originOf(url)} answered a ${options.method} with a redirect (HTTP ${response.status}), which is not followed.`, ); - return makeJsonTolerant( - new Response(JSON.stringify({ error: UPSTREAM_REDIRECT_ERROR_CODE }), { - status: 502, - headers: { "Content-Type": "application/json" }, - }), - ); + return jsonError(502, UPSTREAM_REDIRECT_ERROR_CODE); } return makeJsonTolerant(response); } +function jsonError(status: number, error: string): Response { + return makeJsonTolerant( + new Response(JSON.stringify({ error }), { + status, + headers: { "Content-Type": "application/json" }, + }), + ); +} + +function describeFetchError(err: unknown): string { + if (err instanceof Error && err.name === "TimeoutError") { + return "timed out"; + } + return redactSensitiveText(err instanceof Error ? err.message : String(err)); +} + /** The error code a request gets when the auth API answered it with a redirect. */ export const UPSTREAM_REDIRECT_ERROR_CODE = "upstream_redirect"; diff --git a/packages/core/src/handlers/register.ts b/packages/core/src/handlers/register.ts index 7fa0501..53a44a7 100644 --- a/packages/core/src/handlers/register.ts +++ b/packages/core/src/handlers/register.ts @@ -1,8 +1,10 @@ +import type { AuthServerIssuerOption } from "../authServerIssuer.js"; import { authFetch } from "../authFetch.js"; import { readPassthroughFailure } from "../upstreamError.js"; import { EXTERNAL_DELIVERY_HEADERS } from "../apiContract.js"; import type { ResultFailure } from "../result.js"; import type { CookiePayload } from "../ensureCookies.js"; +import { verifyUpstreamSession } from "../upstreamSession.js"; import type { AuthTransport } from "../transport.js"; import { withoutTokens } from "../bodyTokens.js"; @@ -10,8 +12,9 @@ export interface RegisterInput { body: unknown; } -export interface RegisterOptions { +export interface RegisterOptions extends AuthServerIssuerOption { authServerUrl: string; + audience: string; cookieDomain?: string; registrationCookieName: string; externalDelivery?: boolean; @@ -32,6 +35,9 @@ export interface RegisterResult extends ResultFailure { }[]; } +/** The error code a registration gets when the auth API's token fails verification. */ +export const INVALID_UPSTREAM_SESSION_ERROR_CODE = "invalid_upstream_session"; + export async function registerHandler( input: RegisterInput, opts: RegisterOptions, @@ -58,6 +64,18 @@ export async function registerHandler( }; } + try { + await verifyUpstreamSession( + data, + opts.authServerUrl, + opts.audience, + opts.authServerIssuer, + "ephemeral", + ); + } catch { + return { status: 502, errorCode: INVALID_UPSTREAM_SESSION_ERROR_CODE }; + } + if (opts.transport === "bearer") { return { status: 200, body: data }; } diff --git a/packages/core/src/handlers/requestMagicLinkHandler.ts b/packages/core/src/handlers/requestMagicLinkHandler.ts index ed163b8..c631414 100644 --- a/packages/core/src/handlers/requestMagicLinkHandler.ts +++ b/packages/core/src/handlers/requestMagicLinkHandler.ts @@ -39,7 +39,7 @@ export async function requestMagicLinkHandler( ); const up = await authFetch(url, { - method: "GET", + method: "POST", authorization: input.authorization, forwardedClientIp: opts.forwardedClientIp, forwardedUserAgent: opts.forwardedUserAgent, diff --git a/packages/core/src/handlers/requestOtpHandler.ts b/packages/core/src/handlers/requestOtpHandler.ts index 6347463..8f81e72 100644 --- a/packages/core/src/handlers/requestOtpHandler.ts +++ b/packages/core/src/handlers/requestOtpHandler.ts @@ -41,7 +41,7 @@ export async function requestOtpHandler( : "otp/generate-phone-otp"; const up = await authFetch(`${opts.authServerUrl}/${path}`, { - method: "GET", + method: "POST", authorization: input.authorization, forwardedClientIp: opts.forwardedClientIp, forwardedUserAgent: opts.forwardedUserAgent, diff --git a/packages/core/src/jwks.ts b/packages/core/src/jwks.ts index a7b3998..bd95024 100644 --- a/packages/core/src/jwks.ts +++ b/packages/core/src/jwks.ts @@ -9,7 +9,8 @@ const jwksByUrl = new Map>(); export function getAuthServerJwks( authServerUrl: string, ): ReturnType { - const jwksUrl = new URL("/.well-known/jwks.json", authServerUrl).toString(); + // Appended rather than resolved, so an auth server mounted under a path keeps its prefix. + const jwksUrl = `${authServerUrl.replace(/\/+$/, "")}/.well-known/jwks.json`; let jwks = jwksByUrl.get(jwksUrl); if (!jwks) { diff --git a/packages/core/tests/authFetch.test.js b/packages/core/tests/authFetch.test.js index 2c20569..bf1350a 100644 --- a/packages/core/tests/authFetch.test.js +++ b/packages/core/tests/authFetch.test.js @@ -168,4 +168,45 @@ describe("authFetch against a redirecting upstream", () => { expect(elsewhere).not.toHaveBeenCalled(); }, ); + + it("answers 502 upstream_unavailable when the API does not answer in time", async () => { + const { authFetch, UPSTREAM_UNAVAILABLE_ERROR_CODE } = await import( + "../dist/authFetch.js" + ); + const api = await listen(() => {}); + + const response = await authFetch(`${api}/login`, { + method: "POST", + timeoutMs: 50, + }); + + expect(response.status).toBe(502); + expect(await response.json()).toEqual({ error: UPSTREAM_UNAVAILABLE_ERROR_CODE }); + }); + + it("answers 502 upstream_unavailable when the API cannot be reached", async () => { + const { authFetch, UPSTREAM_UNAVAILABLE_ERROR_CODE } = await import( + "../dist/authFetch.js" + ); + const api = await listen(() => {}); + await new Promise((resolve) => servers.pop().close(resolve)); + + const response = await authFetch(`${api}/login`, { method: "POST" }); + + expect(response.status).toBe(502); + expect(await response.json()).toEqual({ error: UPSTREAM_UNAVAILABLE_ERROR_CODE }); + }); + + it("applies a timeout to every call by default", async () => { + const { authFetch } = await import("../dist/authFetch.js"); + const original = global.fetch; + global.fetch = jest.fn().mockResolvedValue({ ok: true, status: 200 }); + + try { + await authFetch("https://auth.example.com/login", { method: "POST" }); + expect(global.fetch.mock.calls[0][1].signal).toBeInstanceOf(AbortSignal); + } finally { + global.fetch = original; + } + }); }); diff --git a/packages/core/tests/jwks.test.js b/packages/core/tests/jwks.test.js new file mode 100644 index 0000000..6baf0ed --- /dev/null +++ b/packages/core/tests/jwks.test.js @@ -0,0 +1,39 @@ +import { jest } from "@jest/globals"; +import { exportJWK, generateKeyPair, SignJWT } from "jose"; + +describe("auth server JWKS", () => { + const originalFetch = global.fetch; + + afterEach(() => { + global.fetch = originalFetch; + }); + + // Resolving "/.well-known/jwks.json" against the URL dropped the path, so an + // API mounted under a prefix had its keys fetched from the bare origin. + it.each([ + ["https://a.example.com/auth-api", "https://a.example.com/auth-api"], + ["https://b.example.com/auth-api/", "https://b.example.com/auth-api"], + ])("keeps the path prefix of %s", async (authServerUrl, base) => { + const { verifyWithAuthServerJwks } = await import("../dist/jwks.js"); + const { privateKey, publicKey } = await generateKeyPair("RS256"); + const jwk = { ...(await exportJWK(publicKey)), alg: "RS256", kid: "k1" }; + const token = await new SignJWT({ sub: "user-123" }) + .setProtectedHeader({ alg: "RS256", kid: "k1" }) + .setExpirationTime("5m") + .sign(privateKey); + + global.fetch = jest.fn(async (url) => { + if (url.toString() === `${base}/.well-known/jwks.json`) { + return new Response(JSON.stringify({ keys: [jwk] }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + } + throw new Error(`Unexpected fetch URL: ${url}`); + }); + + const payload = await verifyWithAuthServerJwks(token, authServerUrl, {}); + + expect(payload.sub).toBe("user-123"); + }); +}); diff --git a/packages/core/tests/registerHandler.test.js b/packages/core/tests/registerHandler.test.js index 34ee708..06815b4 100644 --- a/packages/core/tests/registerHandler.test.js +++ b/packages/core/tests/registerHandler.test.js @@ -1,96 +1,135 @@ import { jest } from "@jest/globals"; +import { exportJWK, generateKeyPair, SignJWT } from "jose"; -function createJsonResponse(status, body) { - return { - ok: status >= 200 && status < 300, +const AUTH = "https://register.example.com"; + +function jsonResponse(status, body) { + return new Response(JSON.stringify(body), { status, - json: async () => body, + headers: { "Content-Type": "application/json" }, + }); +} + +const keys = generateKeyPair("RS256").then(async ({ privateKey, publicKey }) => { + const jwk = await exportJWK(publicKey); + Object.assign(jwk, { alg: "RS256", kid: "register-key", use: "sig" }); + return { privateKey, jwk }; +}); + +async function sign(claims, issuer = AUTH) { + const { privateKey } = await keys; + return new SignJWT(claims) + .setProtectedHeader({ alg: "RS256", kid: "register-key" }) + .setIssuer(issuer) + .setSubject(claims.sub) + .setExpirationTime("5m") + .sign(privateKey); +} + +async function mockRegister(body) { + const { jwk } = await keys; + global.fetch = jest.fn(async (url) => { + if (url === `${AUTH}/registration/register`) { + return jsonResponse(200, body); + } + if (url === `${AUTH}/.well-known/jwks.json`) { + return jsonResponse(200, { keys: [jwk] }); + } + throw new Error(`Unexpected fetch URL: ${url}`); + }); +} + +async function registered(claims = { sub: "user-123", typ: "ephemeral" }) { + return { + message: "Success", + sub: "user-123", + token: await sign(claims), + ttl: 300, }; } +const OPTS = { authServerUrl: AUTH, registrationCookieName: "registration" }; + describe("registerHandler", () => { const originalFetch = global.fetch; - beforeEach(() => { - global.fetch = jest.fn(); - }); - afterEach(() => { global.fetch = originalFetch; }); - it("stores the upstream ephemeral token in the registration cookie", async () => { + it("stores the verified ephemeral token in the registration cookie", async () => { const { registerHandler } = await import("../dist/handlers/register.js"); - - global.fetch.mockResolvedValue( - createJsonResponse(200, { - message: "Success", - sub: "user-123", - token: "ephemeral-token", - ttl: 300, - }), - ); + const body = await registered(); + await mockRegister(body); const result = await registerHandler( - { - body: { email: "user@example.com", phone: "+14155552671" }, - }, - { - authServerUrl: "https://auth.example.com", - registrationCookieName: "registration", - }, + { body: { email: "user@example.com", phone: "+14155552671" } }, + OPTS, ); expect(result.status).toBe(200); expect(result.setCookies).toEqual([ { name: "registration", - value: { sub: "user-123", token: "ephemeral-token" }, + value: { sub: "user-123", token: body.token }, ttl: 300, domain: undefined, }, ]); }); - const REGISTERED = { - message: "Success", - sub: "user-123", - token: "ephemeral-token", - ttl: 300, - }; - // The cookie holds the registration token. Returning it in the body as well // let page scripts read the token the httpOnly cookie exists to hide (#202). it("keeps the registration token out of a cookie-transport body", async () => { const { registerHandler } = await import("../dist/handlers/register.js"); - global.fetch.mockResolvedValue(createJsonResponse(200, REGISTERED)); + const body = await registered(); + await mockRegister(body); - const result = await registerHandler( - { body: { email: "user@example.com" } }, - { - authServerUrl: "https://auth.example.com", - registrationCookieName: "registration", - }, - ); + const result = await registerHandler({ body: { email: "user@example.com" } }, OPTS); expect(result.body).toEqual({ message: "Success", sub: "user-123", ttl: 300 }); - expect(result.setCookies[0].value.token).toBe("ephemeral-token"); + expect(result.setCookies[0].value.token).toBe(body.token); }); it("returns the whole body in bearer transport", async () => { const { registerHandler } = await import("../dist/handlers/register.js"); - global.fetch.mockResolvedValue(createJsonResponse(200, REGISTERED)); + const body = await registered(); + await mockRegister(body); const result = await registerHandler( { body: { email: "user@example.com" } }, - { - authServerUrl: "https://auth.example.com", - registrationCookieName: "registration", - transport: "bearer", - }, + { ...OPTS, transport: "bearer" }, ); - expect(result.body).toEqual(REGISTERED); + expect(result.body).toEqual(body); expect(result.setCookies).toBeUndefined(); }); + + it.each([ + ["an unsigned token", async () => ({ ...(await registered()), token: "not-a-jwt" })], + ["an access token", () => registered({ sub: "user-123", typ: "access" })], + [ + "a token for another subject", + async () => ({ ...(await registered()), sub: "someone-else" }), + ], + [ + "a token from another issuer", + async () => ({ + ...(await registered()), + token: await sign({ sub: "user-123", typ: "ephemeral" }, "https://evil.example.com"), + }), + ], + ])("answers 502 and sets no cookie for %s", async (_label, build) => { + const { registerHandler } = await import("../dist/handlers/register.js"); + await mockRegister(await build()); + + for (const transport of [undefined, "bearer"]) { + const result = await registerHandler( + { body: { email: "user@example.com" } }, + { ...OPTS, transport }, + ); + + expect(result).toEqual({ status: 502, errorCode: "invalid_upstream_session" }); + } + }); }); diff --git a/packages/core/tests/requestMagicLinkHandler.test.js b/packages/core/tests/requestMagicLinkHandler.test.js index 300cc9f..5a61944 100644 --- a/packages/core/tests/requestMagicLinkHandler.test.js +++ b/packages/core/tests/requestMagicLinkHandler.test.js @@ -43,6 +43,17 @@ describe("requestMagicLinkHandler", () => { expect(requestedUrl()).toBe("https://auth.example.com/magic-link"); }); + it("requests the link with POST, not the deprecated GET", async () => { + const requestMagicLinkHandler = await load(); + + await requestMagicLinkHandler( + {}, + { authServerUrl: "https://auth.example.com" }, + ); + + expect(global.fetch.mock.calls[0][1].method).toBe("POST"); + }); + it("forwards a requested target as a query parameter", async () => { const requestMagicLinkHandler = await load(); diff --git a/packages/core/tests/requestOtpHandler.test.js b/packages/core/tests/requestOtpHandler.test.js index 5451827..c461102 100644 --- a/packages/core/tests/requestOtpHandler.test.js +++ b/packages/core/tests/requestOtpHandler.test.js @@ -32,7 +32,7 @@ describe("requestOtpHandler", () => { expect(global.fetch).toHaveBeenCalledWith( "https://auth.example.com/otp/generate-email-otp", - expect.objectContaining({ method: "GET" }), + expect.objectContaining({ method: "POST" }), ); }); @@ -52,7 +52,7 @@ describe("requestOtpHandler", () => { expect(global.fetch).toHaveBeenCalledWith( "https://auth.example.com/otp/generate-login-phone-otp", - expect.objectContaining({ method: "GET" }), + expect.objectContaining({ method: "POST" }), ); }); diff --git a/packages/express/src/handlers/register.ts b/packages/express/src/handlers/register.ts index 68db96c..790a2d1 100644 --- a/packages/express/src/handlers/register.ts +++ b/packages/express/src/handlers/register.ts @@ -20,6 +20,8 @@ export async function register( { body: req.body }, { authServerUrl: opts.authServerUrl, + audience: opts.audience, + authServerIssuer: opts.authServerIssuer, cookieDomain: opts.cookieDomain, registrationCookieName: opts.registrationCookieName!, transport: transportOf(req), @@ -32,7 +34,7 @@ export async function register( }, ); - if (result.errorBody) { + if (result.errorBody || result.errorCode) { return respond(res, result, opts); } diff --git a/packages/express/tests/loginOtpRoutes.test.js b/packages/express/tests/loginOtpRoutes.test.js index 9cf87e0..6811cd9 100644 --- a/packages/express/tests/loginOtpRoutes.test.js +++ b/packages/express/tests/loginOtpRoutes.test.js @@ -70,7 +70,7 @@ describe("login OTP routes", () => { expect(global.fetch).toHaveBeenCalledWith( "https://auth.example.com/otp/generate-login-email-otp", expect.objectContaining({ - method: "GET", + method: "POST", headers: expect.objectContaining({ Authorization: "Bearer ephemeral-token", "x-seamless-service-token": expect.not.stringContaining("ephemeral-token"), diff --git a/packages/express/tests/logoutRoutes.test.js b/packages/express/tests/logoutRoutes.test.js index ff93abb..1552707 100644 --- a/packages/express/tests/logoutRoutes.test.js +++ b/packages/express/tests/logoutRoutes.test.js @@ -124,7 +124,7 @@ describe("logout routes", () => { expect(res.status).toBe(200); expect(global.fetch).toHaveBeenCalledWith( "https://auth.example.com/magic-link", - expect.objectContaining({ method: "GET" }), + expect.objectContaining({ method: "POST" }), ); }); diff --git a/packages/express/tests/messagingDelivery.test.js b/packages/express/tests/messagingDelivery.test.js index 4f56ba0..d29fec7 100644 --- a/packages/express/tests/messagingDelivery.test.js +++ b/packages/express/tests/messagingDelivery.test.js @@ -96,7 +96,7 @@ describe("messaging delivery routes", () => { expect(global.fetch).toHaveBeenCalledWith( "https://auth.example.com/magic-link", expect.objectContaining({ - method: "GET", + method: "POST", headers: expect.objectContaining({ "Content-Type": "application/json", "x-seamless-auth-delivery-mode": "external", diff --git a/packages/express/tests/routeErrorHandling.test.js b/packages/express/tests/routeErrorHandling.test.js index d62cde7..3b3101b 100644 --- a/packages/express/tests/routeErrorHandling.test.js +++ b/packages/express/tests/routeErrorHandling.test.js @@ -37,6 +37,18 @@ function createApp() { return app; } +// A response whose body cannot be read, which no handler expects. +function unreadable(message) { + return { + ok: true, + status: 200, + headers: new Headers(), + text: async () => { + throw new Error(message); + }, + }; +} + describe("route error handling", () => { const originalFetch = global.fetch; let errorSpy; @@ -51,7 +63,7 @@ describe("route error handling", () => { errorSpy.mockRestore(); }); - it("answers 500 instead of hanging when an upstream request rejects", async () => { + it("answers 502 upstream_unavailable instead of hanging when the API cannot be reached", async () => { global.fetch.mockRejectedValue(new Error("network down")); const res = await request(createApp()) @@ -59,11 +71,26 @@ describe("route error handling", () => { .set("Cookie", createPreAuthCookie()) .timeout({ response: 5000 }); - expect(res.status).toBe(500); + expect(res.status).toBe(502); + expect(res.body).toEqual({ error: "upstream_unavailable" }); + }); + + it("redacts sensitive values when it logs an unreachable API", async () => { + global.fetch.mockRejectedValue( + new Error("upstream rejected Bearer sk-secret-token-value"), + ); + + await request(createApp()) + .get("/auth/magic-link/check") + .set("Cookie", createPreAuthCookie()); + + const [logged] = errorSpy.mock.calls[0]; + expect(logged).not.toContain("sk-secret-token-value"); + expect(logged).toContain("[REDACTED]"); }); it("returns a JSON error body without leaking internals", async () => { - global.fetch.mockRejectedValue(new Error("SECRET_INTERNAL_DETAIL")); + global.fetch.mockResolvedValue(unreadable("SECRET_INTERNAL_DETAIL")); const res = await request(createApp()) .get("/auth/magic-link/check") @@ -116,7 +143,7 @@ describe("route error handling", () => { }); it("logs the underlying error server side", async () => { - global.fetch.mockRejectedValue(new Error("network down")); + global.fetch.mockResolvedValue(unreadable("network down")); await request(createApp()) .get("/auth/magic-link/check") @@ -129,8 +156,8 @@ describe("route error handling", () => { }); it("redacts sensitive values in the logged error", async () => { - global.fetch.mockRejectedValue( - new Error("upstream rejected Bearer sk-secret-token-value"), + global.fetch.mockResolvedValue( + unreadable("upstream rejected Bearer sk-secret-token-value"), ); await request(createApp()) diff --git a/packages/fastify/src/routes/authRoutes.ts b/packages/fastify/src/routes/authRoutes.ts index 9ea0382..fb119d1 100644 --- a/packages/fastify/src/routes/authRoutes.ts +++ b/packages/fastify/src/routes/authRoutes.ts @@ -110,6 +110,8 @@ export function registerAuthRoutes( { body: req.body }, { ...common(req), + audience: opts.audience, + authServerIssuer: opts.authServerIssuer, cookieDomain: opts.cookieDomain, registrationCookieName: opts.registrationCookieName, transport: transportOf(req), @@ -118,7 +120,7 @@ export function registerAuthRoutes( }, ); - if (result.errorBody) { + if (result.errorBody || result.errorCode) { return respond(reply, result, opts); } diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index ec69d44..ffbe450 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -48,6 +48,19 @@ const REFRESHED_ACCESS_TOKEN = await accessToken({ roles: ["admin"], }); +// What the auth API answers a registration with: an ephemeral token the adapter +// verifies before it sets the registration cookie. +const REGISTRATION_TOKEN = await accessToken({ sub: "user-123", typ: "ephemeral" }); + +// Serves the key set alongside the response, for routes that verify the token. +function withJwks(response) { + return jest.fn(async (url) => + String(url).endsWith("/.well-known/jwks.json") + ? upstream(200, { keys: [jwk] }) + : response, + ); +} + function upstream(status, body) { return { ok: status >= 200 && status < 300, status, json: async () => body }; } @@ -156,10 +169,10 @@ function refreshUpstream(refreshBody) { } async function bothAdapters(scenario, upstreamResponse) { - global.fetch = jest.fn(async () => upstreamResponse); + global.fetch = withJwks(upstreamResponse); const fastify = await viaFastify(scenario); - global.fetch = jest.fn(async () => upstreamResponse); + global.fetch = withJwks(upstreamResponse); const expressResult = await viaExpress(scenario); return { fastify, express: expressResult }; @@ -637,7 +650,7 @@ describe("fastify and express adapters agree", () => { const upstreamResponse = upstream(200, { message: "Registration started", sub: "user-123", - token: "registration-token", + token: REGISTRATION_TOKEN, ttl: "300", }); @@ -1561,13 +1574,13 @@ describe("no adapter returns an ephemeral token in a cookie-transport body", () const registered = upstream(200, { message: "Success", sub: "user-123", - token: "registration-token", + token: REGISTRATION_TOKEN, ttl: 300, }); - global.fetch = jest.fn(async () => registered); + global.fetch = withJwks(registered); const fastify = await viaFastify(scenario); - global.fetch = jest.fn(async () => registered); + global.fetch = withJwks(registered); const expressResult = await viaExpress(scenario); expect(fastify.status).toBe(200); diff --git a/packages/nextjs/package.json b/packages/nextjs/package.json index 1dca1f9..bf3ff7b 100644 --- a/packages/nextjs/package.json +++ b/packages/nextjs/package.json @@ -53,7 +53,7 @@ }, "dependencies": { "@seamless-auth/core": "workspace:^", - "@seamless-auth/types": "^0.34.0", + "@seamless-auth/types": "^0.35.0", "cookie": "^1.1.1" }, "devDependencies": { diff --git a/packages/nextjs/src/routes/authRoutes.ts b/packages/nextjs/src/routes/authRoutes.ts index eea7717..0b86c4b 100644 --- a/packages/nextjs/src/routes/authRoutes.ts +++ b/packages/nextjs/src/routes/authRoutes.ts @@ -186,6 +186,8 @@ export const AUTH_ROUTES: Route[] = [ { body: ctx.body }, { ...common(ctx, opts), + audience: opts.audience, + authServerIssuer: opts.authServerIssuer, cookieDomain: opts.cookieDomain, registrationCookieName: opts.registrationCookieName, transport: ctx.transport, @@ -194,7 +196,7 @@ export const AUTH_ROUTES: Route[] = [ }, ); - if (result.errorBody) { + if (result.errorBody || result.errorCode) { return result; } diff --git a/packages/nextjs/tests/handler.test.js b/packages/nextjs/tests/handler.test.js index 62b713c..4d3f351 100644 --- a/packages/nextjs/tests/handler.test.js +++ b/packages/nextjs/tests/handler.test.js @@ -291,11 +291,20 @@ describe("createSeamlessAuthHandler", () => { }); describe("errors", () => { + // A response whose body cannot be read, which no route expects. + const unreadable = (message) => ({ + ok: true, + status: 200, + text: async () => { + throw new Error(message); + }, + }); + it("answers 500 without leaking the failure", async () => { const spy = jest.spyOn(console, "error").mockImplementation(() => {}); - global.fetch = jest.fn(async () => { - throw new Error("upstream exploded at /srv/secret/path"); - }); + global.fetch = jest.fn(async () => + unreadable("upstream exploded at /srv/secret/path"), + ); const res = await call("/auth/oauth/providers"); @@ -304,6 +313,19 @@ describe("createSeamlessAuthHandler", () => { spy.mockRestore(); }); + it("answers 502 upstream_unavailable when the API cannot be reached", async () => { + const spy = jest.spyOn(console, "error").mockImplementation(() => {}); + global.fetch = jest.fn(async () => { + throw new Error("connect ECONNREFUSED"); + }); + + const res = await call("/auth/oauth/providers"); + + expect(res.status).toBe(502); + expect(await res.json()).toEqual({ error: "upstream_unavailable" }); + spy.mockRestore(); + }); + // ensureCookies rotates the refresh token before the route runs. If the // route then fails and the rotated cookies are dropped, the browser keeps a // spent refresh token and its next refresh revokes the session. @@ -314,7 +336,7 @@ describe("createSeamlessAuthHandler", () => { if (String(url).endsWith("/.well-known/jwks.json")) { return upstream(200, { keys: [jwk] }); } - throw new Error("upstream down"); + return unreadable("upstream down"); }); const res = await call("/auth/users/me", { diff --git a/packages/nextjs/tests/parity.test.js b/packages/nextjs/tests/parity.test.js index b9fbccc..058122a 100644 --- a/packages/nextjs/tests/parity.test.js +++ b/packages/nextjs/tests/parity.test.js @@ -57,6 +57,19 @@ const REFRESHED_ACCESS_TOKEN = await accessToken({ roles: ["admin"], }); +// What the auth API answers a registration with: an ephemeral token the adapter +// verifies before it sets the registration cookie. +const REGISTRATION_TOKEN = await accessToken({ sub: "user-123", typ: "ephemeral" }); + +// Serves the key set alongside the response, for routes that verify the token. +function withJwks(response) { + return jest.fn(async (url) => + String(url).endsWith("/.well-known/jwks.json") + ? upstream(200, { keys: [jwk] }) + : response, + ); +} + function upstream(status, body) { return { ok: status >= 200 && status < 300, status, json: async () => body }; } @@ -159,10 +172,10 @@ function refreshUpstream(refreshBody) { } async function bothAdapters(scenario, upstreamResponse) { - global.fetch = jest.fn(async () => upstreamResponse); + global.fetch = withJwks(upstreamResponse); const next = await viaNext(scenario); - global.fetch = jest.fn(async () => upstreamResponse); + global.fetch = withJwks(upstreamResponse); const expressResult = await viaExpress(scenario); return { next, express: expressResult }; @@ -613,7 +626,7 @@ describe("next.js and express adapters agree", () => { const upstreamResponse = upstream(200, { message: "Registration started", sub: "user-123", - token: "registration-token", + token: REGISTRATION_TOKEN, ttl: "300", }); @@ -1530,13 +1543,13 @@ describe("no adapter returns an ephemeral token in a cookie-transport body", () const registered = upstream(200, { message: "Success", sub: "user-123", - token: "registration-token", + token: REGISTRATION_TOKEN, ttl: 300, }); - global.fetch = jest.fn(async () => registered); + global.fetch = withJwks(registered); const next = await viaNext(scenario); - global.fetch = jest.fn(async () => registered); + global.fetch = withJwks(registered); const expressResult = await viaExpress(scenario); expect(next.status).toBe(200); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 63b28d2..28b1344 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -39,8 +39,8 @@ importers: packages/core: dependencies: '@seamless-auth/types': - specifier: ^0.34.0 - version: 0.34.0 + specifier: ^0.35.0 + version: 0.35.0 '@types/jsonwebtoken': specifier: ^9.0.10 version: 9.0.10 @@ -159,8 +159,8 @@ importers: specifier: workspace:^ version: link:../core '@seamless-auth/types': - specifier: ^0.34.0 - version: 0.34.0 + specifier: ^0.35.0 + version: 0.35.0 cookie: specifier: ^1.1.1 version: 1.1.1 @@ -1116,8 +1116,8 @@ packages: cpu: [x64] os: [win32] - '@seamless-auth/types@0.34.0': - resolution: {integrity: sha512-bTgZkEdpXwAejxwnA1J8vTGAq+FW9L6J4n97q2MI6SqWWIOqXyaF6zMTkzeKqNTvfgF0NIn2DHh9qXL+92FN/A==} + '@seamless-auth/types@0.35.0': + resolution: {integrity: sha512-T/1E72d43M+Bzivgo9k8yxaSBRpaXO9XJYlsenCwYVPb71Qnis+w10nXS7X/Qh/zKp7otOjRys2T3jMgP1L6Eg==} engines: {node: '>=22'} '@sinclair/typebox@0.27.10': @@ -3736,7 +3736,7 @@ snapshots: '@rollup/rollup-win32-x64-msvc@4.60.4': optional: true - '@seamless-auth/types@0.34.0': + '@seamless-auth/types@0.35.0': dependencies: zod: 4.6.5