From c5b290236e2f09961b3c092a76ff966964f5f0a3 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sun, 1 Nov 2026 19:11:37 -0500 Subject: [PATCH] refactor: serve passthrough routes from the manifest and align guard and 404 responses Delete the static passthrough tables in the three bindings so those routes come from the adapter manifest, which strips tokens from cookie-transport bodies. Express and Fastify apply the recovery enrollment refresh to manifest routes, as Next.js already did. POST /users/update is dropped: the auth API has no such route. requireAuth answers 401 unauthenticated and requireRole answers 403 insufficient_role without echoing roles. Express answers unmatched auth paths with a JSON 404. OTP verify, magic link verify and poll, and organization switch strip tokens from incomplete cookie-transport bodies. setSeamlessLogger is re-exported from each binding, and the bindings log through it. A manifest workflow checks the bundled manifest against the auth API's. Closes #263 --- .../manifest-passthrough-guard-codes.md | 21 ++ .github/workflows/manifest.yml | 27 ++ AGENTS.md | 4 +- packages/core/src/guards.ts | 28 +- .../pollMagicLinkConfirmationHandler.ts | 3 +- .../src/handlers/switchOrganizationHandler.ts | 3 +- .../src/handlers/verifyLoginOtpHandler.ts | 3 +- .../src/handlers/verifyMagicLinkHandler.ts | 5 +- .../core/tests/authenticateBearer.test.js | 16 +- .../core/tests/incompleteSessionBody.test.js | 80 +++++ packages/express/README.md | 32 ++ packages/express/src/createServer.ts | 315 +----------------- .../express/src/handlers/verifyMagicLink.ts | 1 + packages/express/src/index.ts | 7 +- .../src/internal/buildForwardedClientIp.ts | 3 +- .../express/src/internal/validateSecrets.ts | 4 +- .../express/src/middleware/requireAuth.ts | 8 +- .../tests/cookieRequirementRoutes.test.js | 31 +- packages/express/tests/logger.test.js | 31 ++ packages/express/tests/manifestRoutes.test.js | 46 +++ .../express/tests/requireAuth.bearer.test.js | 8 +- packages/express/tests/requireRole.test.js | 20 +- .../express/tests/routeErrorHandling.test.js | 15 +- packages/express/tests/stepUpProxy.test.js | 2 +- packages/fastify/README.md | 25 ++ packages/fastify/src/index.ts | 7 +- .../src/internal/buildForwardedClientIp.ts | 5 +- packages/fastify/src/plugin.ts | 70 +--- packages/fastify/src/routes/authRoutes.ts | 1 + packages/fastify/src/routes/proxyRoutes.ts | 306 ----------------- .../fastify/tests/dotSegmentParams.test.js | 14 + packages/fastify/tests/guards.test.js | 13 +- packages/fastify/tests/logger.test.js | 36 ++ packages/fastify/tests/parity.test.js | 34 +- packages/nextjs/README.md | 31 +- packages/nextjs/src/guards.ts | 11 +- packages/nextjs/src/handler.ts | 61 +--- packages/nextjs/src/index.ts | 7 +- packages/nextjs/src/internal/clientIp.ts | 3 +- packages/nextjs/src/routes/authRoutes.ts | 1 + packages/nextjs/src/routes/proxyRoutes.ts | 292 ---------------- packages/nextjs/tests/handler.test.js | 4 +- packages/nextjs/tests/logger.test.js | 32 ++ packages/nextjs/tests/parity.test.js | 35 +- scripts/check-adapter-manifest.mjs | 67 ++++ 45 files changed, 631 insertions(+), 1137 deletions(-) create mode 100644 .changeset/manifest-passthrough-guard-codes.md create mode 100644 .github/workflows/manifest.yml create mode 100644 packages/core/tests/incompleteSessionBody.test.js create mode 100644 packages/express/tests/logger.test.js delete mode 100644 packages/fastify/src/routes/proxyRoutes.ts create mode 100644 packages/fastify/tests/logger.test.js delete mode 100644 packages/nextjs/src/routes/proxyRoutes.ts create mode 100644 packages/nextjs/tests/logger.test.js create mode 100644 scripts/check-adapter-manifest.mjs diff --git a/.changeset/manifest-passthrough-guard-codes.md b/.changeset/manifest-passthrough-guard-codes.md new file mode 100644 index 0000000..acf690e --- /dev/null +++ b/.changeset/manifest-passthrough-guard-codes.md @@ -0,0 +1,21 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": minor +"@seamless-auth/fastify": minor +"@seamless-auth/nextjs": minor +--- + +Passthrough routes come from the auth API's adapter manifest, and guard and not-found responses match the Go, Python and Rust adapters. + +Breaking response changes: + +- `requireAuth` answers `401 {"error":"unauthenticated"}` for a missing, invalid or expired session and for an ephemeral token. It used to answer `"Failed to find authentication token required"` or `"Invalid or expired session"`. +- `requireRole` answers `403 {"error":"insufficient_role"}` when no listed role is granted, including for a user with no roles. The body no longer carries `required` and `actual`, so a client never sees the user's roles. A request no guard authenticated gets `401 {"error":"unauthenticated"}`. +- In Express, a path under the auth router that no route and no manifest entry matches answers `404 {"error":"not_found"}` instead of calling `next()`. Fastify and Next.js already did. +- `POST /users/update` is no longer served. The auth API has no such route, so it only ever answered the API's 404. + +The static passthrough tables are gone. Organizations, step-up, TOTP, credentials, OAuth provider configuration, admin organizations, enrollment, review accounts, the auth event export and integrity check, and the coverage report are served from the manifest, like any other route the adapter has no handler for. In cookie transport their bodies no longer carry `token` or `refreshToken`. A dot segment in one of their path parameters answers `404 not_found` rather than `400 bad_request`, and no upstream call is made either way. A recovery session that enrolls an authenticator app is still rotated right away. + +OTP verification, magic link polling and verification, and organization switching also drop tokens from a cookie-transport body when the auth API's answer is not a complete session. + +`setSeamlessLogger` and the `SeamlessLogger` type are exported from `@seamless-auth/express`, `@seamless-auth/fastify` and `@seamless-auth/nextjs`, and every warning and error those packages log goes through it. diff --git a/.github/workflows/manifest.yml b/.github/workflows/manifest.yml new file mode 100644 index 0000000..a5b7482 --- /dev/null +++ b/.github/workflows/manifest.yml @@ -0,0 +1,27 @@ +name: manifest + +# The bundled adapter manifest must list the routes seamless-auth-api main +# publishes. The schedule catches API route changes that land between PRs here. +on: + push: + branches: [main] + pull_request: + schedule: + - cron: '17 6 * * *' + workflow_dispatch: + +permissions: + contents: read + +jobs: + drift: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0 + with: + node-version: 22 + + - run: node scripts/check-adapter-manifest.mjs diff --git a/AGENTS.md b/AGENTS.md index d5952e0..93670d5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,7 +116,8 @@ conformance/ handler of its own for, through `handleManifestRoute` in core. Add a dedicated handler only for behaviour the manifest cannot describe. Refresh the bundled fallback with `node scripts/sync-adapter-manifest.mjs` after the API's - manifest changes. + manifest changes. `node scripts/check-adapter-manifest.mjs` (the `manifest` + workflow, also run daily) fails while the two list different routes. ## Tooling @@ -125,6 +126,7 @@ conformance/ | Install | `pnpm install` | | Build | `pnpm build` | | Test | `pnpm test` | +| Manifest | `node scripts/check-adapter-manifest.mjs` | - Supported Node is `>=22`. CI tests 22, 24, and the latest release; `.nvmrc` pins Node 24 for development and releases. Run `nvm use` and diff --git a/packages/core/src/guards.ts b/packages/core/src/guards.ts index 40f3e35..91fd5d4 100644 --- a/packages/core/src/guards.ts +++ b/packages/core/src/guards.ts @@ -18,6 +18,12 @@ import { verifyCookieJwt } from "./verifyCookieJwt.js"; */ export type { SeamlessAuthUser } from "@seamless-auth/types"; +/** What every first-party guard answers for a missing, invalid or pre-auth credential. */ +export const UNAUTHENTICATED = "unauthenticated"; + +/** What a role guard answers for a session that holds none of the required roles. */ +export const INSUFFICIENT_ROLE = "insufficient_role"; + /** * A guard's decision to refuse a request. Adapters render it as * `{ error: errorCode, ...detail }` and log `warn` when it is set. @@ -206,7 +212,7 @@ export function authenticateCookie(input: CookieAuthInput): CookieAuthResult { return { rejection: { status: 401, - errorCode: "Failed to find authentication token required", + errorCode: UNAUTHENTICATED, warn: "Missing expected auth cookie.", }, }; @@ -216,7 +222,7 @@ export function authenticateCookie(input: CookieAuthInput): CookieAuthResult { if (!payload || !payload.sub || !holdsAccessToken(payload.token)) { return { - rejection: { status: 401, errorCode: "Invalid or expired session" }, + rejection: { status: 401, errorCode: UNAUTHENTICATED }, }; } @@ -255,7 +261,7 @@ export type AuthResult = CookieAuthResult; function missingTokenRejection(warn: string): GuardRejection { return { status: 401, - errorCode: "Failed to find authentication token required", + errorCode: UNAUTHENTICATED, warn, }; } @@ -291,7 +297,7 @@ export async function authenticateBearer( if (!claims) { return { - rejection: { status: 401, errorCode: "Invalid or expired session" }, + rejection: { status: 401, errorCode: UNAUTHENTICATED }, }; } @@ -381,7 +387,7 @@ async function verifyCookieAccessToken( if (!claims || claims.sub !== user.id) { return { - rejection: { status: 401, errorCode: "Invalid or expired session" }, + rejection: { status: 401, errorCode: UNAUTHENTICATED }, }; } @@ -408,19 +414,17 @@ export function authorizeRoles( const roles = Array.isArray(requiredRoles) ? requiredRoles : [requiredRoles]; if (!user) { - return { status: 401, errorCode: "Authentication required" }; + return { status: 401, errorCode: UNAUTHENTICATED }; } if (!Array.isArray(user.roles)) { - return { status: 403, errorCode: "User has no roles assigned" }; + return { status: 403, errorCode: INSUFFICIENT_ROLE }; } + // The body names neither the user's roles nor the required ones: the client + // gains nothing from them, and they map out the app's authorization model. if (!hasScopedRole(user.roles, roles)) { - return { - status: 403, - errorCode: "Insufficient role", - detail: { required: roles, actual: user.roles }, - }; + return { status: 403, errorCode: INSUFFICIENT_ROLE }; } return undefined; diff --git a/packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts b/packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts index a2301a9..2e424f0 100644 --- a/packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts +++ b/packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts @@ -1,4 +1,5 @@ import type { AuthServerIssuerOption } from "../authServerIssuer.js"; +import { withoutTokens } from "../bodyTokens.js"; import { authFetch } from "../authFetch.js"; import { sessionResult } from "../upstreamSession.js"; import type { AuthTransport } from "../transport.js"; @@ -63,7 +64,7 @@ export async function pollMagicLinkConfirmationHandler( if (!data?.token || !data?.refreshToken || !data?.sub) { return { status: up.status, - body: data, + body: opts.transport === "bearer" ? data : withoutTokens(data), }; } diff --git a/packages/core/src/handlers/switchOrganizationHandler.ts b/packages/core/src/handlers/switchOrganizationHandler.ts index 5c3d255..8c8a792 100644 --- a/packages/core/src/handlers/switchOrganizationHandler.ts +++ b/packages/core/src/handlers/switchOrganizationHandler.ts @@ -1,5 +1,6 @@ import { encodePathParam } from "../pathParam.js"; import type { AuthServerIssuerOption } from "../authServerIssuer.js"; +import { withoutTokens } from "../bodyTokens.js"; import { authFetch } from "../authFetch.js"; import { sessionResult } from "../upstreamSession.js"; import type { AuthTransport } from "../transport.js"; @@ -61,7 +62,7 @@ export async function switchOrganizationHandler( if (!data?.token || !data?.sub) { return { status: up.status, - body: data, + body: opts.transport === "bearer" ? data : withoutTokens(data), }; } diff --git a/packages/core/src/handlers/verifyLoginOtpHandler.ts b/packages/core/src/handlers/verifyLoginOtpHandler.ts index 35eef77..42f653e 100644 --- a/packages/core/src/handlers/verifyLoginOtpHandler.ts +++ b/packages/core/src/handlers/verifyLoginOtpHandler.ts @@ -1,4 +1,5 @@ import type { AuthServerIssuerOption } from "../authServerIssuer.js"; +import { withoutTokens } from "../bodyTokens.js"; import { authFetch } from "../authFetch.js"; import { sessionResult } from "../upstreamSession.js"; import type { AuthTransport } from "../transport.js"; @@ -66,7 +67,7 @@ async function verifyOtp( if (!data?.token) { return { status: up.status, - body: data, + body: opts.transport === "bearer" ? data : withoutTokens(data), }; } diff --git a/packages/core/src/handlers/verifyMagicLinkHandler.ts b/packages/core/src/handlers/verifyMagicLinkHandler.ts index 6c04551..839ce59 100644 --- a/packages/core/src/handlers/verifyMagicLinkHandler.ts +++ b/packages/core/src/handlers/verifyMagicLinkHandler.ts @@ -1,4 +1,5 @@ import { encodePathParam } from "../pathParam.js"; +import { withoutTokens } from "../bodyTokens.js"; import { authFetch } from "../authFetch.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; @@ -18,6 +19,8 @@ export interface VerifyMagicLinkInput { export interface VerifyMagicLinkOptions { authServerUrl: string; + /** Cookie transport (the default) drops any token from the body. */ + transport?: AuthTransport; serviceAuthorization?: string; forwardedClientIp?: string; forwardedUserAgent?: string; @@ -90,6 +93,6 @@ export async function verifyMagicLinkHandler( return { status: up.status, - body: data, + body: opts.transport === "bearer" ? data : withoutTokens(data), }; } diff --git a/packages/core/tests/authenticateBearer.test.js b/packages/core/tests/authenticateBearer.test.js index 833959e..86af594 100644 --- a/packages/core/tests/authenticateBearer.test.js +++ b/packages/core/tests/authenticateBearer.test.js @@ -104,7 +104,7 @@ describe("authenticateBearer", () => { expect(result.rejection).toMatchObject({ status: 401, - errorCode: "Failed to find authentication token required", + errorCode: "unauthenticated", }); expect(result.rejection.warn).toMatch(/bearer/i); }); @@ -120,7 +120,7 @@ describe("authenticateBearer", () => { expect(result.rejection.status).toBe(401); expect(result.rejection.errorCode).toBe( - "Failed to find authentication token required", + "unauthenticated", ); }); @@ -136,7 +136,7 @@ describe("authenticateBearer", () => { expect(result.rejection).toEqual({ status: 401, - errorCode: "Invalid or expired session", + errorCode: "unauthenticated", }); }); @@ -150,7 +150,7 @@ describe("authenticateBearer", () => { audience: "https://someone-else.example.com", }); - expect(result.rejection.errorCode).toBe("Invalid or expired session"); + expect(result.rejection.errorCode).toBe("unauthenticated"); }); }); @@ -183,7 +183,7 @@ describe("authenticateRequest", () => { }); expect(withoutCookie.rejection).toMatchObject({ status: 401, - errorCode: "Failed to find authentication token required", + errorCode: "unauthenticated", warn: "Missing expected auth cookie.", }); }); @@ -201,7 +201,7 @@ describe("authenticateRequest", () => { expect(result.rejection).toEqual({ status: 401, - errorCode: "Invalid or expired session", + errorCode: "unauthenticated", }); expect(global.fetch).not.toHaveBeenCalled(); }); @@ -246,7 +246,7 @@ describe("authenticateRequest", () => { }); expect(unconfigured.rejection).toEqual({ status: 401, - errorCode: "Invalid or expired session", + errorCode: "unauthenticated", }); }); @@ -263,7 +263,7 @@ describe("authenticateRequest", () => { expect(result.rejection).toEqual({ status: 401, - errorCode: "Invalid or expired session", + errorCode: "unauthenticated", }); }); diff --git a/packages/core/tests/incompleteSessionBody.test.js b/packages/core/tests/incompleteSessionBody.test.js new file mode 100644 index 0000000..c360e26 --- /dev/null +++ b/packages/core/tests/incompleteSessionBody.test.js @@ -0,0 +1,80 @@ +import { jest } from "@jest/globals"; + +function jsonResponse(status, body) { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +const AUTH = "https://auth.example.com"; +const SESSION_OPTS = { + authServerUrl: AUTH, + audience: AUTH, + accessCookieName: "seamless-access", + refreshCookieName: "seamless-refresh", +}; + +// Each handler builds cookies only from a complete session body. Anything short of +// one used to go back as-is, tokens included, in cookie transport too. +const HANDLERS = [ + [ + "verifyLoginOtpHandler", + "verifyLoginOtpHandler.js", + (fn, opts) => fn({ kind: "email", body: { code: "1" } }, opts), + { message: "partial", refreshToken: "leaked" }, + ], + [ + "pollMagicLinkConfirmationHandler", + "pollMagicLinkConfirmationHandler.js", + (fn, opts) => fn({}, opts), + { message: "partial", token: "leaked", sub: "user-1" }, + ], + [ + "switchOrganizationHandler", + "switchOrganizationHandler.js", + (fn, opts) => fn({ organizationId: "org-1" }, opts), + { message: "partial", token: "leaked", refreshToken: "leaked" }, + ], + [ + "verifyMagicLinkHandler", + "verifyMagicLinkHandler.js", + (fn, opts) => fn({ token: "link" }, opts), + { message: "verified", token: "leaked" }, + ], +]; + +describe("handlers given an incomplete session body", () => { + const originalFetch = global.fetch; + + afterEach(() => { + global.fetch = originalFetch; + }); + + it.each(HANDLERS)( + "%s drops tokens in cookie transport", + async (name, file, run, body) => { + const mod = await import(`../dist/handlers/${file}`); + global.fetch = jest.fn(async () => jsonResponse(200, body)); + + const result = await run(mod[name], SESSION_OPTS); + + expect(result.setCookies).toBeUndefined(); + expect(result.body).not.toHaveProperty("token"); + expect(result.body).not.toHaveProperty("refreshToken"); + expect(result.body.message).toBe(body.message); + }, + ); + + it.each(HANDLERS)( + "%s returns the body whole in bearer transport", + async (name, file, run, body) => { + const mod = await import(`../dist/handlers/${file}`); + global.fetch = jest.fn(async () => jsonResponse(200, body)); + + const result = await run(mod[name], { ...SESSION_OPTS, transport: "bearer" }); + + expect(result.body).toEqual(body); + }, + ); +}); diff --git a/packages/express/README.md b/packages/express/README.md index be621eb..007da9a 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -691,6 +691,38 @@ Compatibility rules: - `admin:write` grants `admin:read` - `admin:read` does not grant write access or satisfy `requireRole("admin")` +`requireAuth` and `requireRole` refuse a request with these bodies: + +| Refusal | Status | Body | +| --- | --- | --- | +| No session, an invalid or expired one, or a sign-in flow's ephemeral token | 401 | `{"error":"unauthenticated"}` | +| A recovery session, unless `allowRecoverySession: true` | 403 | `{"error":"enrollment_required"}` | +| None of the roles granted, or no roles at all | 403 | `{"error":"insufficient_role"}` | + +The body never names the user's roles or the required ones. These are the codes +the other Seamless Auth adapters answer with. + +Under the `/auth` router, a path that neither a route of its own nor the auth API's adapter +manifest lists answers `404 { "error": "not_found" }` as JSON, rather than falling through to +the application's own 404 handler. + +### Logging + +The adapter writes its warnings and errors through one logger, `console` by +default. `setSeamlessLogger` sends them elsewhere: anything with `warn(message)` +and `error(message)` works, including `pino` or a platform logger. Call it with +no argument to go back to `console`. + +```ts +import { setSeamlessLogger } from "@seamless-auth/express"; + +setSeamlessLogger({ + warn: (message) => logger.warn(message), + error: (message) => logger.error(message), +}); +``` + + --- ### `createResourceServer(options)` diff --git a/packages/express/src/createServer.ts b/packages/express/src/createServer.ts index 161d37d..0573998 100644 --- a/packages/express/src/createServer.ts +++ b/packages/express/src/createServer.ts @@ -32,21 +32,17 @@ import { } from "./handlers/oauth.js"; import * as admin from "./handlers/admin.js"; import { - AuthFetchOptions, - checkProxyIdentity, createAdapterManifestSource, - encodePathParam, handleManifestRoute, matchManifestRoute, - proxyRequest, redactSensitiveText, SERVICE_TOKEN_AUDIENCE, SERVICE_TOKEN_ISSUER, + getSeamlessLogger, } from "@seamless-auth/core"; import { buildInternalServiceAuthorization, buildProxyServiceAuthorization, - buildServiceAuthorization, } from "./internal/buildAuthorization.js"; import { assertSecrets, @@ -266,56 +262,6 @@ export function createSeamlessAuthServer( resolveClientIp: opts.resolveClientIp, }; - const proxyWithIdentity = - ( - path: string | ((req: Request) => string), - identity: "preAuth" | "access" | "register", - method: AuthFetchOptions["method"] = "POST", - { raw = false }: { raw?: boolean } = {}, - ) => - async (req: Request & { cookiePayload?: any }, res: Response) => { - const rejection = checkProxyIdentity({ - subject: req.cookiePayload?.sub, - refreshed: req.cookiePayload?.refreshed === true, - cookies: req.cookies ?? {}, - identity, - accessCookieName: resolvedOpts.accessCookieName, - preAuthCookieName: resolvedOpts.preAuthCookieName, - registrationCookieName: resolvedOpts.registrationCookieName, - transport: transportOf(req), - authorization: req.headers.authorization, - }); - - if (rejection) { - if (rejection.warn) { - console.warn( - `[SEAMLESS-AUTH-EXPRESS] - (proxyWithIdentity) - ${rejection.warn}`, - ); - } - - res.status(rejection.status).json({ error: rejection.errorCode }); - return; - } - - const result = await proxyRequest({ - authServerUrl: resolvedOpts.authServerUrl, - path: typeof path === "function" ? path(req) : path, - method, - authorization: buildServiceAuthorization(req, resolvedOpts), - serviceAuthorization: buildProxyServiceAuthorization(resolvedOpts), - forwardedClientIp: buildForwardedClientIp( - req, - resolvedOpts.resolveClientIp, - ), - forwardedUserAgent: buildForwardedUserAgent(req), - query: req.query, - body: req.body, - raw, - }); - - respond(res, await withRecoveryRefresh(req, res, result), resolvedOpts); - }; - // Runs before ensureCookies and the routes so a blocked cross-site request // never triggers a token refresh or reaches a handler. r.use( @@ -351,18 +297,10 @@ export function createSeamlessAuthServer( }), ); - r.post( - "/webAuthn/login/start", - proxyWithIdentity("webAuthn/login/start", "preAuth"), - ); r.post("/webAuthn/login/finish", (req, res) => finishLogin(req, res, resolvedOpts), ); - r.get( - "/webAuthn/register/start", - proxyWithIdentity("webAuthn/register/start", "access", "GET"), - ); r.post("/webAuthn/register/finish", (req, res) => finishRegister(req, res, resolvedOpts), ); @@ -419,98 +357,10 @@ export function createSeamlessAuthServer( deleteAccount(req, res, resolvedOpts), ); - r.get("/organizations", proxyWithIdentity("organizations", "access", "GET")); - r.post("/organizations", proxyWithIdentity("organizations", "access")); - r.get( - "/organizations/:organizationId", - proxyWithIdentity( - (req) => - `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, - "access", - "GET", - ), - ); - r.patch( - "/organizations/:organizationId", - proxyWithIdentity( - (req) => - `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, - "access", - "PATCH", - ), - ); r.post("/organizations/:organizationId/switch", (req, res) => switchOrganization(req, res, resolvedOpts), ); - r.get( - "/organizations/:organizationId/members", - proxyWithIdentity( - (req) => - `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members`, - "access", - "GET", - ), - ); - r.post( - "/organizations/:organizationId/members", - proxyWithIdentity( - (req) => - `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members`, - "access", - ), - ); - r.patch( - "/organizations/:organizationId/members/:userId", - proxyWithIdentity( - (req) => - `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members/${encodePathParam(routeParam(req, "userId"), "userId")}`, - "access", - "PATCH", - ), - ); - r.delete( - "/organizations/:organizationId/members/:userId", - proxyWithIdentity( - (req) => - `organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members/${encodePathParam(routeParam(req, "userId"), "userId")}`, - "access", - "DELETE", - ), - ); - - r.get( - "/step-up/status", - proxyWithIdentity("step-up/status", "access", "GET"), - ); - r.post( - "/step-up/webauthn/start", - proxyWithIdentity("step-up/webauthn/start", "access"), - ); - r.post( - "/step-up/webauthn/finish", - proxyWithIdentity("step-up/webauthn/finish", "access"), - ); - r.get("/totp/status", proxyWithIdentity("totp/status", "access", "GET")); - r.post( - "/totp/enroll/start", - proxyWithIdentity("totp/enroll/start", "access"), - ); - r.post( - "/totp/enroll/verify", - proxyWithIdentity("totp/enroll/verify", "access"), - ); - r.post("/totp/disable", proxyWithIdentity("totp/disable", "access")); - r.post("/totp/verify-mfa", proxyWithIdentity("totp/verify-mfa", "access")); - - r.post("/users/update", proxyWithIdentity("users/update", "access")); - r.post( - "/users/credentials", - proxyWithIdentity("users/credentials", "access"), - ); - r.delete( - "/users/credentials", - proxyWithIdentity("users/credentials", "access", "DELETE"), - ); + r.post("/magic-link", (req, res) => requestMagicLink(req, res, resolvedOpts)); r.get("/magic-link/verify/:token", (req, res) => verifyMagicLink(req, res, routeParam(req, "token"), resolvedOpts), @@ -535,33 +385,6 @@ export function createSeamlessAuthServer( updateSystemConfig(req, res, resolvedOpts), ); - r.get( - "/system-config/oauth-providers", - proxyWithIdentity("system-config/oauth-providers", "access", "GET"), - ); - r.post( - "/system-config/oauth-providers", - proxyWithIdentity("system-config/oauth-providers", "access", "POST"), - ); - r.patch( - "/system-config/oauth-providers/:id", - proxyWithIdentity( - (req) => - `system-config/oauth-providers/${encodePathParam(routeParam(req, "id"), "id")}`, - "access", - "PATCH", - ), - ); - r.delete( - "/system-config/oauth-providers/:id", - proxyWithIdentity( - (req) => - `system-config/oauth-providers/${encodePathParam(routeParam(req, "id"), "id")}`, - "access", - "DELETE", - ), - ); - r.get("/internal/auth-events/summary", (req, res) => getAuthEventSummary(req, res, resolvedOpts), ); @@ -621,129 +444,6 @@ export function createSeamlessAuthServer( admin.getCredentialCount(req, res, resolvedOpts), ); - r.get( - "/admin/organizations", - proxyWithIdentity("admin/organizations", "access", "GET"), - ); - r.post( - "/admin/organizations", - proxyWithIdentity("admin/organizations", "access"), - ); - r.get( - "/admin/organizations/:organizationId", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, - "access", - "GET", - ), - ); - r.patch( - "/admin/organizations/:organizationId", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, - "access", - "PATCH", - ), - ); - r.delete( - "/admin/organizations/:organizationId", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}`, - "access", - "DELETE", - ), - ); - r.get( - "/admin/organizations/:organizationId/members", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members`, - "access", - "GET", - ), - ); - r.post( - "/admin/organizations/:organizationId/members", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members`, - "access", - ), - ); - r.patch( - "/admin/organizations/:organizationId/members/:userId", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members/${encodePathParam(routeParam(req, "userId"), "userId")}`, - "access", - "PATCH", - ), - ); - r.delete( - "/admin/organizations/:organizationId/members/:userId", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/members/${encodePathParam(routeParam(req, "userId"), "userId")}`, - "access", - "DELETE", - ), - ); - r.put( - "/admin/organizations/:organizationId/oauth-providers/:providerId/retirement", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/oauth-providers/${encodePathParam(routeParam(req, "providerId"), "providerId")}/retirement`, - "access", - "PUT", - ), - ); - r.delete( - "/admin/organizations/:organizationId/oauth-providers/:providerId/retirement", - proxyWithIdentity( - (req) => - `admin/organizations/${encodePathParam(routeParam(req, "organizationId"), "organizationId")}/oauth-providers/${encodePathParam(routeParam(req, "providerId"), "providerId")}/retirement`, - "access", - "DELETE", - ), - ); - - r.get( - "/admin/enrollment", - proxyWithIdentity("admin/enrollment", "access", "GET"), - ); - r.post( - "/admin/enrollment/invites", - proxyWithIdentity("admin/enrollment/invites", "access"), - ); - r.get( - "/admin/auth-events/integrity", - proxyWithIdentity("admin/auth-events/integrity", "access", "GET"), - ); - r.get( - "/admin/auth-events/export", - proxyWithIdentity("admin/auth-events/export", "access", "GET", { - raw: true, - }), - ); - r.get( - "/admin/reports/authentication-coverage", - proxyWithIdentity( - "admin/reports/authentication-coverage", - "access", - "GET", - { - raw: true, - }, - ), - ); - r.get( - "/admin/review-accounts", - proxyWithIdentity("admin/review-accounts", "access", "GET"), - ); - r.get("/admin/sessions", (req, res) => admin.listAllSessions(req, res, resolvedOpts), ); @@ -768,7 +468,7 @@ export function createSeamlessAuthServer( ); // Last, so a route with a handler of its own always wins. - r.use(async (req: Request & { cookiePayload?: any }, res, next) => { + r.use(async (req: Request & { cookiePayload?: any }, res) => { const match = matchManifestRoute( await manifestSource.get(), req.method, @@ -776,7 +476,7 @@ export function createSeamlessAuthServer( ); if (!match) { - next(); + res.status(404).json({ error: "not_found" }); return; } @@ -804,7 +504,7 @@ export function createSeamlessAuthServer( }, ); - respond(res, result, resolvedOpts); + respond(res, await withRecoveryRefresh(req, res, result), resolvedOpts); }); // Express 5 forwards rejected handler promises here. Without this, the @@ -823,9 +523,8 @@ export function createSeamlessAuthServer( if (status >= 500) { const detail = err instanceof Error ? (err.stack ?? err.message) : String(err); - console.error( - "[SEAMLESS-AUTH-EXPRESS] - Unhandled route error.", - redactSensitiveText(detail), + getSeamlessLogger().error( + `[SEAMLESS-AUTH-EXPRESS] - Unhandled route error. ${redactSensitiveText(detail)}`, ); res.status(500).json({ error: "internal_error" }); return; diff --git a/packages/express/src/handlers/verifyMagicLink.ts b/packages/express/src/handlers/verifyMagicLink.ts index 1f09d50..fe62dba 100644 --- a/packages/express/src/handlers/verifyMagicLink.ts +++ b/packages/express/src/handlers/verifyMagicLink.ts @@ -28,6 +28,7 @@ export async function verifyMagicLink( }, { authServerUrl: opts.authServerUrl, + transport: transportOf(req), serviceAuthorization: buildProxyServiceAuthorization(opts), forwardedClientIp: buildForwardedClientIp(req, opts.resolveClientIp), forwardedUserAgent: buildForwardedUserAgent(req), diff --git a/packages/express/src/index.ts b/packages/express/src/index.ts index a125025..e90cb4c 100644 --- a/packages/express/src/index.ts +++ b/packages/express/src/index.ts @@ -32,6 +32,11 @@ export type { SmsMessage, SmsTransport, } from "@seamless-auth/core"; -export { hasScopedRole, roleGrantsAccess } from "@seamless-auth/core"; +export { + hasScopedRole, + roleGrantsAccess, + setSeamlessLogger, +} from "@seamless-auth/core"; +export type { SeamlessLogger } from "@seamless-auth/core"; export default createSeamlessAuthServer; diff --git a/packages/express/src/internal/buildForwardedClientIp.ts b/packages/express/src/internal/buildForwardedClientIp.ts index 6a851ac..b027fbe 100644 --- a/packages/express/src/internal/buildForwardedClientIp.ts +++ b/packages/express/src/internal/buildForwardedClientIp.ts @@ -1,3 +1,4 @@ +import { getSeamlessLogger } from "@seamless-auth/core"; import { isIP } from "node:net"; import { Request } from "express"; @@ -13,7 +14,7 @@ function derivedFromTrustedHop(req: Request): string | undefined { if (req.app?.get?.("trust proxy") === true) { if (!warnedBlanketTrustProxy) { warnedBlanketTrustProxy = true; - console.warn( + getSeamlessLogger().warn( "[seamless-auth] Express 'trust proxy' is set to true, so req.ip is client-controlled. " + "The client IP will not be forwarded to the auth API. Set 'trust proxy' to an explicit " + "hop count or subnet, or pass resolveClientIp to createSeamlessAuthServer.", diff --git a/packages/express/src/internal/validateSecrets.ts b/packages/express/src/internal/validateSecrets.ts index 938f4a6..891f0bd 100644 --- a/packages/express/src/internal/validateSecrets.ts +++ b/packages/express/src/internal/validateSecrets.ts @@ -4,14 +4,14 @@ export { assertSecrets, } from "@seamless-auth/core"; -import { DEV_JWKS_KID } from "@seamless-auth/core"; +import { DEV_JWKS_KID, getSeamlessLogger } from "@seamless-auth/core"; export function warnOnDevJwksKid(jwksKid: string | undefined): void { if (!jwksKid || jwksKid === DEV_JWKS_KID) { const state = jwksKid ? `is "${DEV_JWKS_KID}"` : `is not set and defaults to "${DEV_JWKS_KID}"`; - console.warn( + getSeamlessLogger().warn( `[SEAMLESS-AUTH-EXPRESS] - jwksKid ${state}, a placeholder. It is the kid header on the HS256 service tokens this adapter signs with serviceSecret; set it to name that key explicitly.`, ); } diff --git a/packages/express/src/middleware/requireAuth.ts b/packages/express/src/middleware/requireAuth.ts index cb7a919..effcf54 100644 --- a/packages/express/src/middleware/requireAuth.ts +++ b/packages/express/src/middleware/requireAuth.ts @@ -1,5 +1,9 @@ import { Request, Response, NextFunction } from "express"; -import { assertSecretStrength, authenticateRequest } from "@seamless-auth/core"; +import { + assertSecretStrength, + authenticateRequest, + getSeamlessLogger, +} from "@seamless-auth/core"; export interface RequireAuthOptions { cookieName?: string; @@ -114,7 +118,7 @@ export function requireAuth(opts: RequireAuthOptions) { if (rejection) { if (rejection.warn) { - console.warn( + getSeamlessLogger().warn( `[SEAMLESS-AUTH-EXPRESS] - (requireAuth) - ${rejection.warn} ${hint}`, ); } diff --git a/packages/express/tests/cookieRequirementRoutes.test.js b/packages/express/tests/cookieRequirementRoutes.test.js index c375895..a2abf21 100644 --- a/packages/express/tests/cookieRequirementRoutes.test.js +++ b/packages/express/tests/cookieRequirementRoutes.test.js @@ -63,31 +63,6 @@ describe("cookie requirement routes", () => { global.fetch = originalFetch; }); - it("forwards a Bearer token when updating a user (#57)", async () => { - global.fetch.mockResolvedValue( - createJsonResponse(200, { message: "Success" }), - ); - - const body = { email: "user@example.com" }; - - const res = await request(createApp()) - .post("/auth/users/update") - .set("Cookie", createAccessCookie()) - .send(body); - - expect(res.status).toBe(200); - expect(global.fetch).toHaveBeenCalledWith( - "https://auth.example.com/users/update", - expect.objectContaining({ - method: "POST", - body: JSON.stringify(body), - headers: expect.objectContaining({ - Authorization: `Bearer ${ACCESS_TOKEN}`, - }), - }), - ); - }); - it("forwards a Bearer token when adding a credential (#57)", async () => { global.fetch.mockResolvedValue( createJsonResponse(200, { message: "Success" }), @@ -131,8 +106,10 @@ describe("cookie requirement routes", () => { ); }); - it("rejects user update without an access session (#57)", async () => { - const res = await request(createApp()).post("/auth/users/update").send({}); + it("rejects adding a credential without an access session (#57)", async () => { + const res = await request(createApp()) + .post("/auth/users/credentials") + .send({}); expect(res.status).toBeGreaterThanOrEqual(400); expect(global.fetch).not.toHaveBeenCalled(); diff --git a/packages/express/tests/logger.test.js b/packages/express/tests/logger.test.js new file mode 100644 index 0000000..536e9de --- /dev/null +++ b/packages/express/tests/logger.test.js @@ -0,0 +1,31 @@ +import { jest } from "@jest/globals"; + +const { createSeamlessAuthServer, setSeamlessLogger } = await import( + "../dist/index.js" +); + +describe("setSeamlessLogger (express)", () => { + afterEach(() => { + setSeamlessLogger(); + jest.restoreAllMocks(); + }); + + it("receives the adapter's own warnings instead of console", () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + const logger = { warn: jest.fn(), error: jest.fn() }; + setSeamlessLogger(logger); + + createSeamlessAuthServer({ + fetchManifest: false, + authServerUrl: "https://auth.example.com", + cookieSecret: "cookie-secret-cookie-secret-cookie-secret", + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + }); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[SEAMLESS-AUTH-EXPRESS\] - jwksKid is not set/), + ); + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/express/tests/manifestRoutes.test.js b/packages/express/tests/manifestRoutes.test.js index 2c9d832..bb9ebd4 100644 --- a/packages/express/tests/manifestRoutes.test.js +++ b/packages/express/tests/manifestRoutes.test.js @@ -133,6 +133,52 @@ describe("routes proxied from the adapter manifest", () => { ); expect(res.status).toBe(404); + expect(res.headers["content-type"]).toMatch(/application\/json/); + expect(res.body).toEqual({ error: "not_found" }); + }); + + // These were hand-registered passthroughs that returned the upstream body as-is. + it("keeps tokens out of a former passthrough route's cookie-transport body", async () => { + routeFetch({ + "GET /organizations": async () => + json(200, { organizations: [], token: "leaked", refreshToken: "leaked" }), + }); + const token = await signAccessToken(); + const access = cookie("seamless-access", { sub: "user-1", token }); + + const res = await request(createApp({ fetchManifest: false })) + .get("/auth/organizations") + .set("Cookie", access); + + expect(res.status).toBe(200); + expect(res.body).toEqual({ organizations: [] }); + const [[, init]] = upstreamCalls("/organizations"); + expect(init.headers.Authorization).toBe(`Bearer ${token}`); + }); + + it("returns a former passthrough route's body whole in bearer transport", async () => { + routeFetch({ + "GET /organizations": async () => json(200, { organizations: [], token: "kept" }), + }); + + const res = await request(createApp({ fetchManifest: false })) + .get("/auth/organizations") + .set("x-seamless-auth-transport", "bearer") + .set("Authorization", "Bearer access-token"); + + expect(res.body).toEqual({ organizations: [], token: "kept" }); + }); + + it("no longer serves /users/update, which the auth API does not have", async () => { + routeFetch({}); + + const res = await request(createApp({ fetchManifest: false })) + .post("/auth/users/update") + .set("Cookie", cookie("seamless-access", { sub: "user-1", token: await signAccessToken() })) + .send({}); + + expect(res.status).toBe(404); + expect(upstreamCalls("/users/update")).toHaveLength(0); }); it("does not expose a deprecated GET the manifest leaves out", async () => { diff --git a/packages/express/tests/requireAuth.bearer.test.js b/packages/express/tests/requireAuth.bearer.test.js index dcb666b..175bd25 100644 --- a/packages/express/tests/requireAuth.bearer.test.js +++ b/packages/express/tests/requireAuth.bearer.test.js @@ -106,7 +106,7 @@ describe("requireAuth with bearer tokens (express)", () => { .set("Authorization", `Bearer ${token}`); expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Failed to find authentication token required" }); + expect(res.body).toEqual({ error: "unauthenticated" }); expect(global.fetch).not.toHaveBeenCalled(); }); @@ -122,7 +122,7 @@ describe("requireAuth with bearer tokens (express)", () => { .set("Authorization", `Bearer ${token}`); expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Invalid or expired session" }); + expect(res.body).toEqual({ error: "unauthenticated" }); }); it("still honours the cookie, and prefers it over a bearer header", async () => { @@ -164,7 +164,7 @@ describe("requireAuth with bearer tokens (express)", () => { .set("Cookie", [cookieFor({ sub: "user-1", roles: ["admin"], token: INNER })]); expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Invalid or expired session" }); + expect(res.body).toEqual({ error: "unauthenticated" }); }); it("refuses a cookie naming a different subject than its inner token", async () => { @@ -352,7 +352,7 @@ describe("authServerIssuer (express)", () => { for (const res of [unconfigured, misconfigured]) { expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Invalid or expired session" }); + expect(res.body).toEqual({ error: "unauthenticated" }); } }); diff --git a/packages/express/tests/requireRole.test.js b/packages/express/tests/requireRole.test.js index ff31d81..ec5dfff 100644 --- a/packages/express/tests/requireRole.test.js +++ b/packages/express/tests/requireRole.test.js @@ -51,9 +51,27 @@ describe("requireRole", () => { }); }); - it("rejects read roles on write routes", async () => { + it("rejects read roles on write routes without naming any role", async () => { const res = await request(appWithRoles(["admin:read"])).post("/protected"); expect(res.status).toBe(403); + expect(res.body).toEqual({ error: "insufficient_role" }); + }); + + it("rejects a user with no roles as lacking the role", async () => { + const res = await request(appWithRoles(undefined)).get("/protected"); + + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: "insufficient_role" }); + }); + + it("rejects a request no guard authenticated", async () => { + const app = express(); + app.get("/protected", requireRole("admin"), (_req, res) => res.json({})); + + const res = await request(app).get("/protected"); + + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: "unauthenticated" }); }); }); diff --git a/packages/express/tests/routeErrorHandling.test.js b/packages/express/tests/routeErrorHandling.test.js index 3b3101b..c645965 100644 --- a/packages/express/tests/routeErrorHandling.test.js +++ b/packages/express/tests/routeErrorHandling.test.js @@ -105,7 +105,7 @@ describe("route error handling", () => { it("answers 400 for a malformed JSON body", async () => { const res = await request(createApp()) - .post("/auth/users/update") + .post("/auth/users/credentials") .set("Content-Type", "application/json") .send("{bad json"); @@ -115,7 +115,7 @@ describe("route error handling", () => { it("answers 413 for a body over the parser limit", async () => { const res = await request(createApp()) - .post("/auth/users/update") + .post("/auth/users/credentials") .set("Content-Type", "application/json") .send(JSON.stringify({ name: "a".repeat(200 * 1024) })); @@ -125,7 +125,7 @@ describe("route error handling", () => { it("does not echo the rejected payload back to the client", async () => { const res = await request(createApp()) - .post("/auth/users/update") + .post("/auth/users/credentials") .set("Content-Type", "application/json") .send('{"secret":"SECRET_INTERNAL_DETAIL"'); @@ -135,7 +135,7 @@ describe("route error handling", () => { it("does not log client errors at error level", async () => { await request(createApp()) - .post("/auth/users/update") + .post("/auth/users/credentials") .set("Content-Type", "application/json") .send("{bad json"); @@ -150,8 +150,9 @@ describe("route error handling", () => { .set("Cookie", createPreAuthCookie()); expect(errorSpy).toHaveBeenCalledWith( - "[SEAMLESS-AUTH-EXPRESS] - Unhandled route error.", - expect.stringContaining("network down"), + expect.stringMatching( + /^\[SEAMLESS-AUTH-EXPRESS\] - Unhandled route error\. .*network down/s, + ), ); }); @@ -164,7 +165,7 @@ describe("route error handling", () => { .get("/auth/magic-link/check") .set("Cookie", createPreAuthCookie()); - const [, logged] = errorSpy.mock.calls[0]; + const [logged] = errorSpy.mock.calls[0]; expect(logged).not.toContain("sk-secret-token-value"); expect(logged).toContain("[REDACTED]"); }); diff --git a/packages/express/tests/stepUpProxy.test.js b/packages/express/tests/stepUpProxy.test.js index a170951..b9d9168 100644 --- a/packages/express/tests/stepUpProxy.test.js +++ b/packages/express/tests/stepUpProxy.test.js @@ -197,7 +197,7 @@ describe("step-up proxy routes", () => { expect(res.status).toBe(200); expect(global.fetch).toHaveBeenCalledWith( - "https://auth.example.com/webAuthn/register/start?requirePrf=true", + "https://auth.example.com/webauthn/register/start?requirePrf=true", expect.objectContaining({ method: "GET", headers: expect.objectContaining({ diff --git a/packages/fastify/README.md b/packages/fastify/README.md index 29c95b1..fb7b944 100644 --- a/packages/fastify/README.md +++ b/packages/fastify/README.md @@ -102,6 +102,15 @@ from a recovery session, so the new access cookie drops the claim at once. If th refresh fails, the next `POST /auth/refresh` drops it instead. A bearer client calls `/auth/refresh` itself. +| Refusal | Status | Body | +| --- | --- | --- | +| No session, an invalid or expired one, or a sign-in flow's ephemeral token | 401 | `{"error":"unauthenticated"}` | +| A recovery session, unless `allowRecoverySession: true` | 403 | `{"error":"enrollment_required"}` | +| None of the roles granted, or no roles at all | 403 | `{"error":"insufficient_role"}` | + +The body never names the user's roles or the required ones. These are the codes +the other Seamless Auth adapters answer with. + For the hydrated profile rather than the token payload, `getSeamlessUser(request, options)` fetches it from the auth API and returns `SeamlessUser | null`. It resolves a cookie session or a bearer access token, and returns `null` without calling the auth API when @@ -281,6 +290,22 @@ The browser's `User-Agent` is forwarded alongside it as record the device a person signed in from rather than this adapter. It needs no trust decision: it is self-reported by the browser either way. +## Logging + +The adapter writes its warnings and errors through one logger, `console` by +default. `setSeamlessLogger` sends them elsewhere: anything with `warn(message)` +and `error(message)` works, including `pino` or a platform logger. Call it with +no argument to go back to `console`. + +```ts +import { setSeamlessLogger } from "@seamless-auth/fastify"; + +setSeamlessLogger({ + warn: (message) => logger.warn(message), + error: (message) => logger.error(message), +}); +``` + ## Relationship to `@seamless-auth/express` Both adapters serve the same routes and issue the same cookies. A parity suite diff --git a/packages/fastify/src/index.ts b/packages/fastify/src/index.ts index d98f0d6..6352718 100644 --- a/packages/fastify/src/index.ts +++ b/packages/fastify/src/index.ts @@ -31,6 +31,11 @@ export type { SmsTransport, StepUpRequirement, } from "@seamless-auth/core"; -export { hasScopedRole, roleGrantsAccess } from "@seamless-auth/core"; +export { + hasScopedRole, + roleGrantsAccess, + setSeamlessLogger, +} from "@seamless-auth/core"; +export type { SeamlessLogger } from "@seamless-auth/core"; export default seamlessAuth; diff --git a/packages/fastify/src/internal/buildForwardedClientIp.ts b/packages/fastify/src/internal/buildForwardedClientIp.ts index f636990..deed073 100644 --- a/packages/fastify/src/internal/buildForwardedClientIp.ts +++ b/packages/fastify/src/internal/buildForwardedClientIp.ts @@ -1,3 +1,4 @@ +import { getSeamlessLogger } from "@seamless-auth/core"; import { isIP } from "node:net"; import type { FastifyRequest } from "fastify"; @@ -64,7 +65,7 @@ function derivedFromTrustedHop(req: FastifyRequest): string | undefined { if (trustsAnyAddress(req)) { if (!warnedBlanketTrustProxy) { warnedBlanketTrustProxy = true; - console.warn( + getSeamlessLogger().warn( "[seamless-auth] Fastify 'trustProxy' trusts every address (for example `true`), so request.ip is client-controlled. " + "The client IP will not be forwarded to the auth API. Set 'trustProxy' to the proxy's address or subnet, " + "or pass resolveClientIp to the plugin.", @@ -83,7 +84,7 @@ function derivedFromTrustedHop(req: FastifyRequest): string | undefined { req.ip === req.socket?.remoteAddress ) { warnedUntrustedProxy = true; - console.warn( + getSeamlessLogger().warn( "[seamless-auth] Requests carry X-Forwarded-For, but Fastify's 'trustProxy' trusts no proxy, so request.ip is the " + "connecting peer. If this app is behind a proxy, set 'trustProxy' to the proxy's address or subnet (Fastify " + "5.12.1 and later ignore a hop count), or pass resolveClientIp to the plugin.", diff --git a/packages/fastify/src/plugin.ts b/packages/fastify/src/plugin.ts index 1795d68..e6ff4f6 100644 --- a/packages/fastify/src/plugin.ts +++ b/packages/fastify/src/plugin.ts @@ -9,13 +9,12 @@ import { type AdapterManifestSource, applyExternalDelivery, assertSecrets, - checkProxyIdentity, createAdapterManifestSource, DEV_JWKS_KID, handleManifestRoute, matchManifestRoute, - proxyRequest, redactSensitiveText, + getSeamlessLogger, } from "@seamless-auth/core"; import { createEnsureCookiesHook, mountRelativePath } from "./hooks/ensureCookies.js"; @@ -23,14 +22,12 @@ import { createOriginGuardHook } from "./hooks/originGuard.js"; import { buildInternalServiceAuthorization, buildProxyServiceAuthorization, - buildServiceAuthorization, } from "./internal/buildAuthorization.js"; import { buildForwardedClientIp } from "./internal/buildForwardedClientIp.js"; import { buildForwardedUserAgent } from "./internal/buildForwardedUserAgent.js"; import { respond } from "./internal/respond.js"; import { transportOf } from "./internal/transport.js"; import type { ResolvedOptions, SeamlessAuthServerOptions } from "./options.js"; -import { PROXY_ROUTES, resolveUpstreamPath } from "./routes/proxyRoutes.js"; import { registerAuthRoutes } from "./routes/authRoutes.js"; import { registerAdminRoutes } from "./routes/adminRoutes.js"; import { withRecoveryRefresh } from "./internal/recoveryRefresh.js"; @@ -40,7 +37,7 @@ function warnOnDevJwksKid(jwksKid: string | undefined): void { const state = jwksKid ? `is "${DEV_JWKS_KID}"` : `is not set and defaults to "${DEV_JWKS_KID}"`; - console.warn( + getSeamlessLogger().warn( `[SEAMLESS-AUTH-FASTIFY] - jwksKid ${state}, a placeholder. It is the kid header on the HS256 service tokens this adapter signs with serviceSecret; set it to name that key explicitly.`, ); } @@ -104,7 +101,6 @@ export const seamlessAuth: FastifyPluginAsync< registerAuthRoutes(fastify, resolved); registerAdminRoutes(fastify, resolved); - registerProxyRoutes(fastify, resolved); registerManifestRoutes(fastify, resolved, manifestSource); fastify.setErrorHandler((error, request, reply) => { @@ -134,64 +130,6 @@ function clientErrorStatus(err: unknown): number | null { return status !== null && status >= 400 && status < 500 ? status : null; } -function registerProxyRoutes( - fastify: FastifyInstance, - opts: ResolvedOptions, -): void { - for (const route of PROXY_ROUTES) { - fastify.route({ - method: route.method, - url: route.path, - handler: async (req: FastifyRequest, reply: FastifyReply) => { - const rejection = checkProxyIdentity({ - subject: req.cookiePayload?.sub, - refreshed: req.cookiePayload?.refreshed === true, - cookies: req.cookies ?? {}, - identity: route.identity, - accessCookieName: opts.accessCookieName, - preAuthCookieName: opts.preAuthCookieName, - registrationCookieName: opts.registrationCookieName, - transport: transportOf(req), - authorization: req.headers.authorization, - }); - - if (rejection) { - if (rejection.warn) { - req.log.warn( - `[SEAMLESS-AUTH-FASTIFY] - (proxy) - ${rejection.warn}`, - ); - } - - return reply - .status(rejection.status) - .send({ error: rejection.errorCode }); - } - - const result = await proxyRequest({ - authServerUrl: opts.authServerUrl, - path: resolveUpstreamPath( - route.upstream, - req.params as Record, - ), - method: route.method, - authorization: buildServiceAuthorization(req), - serviceAuthorization: buildProxyServiceAuthorization(opts), - forwardedClientIp: buildForwardedClientIp(req, opts.resolveClientIp), - forwardedUserAgent: buildForwardedUserAgent(req), - query: req.query as Record, - body: req.body, - raw: route.raw, - }); - - respond(reply, await withRecoveryRefresh(req, result), opts); - // Returned so Fastify waits for a streamed download rather than finishing the - // reply when this handler resolves. - return reply; - }, - }); - } -} - /** * Serves every manifest route the routes above do not. Fastify prefers a static * route over a wildcard, so a route with a handler of its own always wins. Only @@ -238,7 +176,9 @@ function registerManifestRoutes( }, ); - respond(reply, result, opts); + respond(reply, await withRecoveryRefresh(req, result), opts); + // Returned so Fastify waits for a streamed download rather than finishing the + // reply when this handler resolves. return reply; }, }); diff --git a/packages/fastify/src/routes/authRoutes.ts b/packages/fastify/src/routes/authRoutes.ts index fb119d1..97beb09 100644 --- a/packages/fastify/src/routes/authRoutes.ts +++ b/packages/fastify/src/routes/authRoutes.ts @@ -298,6 +298,7 @@ export function registerAuthRoutes( }, { ...common(req), + transport: transportOf(req), serviceAuthorization: buildProxyServiceAuthorization(opts), }, ); diff --git a/packages/fastify/src/routes/proxyRoutes.ts b/packages/fastify/src/routes/proxyRoutes.ts deleted file mode 100644 index 4fd7ff9..0000000 --- a/packages/fastify/src/routes/proxyRoutes.ts +++ /dev/null @@ -1,306 +0,0 @@ -import { encodePathParam, type ProxyIdentity } from "@seamless-auth/core"; - -export interface ProxyRouteDefinition { - method: "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; - /** Route path, with the `:params` Fastify binds. */ - path: string; - /** - * Upstream path. `:params` here are filled from the route's params and always - * encoded, so a param cannot escape its own path segment. - */ - upstream: string; - identity: ProxyIdentity; - /** Forward the upstream body and its content headers unparsed, for file downloads. */ - raw?: boolean; -} - -/** - * The passthrough routes, forwarded to the auth API with no interpretation. - * - * A table rather than one path builder per route, so the encoding happens in a - * single place. Hand-writing these is how two of them ended up interpolating a - * param unencoded in the Express adapter. - */ -export const PROXY_ROUTES: ProxyRouteDefinition[] = [ - { - method: "POST", - path: "/webAuthn/login/start", - upstream: "webAuthn/login/start", - identity: "preAuth", - }, - { - method: "GET", - path: "/webAuthn/register/start", - upstream: "webAuthn/register/start", - identity: "access", - }, - - { - method: "GET", - path: "/organizations", - upstream: "organizations", - identity: "access", - }, - { - method: "POST", - path: "/organizations", - upstream: "organizations", - identity: "access", - }, - { - method: "GET", - path: "/organizations/:organizationId", - upstream: "organizations/:organizationId", - identity: "access", - }, - { - method: "PATCH", - path: "/organizations/:organizationId", - upstream: "organizations/:organizationId", - identity: "access", - }, - { - method: "GET", - path: "/organizations/:organizationId/members", - upstream: "organizations/:organizationId/members", - identity: "access", - }, - { - method: "POST", - path: "/organizations/:organizationId/members", - upstream: "organizations/:organizationId/members", - identity: "access", - }, - { - method: "PATCH", - path: "/organizations/:organizationId/members/:userId", - upstream: "organizations/:organizationId/members/:userId", - identity: "access", - }, - { - method: "DELETE", - path: "/organizations/:organizationId/members/:userId", - upstream: "organizations/:organizationId/members/:userId", - identity: "access", - }, - - { - method: "GET", - path: "/step-up/status", - upstream: "step-up/status", - identity: "access", - }, - { - method: "POST", - path: "/step-up/webauthn/start", - upstream: "step-up/webauthn/start", - identity: "access", - }, - { - method: "POST", - path: "/step-up/webauthn/finish", - upstream: "step-up/webauthn/finish", - identity: "access", - }, - - { - method: "GET", - path: "/totp/status", - upstream: "totp/status", - identity: "access", - }, - { - method: "POST", - path: "/totp/enroll/start", - upstream: "totp/enroll/start", - identity: "access", - }, - { - method: "POST", - path: "/totp/enroll/verify", - upstream: "totp/enroll/verify", - identity: "access", - }, - { - method: "POST", - path: "/totp/disable", - upstream: "totp/disable", - identity: "access", - }, - { - method: "POST", - path: "/totp/verify-mfa", - upstream: "totp/verify-mfa", - identity: "access", - }, - - { - method: "POST", - path: "/users/update", - upstream: "users/update", - identity: "access", - }, - { - method: "POST", - path: "/users/credentials", - upstream: "users/credentials", - identity: "access", - }, - { - method: "DELETE", - path: "/users/credentials", - upstream: "users/credentials", - identity: "access", - }, - - { - method: "GET", - path: "/system-config/oauth-providers", - upstream: "system-config/oauth-providers", - identity: "access", - }, - { - method: "POST", - path: "/system-config/oauth-providers", - upstream: "system-config/oauth-providers", - identity: "access", - }, - { - method: "PATCH", - path: "/system-config/oauth-providers/:id", - upstream: "system-config/oauth-providers/:id", - identity: "access", - }, - { - method: "DELETE", - path: "/system-config/oauth-providers/:id", - upstream: "system-config/oauth-providers/:id", - identity: "access", - }, - - { - method: "GET", - path: "/admin/organizations", - upstream: "admin/organizations", - identity: "access", - }, - { - method: "POST", - path: "/admin/organizations", - upstream: "admin/organizations", - identity: "access", - }, - { - method: "GET", - path: "/admin/organizations/:organizationId", - upstream: "admin/organizations/:organizationId", - identity: "access", - }, - { - method: "PATCH", - path: "/admin/organizations/:organizationId", - upstream: "admin/organizations/:organizationId", - identity: "access", - }, - { - method: "DELETE", - path: "/admin/organizations/:organizationId", - upstream: "admin/organizations/:organizationId", - identity: "access", - }, - { - method: "GET", - path: "/admin/organizations/:organizationId/members", - upstream: "admin/organizations/:organizationId/members", - identity: "access", - }, - { - method: "POST", - path: "/admin/organizations/:organizationId/members", - upstream: "admin/organizations/:organizationId/members", - identity: "access", - }, - { - method: "PATCH", - path: "/admin/organizations/:organizationId/members/:userId", - upstream: "admin/organizations/:organizationId/members/:userId", - identity: "access", - }, - { - method: "DELETE", - path: "/admin/organizations/:organizationId/members/:userId", - upstream: "admin/organizations/:organizationId/members/:userId", - identity: "access", - }, - { - method: "PUT", - path: "/admin/organizations/:organizationId/oauth-providers/:providerId/retirement", - upstream: - "admin/organizations/:organizationId/oauth-providers/:providerId/retirement", - identity: "access", - }, - { - method: "DELETE", - path: "/admin/organizations/:organizationId/oauth-providers/:providerId/retirement", - upstream: - "admin/organizations/:organizationId/oauth-providers/:providerId/retirement", - identity: "access", - }, - { - method: "GET", - path: "/admin/enrollment", - upstream: "admin/enrollment", - identity: "access", - }, - { - method: "POST", - path: "/admin/enrollment/invites", - upstream: "admin/enrollment/invites", - identity: "access", - }, - { - method: "GET", - path: "/admin/auth-events/integrity", - upstream: "admin/auth-events/integrity", - identity: "access", - }, - { - method: "GET", - path: "/admin/auth-events/export", - upstream: "admin/auth-events/export", - identity: "access", - raw: true, - }, - { - method: "GET", - path: "/admin/reports/authentication-coverage", - upstream: "admin/reports/authentication-coverage", - identity: "access", - raw: true, - }, - { - method: "GET", - path: "/admin/review-accounts", - upstream: "admin/review-accounts", - identity: "access", - }, -]; - -/** - * Fills the `:params` in an upstream template from the route's params, encoding - * each so it stays inside a single path segment. - */ -export function resolveUpstreamPath( - template: string, - params: Record, -): string { - return template.replace(/:([A-Za-z0-9_]+)/g, (_match, name: string) => { - const value = params[name]; - - if (typeof value !== "string") { - throw new Error(`Missing route parameter "${name}"`); - } - - return encodePathParam(value, name); - }); -} diff --git a/packages/fastify/tests/dotSegmentParams.test.js b/packages/fastify/tests/dotSegmentParams.test.js index 81f19d1..bf79b7d 100644 --- a/packages/fastify/tests/dotSegmentParams.test.js +++ b/packages/fastify/tests/dotSegmentParams.test.js @@ -100,12 +100,26 @@ describe("dot-segment route params", () => { return app; }; + // A manifest route has no match for a dot segment, so it is not found, as in the + // Go, Python and Rust adapters. it.each([ ["get", "/organizations/%2e%2e"], ["patch", "/organizations/%2E/members/u1"], ["delete", "/organizations/org-1/members/%2e%2e"], ["patch", "/system-config/oauth-providers/%2e%2e"], ["get", "/admin/organizations/%2e%2e"], + ])("answers %s %s as not found without calling upstream", async (method, path) => { + for (const server of [fastifyHandler, expressApp()]) { + const res = await send(server, method, path); + + expect(res.status).toBe(404); + expect(res.body).toEqual({ error: "not_found" }); + } + + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it.each([ ["delete", "/sessions/%2e%2e"], ["get", "/admin/users/%2e%2e"], ["post", "/organizations/%2e%2e/switch"], diff --git a/packages/fastify/tests/guards.test.js b/packages/fastify/tests/guards.test.js index 712bf7a..b392966 100644 --- a/packages/fastify/tests/guards.test.js +++ b/packages/fastify/tests/guards.test.js @@ -109,7 +109,7 @@ describe("requireAuth (fastify)", () => { const res = await get(await buildApp({ cookieSecret: COOKIE_SECRET })); expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Failed to find authentication token required" }); + expect(res.body).toEqual({ error: "unauthenticated" }); }); it("rejects a cookie signed by another secret", async () => { @@ -120,7 +120,7 @@ describe("requireAuth (fastify)", () => { }); expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Invalid or expired session" }); + expect(res.body).toEqual({ error: "unauthenticated" }); }); it("accepts the auth API's access token when authServerUrl and audience are configured", async () => { @@ -159,7 +159,7 @@ describe("requireAuth (fastify)", () => { ); expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Invalid or expired session" }); + expect(res.body).toEqual({ error: "unauthenticated" }); }); it("prefers the cookie over a bearer header", async () => { @@ -194,7 +194,7 @@ describe("requireAuth (fastify)", () => { ); expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Invalid or expired session" }); + expect(res.body).toEqual({ error: "unauthenticated" }); }); it("takes a cookie session's roles from its verified inner token", async () => { @@ -234,7 +234,8 @@ describe("requireAuth (fastify)", () => { { authorization: `Bearer ${await accessToken(server)}` }, ); expect(denied.status).toBe(403); - expect(denied.body.error).toBe("Insufficient role"); + // The user's roles and the required ones stay out of the body. + expect(denied.body).toEqual({ error: "insufficient_role" }); }); it("refuses a half-configured option pair at setup", () => { @@ -380,7 +381,7 @@ describe("authServerIssuer (fastify)", () => { for (const res of [unconfigured, misconfigured]) { expect(res.status).toBe(401); - expect(res.body).toEqual({ error: "Invalid or expired session" }); + expect(res.body).toEqual({ error: "unauthenticated" }); } }); diff --git a/packages/fastify/tests/logger.test.js b/packages/fastify/tests/logger.test.js new file mode 100644 index 0000000..392c805 --- /dev/null +++ b/packages/fastify/tests/logger.test.js @@ -0,0 +1,36 @@ +import { jest } from "@jest/globals"; +import Fastify from "fastify"; + +const { default: seamlessAuth, setSeamlessLogger } = await import( + "../dist/index.js" +); + +describe("setSeamlessLogger (fastify)", () => { + afterEach(() => { + setSeamlessLogger(); + jest.restoreAllMocks(); + }); + + it("receives the adapter's own warnings instead of console", async () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + const logger = { warn: jest.fn(), error: jest.fn() }; + setSeamlessLogger(logger); + + const app = Fastify(); + await app.register(seamlessAuth, { + prefix: "/auth", + fetchManifest: false, + authServerUrl: "https://auth.example.com", + cookieSecret: "cookie-secret-cookie-secret-cookie-secret", + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + }); + await app.ready(); + await app.close(); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[SEAMLESS-AUTH-FASTIFY\] - jwksKid is not set/), + ); + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index ffbe450..264d590 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -538,18 +538,42 @@ describe("fastify and express adapters agree", () => { expect(fastify).toEqual(expressResult); }); + it("answers an unknown path with a JSON 404", async () => { + const { fastify, express: expressResult } = await bothAdapters( + { method: "get", path: "/not-a-route" }, + upstream(200, {}), + ); + + expect(fastify.status).toBe(404); + expect(fastify.body).toEqual({ error: "not_found" }); + expect(expressResult).toEqual(fastify); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + // These were hand-registered passthroughs that returned the upstream body as-is. + it("keeps tokens out of a former passthrough route's cookie-transport body", async () => { + const { fastify, express: expressResult } = await bothAdapters( + { method: "get", path: "/organizations", cookie: accessCookie() }, + upstream(200, { organizations: [], token: "leaked", refreshToken: "leaked" }), + ); + + expect(fastify.status).toBe(200); + expect(fastify.body).toEqual({ organizations: [] }); + expect(expressResult).toEqual(fastify); + }); + it("answers an upstream redirect as a 502 rather than following it", async () => { const scenario = { method: "post", - path: "/users/update", + path: "/users/credentials", cookie: accessCookie(), - payload: { email: "a@b.c" }, + payload: { credentialId: "c1" }, }; const { fastify, express: expressResult } = await bothAdapters( scenario, new Response(null, { status: 302, - headers: { location: "https://elsewhere.example.com/users/update" }, + headers: { location: "https://elsewhere.example.com/users/credentials" }, }), ); @@ -829,9 +853,9 @@ describe("fastify and express adapters agree", () => { async (site, origin, status) => { const scenario = { method: "post", - path: "/users/update", + path: "/users/credentials", cookie: accessCookie(), - payload: { email: "a@b.c" }, + payload: { credentialId: "c1" }, headers: { "sec-fetch-site": site, ...(origin ? { origin } : {}) }, options: { allowedOrigins: ["https://app.example.com"] }, }; diff --git a/packages/nextjs/README.md b/packages/nextjs/README.md index 768c487..e367286 100644 --- a/packages/nextjs/README.md +++ b/packages/nextjs/README.md @@ -172,7 +172,7 @@ the module loads. | None of the roles granted | 403 | `{"error":"insufficient_role"}` | The body never names the user's roles or the required ones. These are the codes -the Go, Python and Rust adapters answer with. +every Seamless Auth adapter answers with. The guards never refresh. An expired access cookie is a `401`, and the client renews the session through the auth routes, as with the Express and Fastify @@ -391,6 +391,22 @@ IPv4-mapped IPv6 address is sent as IPv4. A `trustedProxies` entry that is not a address or range throws when the handler is created. The browser's `User-Agent` is always forwarded, capped at 512 characters: it is self-reported either way. +## Logging + +The adapter writes its warnings and errors through one logger, `console` by +default. `setSeamlessLogger` sends them elsewhere: anything with `warn(message)` +and `error(message)` works, including `pino` or a platform logger. Call it with +no argument to go back to `console`. + +```ts +import { setSeamlessLogger } from "@seamless-auth/nextjs"; + +setSeamlessLogger({ + warn: (message) => logger.warn(message), + error: (message) => logger.error(message), +}); +``` + ## Relationship to the other adapters This adapter serves the same routes and issues the same cookies as @@ -398,16 +414,13 @@ This adapter serves the same routes and issues the same cookies as Fastify parity scenarios, cookie and bearer transport both, against the Express adapter and assert the status, body, and every `Set-Cookie` header match. -Two differences follow from having no framework underneath: - -- request bodies are parsed here, the way Express's `json()` does: only - `application/json`, strict, 100kb maximum -- an unknown route answers `404 { "error": "not_found" }` as JSON +Request bodies are parsed here, since there is no framework underneath, the way +Express's `json()` does: only `application/json`, strict, 100kb maximum. An +unknown route answers `404 { "error": "not_found" }` as JSON, as in the other +adapters. `requireAuth` and `requireRole` are run against the Express guards of the same -names: status, the user handed to the route, and no refresh all match. Their -error bodies are the codes in the table above, which Express and Fastify do not -use yet. +names: status, body, the user handed to the route, and no refresh all match. The console proxy runs the Fastify parity scenarios against `createSeamlessConsoleProxy` from `@seamless-auth/express`. A write is refused diff --git a/packages/nextjs/src/guards.ts b/packages/nextjs/src/guards.ts index f374d42..1f21b41 100644 --- a/packages/nextjs/src/guards.ts +++ b/packages/nextjs/src/guards.ts @@ -5,6 +5,7 @@ import { checkOrigin, type GuardRejection, type GuardUser, + getSeamlessLogger, } from "@seamless-auth/core"; import { readCookies } from "./internal/context.js"; @@ -58,14 +59,8 @@ function refuse(status: number, error: string): Response { return Response.json({ error }, { status }); } -// The core rejections carry the Express wording; these guards answer with the -// codes the other adapters use. function toResponse(rejection: GuardRejection): Response { - if (rejection.status === 401) { - return refuse(401, "unauthenticated"); - } - - return refuse(403, rejection.errorCode); + return refuse(rejection.status, rejection.errorCode); } function prepare(name: string, opts: SeamlessGuardOptions) { @@ -116,7 +111,7 @@ function prepare(name: string, opts: SeamlessGuardOptions) { if (rejection) { if (rejection.warn) { - console.warn(`${LOG_PREFIX} - (${name}) - ${rejection.warn}`); + getSeamlessLogger().warn(`${LOG_PREFIX} - (${name}) - ${rejection.warn}`); } return { response: toResponse(rejection) }; diff --git a/packages/nextjs/src/handler.ts b/packages/nextjs/src/handler.ts index b8cf261..5400968 100644 --- a/packages/nextjs/src/handler.ts +++ b/packages/nextjs/src/handler.ts @@ -5,7 +5,6 @@ import { applyCookies, assertSecrets, checkOrigin, - checkProxyIdentity, createAdapterManifestSource, DEV_JWKS_KID, ensureCookies, @@ -13,18 +12,17 @@ import { handleManifestRoute, InvalidPathParamError, matchManifestRoute, - proxyRequest, redactSensitiveText, refreshAfterRecoveryEnrollment, SERVICE_TOKEN_AUDIENCE, SERVICE_TOKEN_ISSUER, type SessionCookie, + getSeamlessLogger, } from "@seamless-auth/core"; import { buildInternalServiceAuthorization, buildProxyServiceAuthorization, - buildServiceAuthorization, } from "./internal/buildAuthorization.js"; import { createClientIpResolver, @@ -45,7 +43,6 @@ import { matchRoute, type Route } from "./internal/router.js"; import type { ResolvedOptions, SeamlessAuthHandlerOptions } from "./options.js"; import { ADMIN_ROUTES } from "./routes/adminRoutes.js"; import { AUTH_ROUTES } from "./routes/authRoutes.js"; -import { PROXY_ROUTES, resolveUpstreamPath } from "./routes/proxyRoutes.js"; export type RouteHandler = (request: Request) => Promise; @@ -75,59 +72,14 @@ export function resolveOptions( }; } -const PASSTHROUGH_ROUTES: Route[] = PROXY_ROUTES.map((definition) => ({ - method: definition.method, - path: definition.path, - run: async (ctx, opts) => { - const rejection = checkProxyIdentity({ - subject: ctx.cookiePayload?.sub, - refreshed: ctx.cookiePayload?.refreshed === true, - cookies: ctx.cookies, - identity: definition.identity, - accessCookieName: opts.accessCookieName, - preAuthCookieName: opts.preAuthCookieName, - registrationCookieName: opts.registrationCookieName, - transport: ctx.transport, - authorization: ctx.request.headers.get("authorization") ?? undefined, - }); - - if (rejection) { - if (rejection.warn) { - console.warn(`[SEAMLESS-AUTH-NEXTJS] - (proxy) - ${rejection.warn}`); - } - - return { status: rejection.status, errorCode: rejection.errorCode }; - } - - return proxyRequest({ - authServerUrl: opts.authServerUrl, - path: resolveUpstreamPath(definition.upstream, ctx.params), - method: definition.method, - authorization: buildServiceAuthorization(ctx), - serviceAuthorization: buildProxyServiceAuthorization(opts), - forwardedClientIp: opts.clientIpOf(ctx.request), - forwardedUserAgent: forwardedUserAgent(ctx.request), - query: ctx.query, - body: ctx.body, - raw: definition.raw, - }); - }, -})); - -// Mount order of the other adapters: handler-backed routes first, so a -// passthrough can never shadow one. -const ROUTES: Route[] = [ - ...AUTH_ROUTES, - ...ADMIN_ROUTES, - ...PASSTHROUGH_ROUTES, -]; +const ROUTES: Route[] = [...AUTH_ROUTES, ...ADMIN_ROUTES]; function warnOnDevJwksKid(jwksKid: string | undefined): void { if (!jwksKid || jwksKid === DEV_JWKS_KID) { const state = jwksKid ? `is "${DEV_JWKS_KID}"` : `is not set and defaults to "${DEV_JWKS_KID}"`; - console.warn( + getSeamlessLogger().warn( `[SEAMLESS-AUTH-NEXTJS] - jwksKid ${state}, a placeholder. It is the kid header on the HS256 service tokens this adapter signs with serviceSecret; set it to name that key explicitly.`, ); } @@ -178,13 +130,12 @@ export function createSeamlessAuthHandler( return collector.json(400, { error: "bad_request" }); } - console.error( - "[SEAMLESS-AUTH-NEXTJS] - Unhandled route error.", - redactSensitiveText( + getSeamlessLogger().error( + `[SEAMLESS-AUTH-NEXTJS] - Unhandled route error. ${redactSensitiveText( error instanceof Error ? (error.stack ?? error.message) : String(error), - ), + )}`, ); // The same collector, so a refresh that ensureCookies already rotated // still reaches the browser. Dropping it would leave the browser holding a diff --git a/packages/nextjs/src/index.ts b/packages/nextjs/src/index.ts index beca27f..ab9974c 100644 --- a/packages/nextjs/src/index.ts +++ b/packages/nextjs/src/index.ts @@ -51,4 +51,9 @@ export type { SmsTransport, StepUpRequirement, } from "@seamless-auth/core"; -export { hasScopedRole, roleGrantsAccess } from "@seamless-auth/core"; +export { + hasScopedRole, + roleGrantsAccess, + setSeamlessLogger, +} from "@seamless-auth/core"; +export type { SeamlessLogger } from "@seamless-auth/core"; diff --git a/packages/nextjs/src/internal/clientIp.ts b/packages/nextjs/src/internal/clientIp.ts index ba9efb7..b40d89c 100644 --- a/packages/nextjs/src/internal/clientIp.ts +++ b/packages/nextjs/src/internal/clientIp.ts @@ -1,3 +1,4 @@ +import { getSeamlessLogger } from "@seamless-auth/core"; import { BlockList, isIP } from "node:net"; import type { ClientIpResolver } from "../options.js"; @@ -117,7 +118,7 @@ export function warnOnNoClientIp(opts: ClientIpOptions): void { } warnedNoClientIp = true; - console.warn( + getSeamlessLogger().warn( "[SEAMLESS-AUTH-NEXTJS] - Neither resolveClientIp nor trustedProxies is set, so no client IP is forwarded to the auth API and its rate limits and audit log see none. Set trustedProxies to your proxy addresses, or resolveClientIp to read the header your platform sets.", ); } diff --git a/packages/nextjs/src/routes/authRoutes.ts b/packages/nextjs/src/routes/authRoutes.ts index 0b86c4b..1caf43d 100644 --- a/packages/nextjs/src/routes/authRoutes.ts +++ b/packages/nextjs/src/routes/authRoutes.ts @@ -313,6 +313,7 @@ export const AUTH_ROUTES: Route[] = [ }, { ...common(ctx, opts), + transport: ctx.transport, serviceAuthorization: buildProxyServiceAuthorization(opts), }, ), diff --git a/packages/nextjs/src/routes/proxyRoutes.ts b/packages/nextjs/src/routes/proxyRoutes.ts deleted file mode 100644 index bd8eccc..0000000 --- a/packages/nextjs/src/routes/proxyRoutes.ts +++ /dev/null @@ -1,292 +0,0 @@ -import { encodePathParam, type ProxyIdentity } from "@seamless-auth/core"; - -export interface ProxyRouteDefinition { - method: "GET" | "POST" | "PATCH" | "DELETE"; - /** Mount-relative route path, with `:params`. */ - path: string; - /** - * Upstream path. `:params` here are filled from the route's params and always - * encoded, so a param cannot escape its own path segment. - */ - upstream: string; - identity: ProxyIdentity; - /** Forward the upstream body and its content headers unparsed, for file downloads. */ - raw?: boolean; -} - -/** - * The passthrough routes, forwarded to the auth API with no interpretation. - * - * A table rather than one path builder per route, so the encoding happens in a - * single place. Hand-writing these is how two of them ended up interpolating a - * param unencoded in the Express adapter. - */ -export const PROXY_ROUTES: ProxyRouteDefinition[] = [ - { - method: "POST", - path: "/webAuthn/login/start", - upstream: "webAuthn/login/start", - identity: "preAuth", - }, - { - method: "GET", - path: "/webAuthn/register/start", - upstream: "webAuthn/register/start", - identity: "access", - }, - - { - method: "GET", - path: "/organizations", - upstream: "organizations", - identity: "access", - }, - { - method: "POST", - path: "/organizations", - upstream: "organizations", - identity: "access", - }, - { - method: "GET", - path: "/organizations/:organizationId", - upstream: "organizations/:organizationId", - identity: "access", - }, - { - method: "PATCH", - path: "/organizations/:organizationId", - upstream: "organizations/:organizationId", - identity: "access", - }, - { - method: "GET", - path: "/organizations/:organizationId/members", - upstream: "organizations/:organizationId/members", - identity: "access", - }, - { - method: "POST", - path: "/organizations/:organizationId/members", - upstream: "organizations/:organizationId/members", - identity: "access", - }, - { - method: "PATCH", - path: "/organizations/:organizationId/members/:userId", - upstream: "organizations/:organizationId/members/:userId", - identity: "access", - }, - { - method: "DELETE", - path: "/organizations/:organizationId/members/:userId", - upstream: "organizations/:organizationId/members/:userId", - identity: "access", - }, - - { - method: "GET", - path: "/step-up/status", - upstream: "step-up/status", - identity: "access", - }, - { - method: "POST", - path: "/step-up/webauthn/start", - upstream: "step-up/webauthn/start", - identity: "access", - }, - { - method: "POST", - path: "/step-up/webauthn/finish", - upstream: "step-up/webauthn/finish", - identity: "access", - }, - - { - method: "GET", - path: "/totp/status", - upstream: "totp/status", - identity: "access", - }, - { - method: "POST", - path: "/totp/enroll/start", - upstream: "totp/enroll/start", - identity: "access", - }, - { - method: "POST", - path: "/totp/enroll/verify", - upstream: "totp/enroll/verify", - identity: "access", - }, - { - method: "POST", - path: "/totp/disable", - upstream: "totp/disable", - identity: "access", - }, - { - method: "POST", - path: "/totp/verify-mfa", - upstream: "totp/verify-mfa", - identity: "access", - }, - - { - method: "POST", - path: "/users/update", - upstream: "users/update", - identity: "access", - }, - { - method: "POST", - path: "/users/credentials", - upstream: "users/credentials", - identity: "access", - }, - { - method: "DELETE", - path: "/users/credentials", - upstream: "users/credentials", - identity: "access", - }, - - { - method: "GET", - path: "/system-config/oauth-providers", - upstream: "system-config/oauth-providers", - identity: "access", - }, - { - method: "POST", - path: "/system-config/oauth-providers", - upstream: "system-config/oauth-providers", - identity: "access", - }, - { - method: "PATCH", - path: "/system-config/oauth-providers/:id", - upstream: "system-config/oauth-providers/:id", - identity: "access", - }, - { - method: "DELETE", - path: "/system-config/oauth-providers/:id", - upstream: "system-config/oauth-providers/:id", - identity: "access", - }, - - { - method: "GET", - path: "/admin/organizations", - upstream: "admin/organizations", - identity: "access", - }, - { - method: "POST", - path: "/admin/organizations", - upstream: "admin/organizations", - identity: "access", - }, - { - method: "GET", - path: "/admin/organizations/:organizationId", - upstream: "admin/organizations/:organizationId", - identity: "access", - }, - { - method: "PATCH", - path: "/admin/organizations/:organizationId", - upstream: "admin/organizations/:organizationId", - identity: "access", - }, - { - method: "DELETE", - path: "/admin/organizations/:organizationId", - upstream: "admin/organizations/:organizationId", - identity: "access", - }, - { - method: "GET", - path: "/admin/organizations/:organizationId/members", - upstream: "admin/organizations/:organizationId/members", - identity: "access", - }, - { - method: "POST", - path: "/admin/organizations/:organizationId/members", - upstream: "admin/organizations/:organizationId/members", - identity: "access", - }, - { - method: "PATCH", - path: "/admin/organizations/:organizationId/members/:userId", - upstream: "admin/organizations/:organizationId/members/:userId", - identity: "access", - }, - { - method: "DELETE", - path: "/admin/organizations/:organizationId/members/:userId", - upstream: "admin/organizations/:organizationId/members/:userId", - identity: "access", - }, - { - method: "GET", - path: "/admin/enrollment", - upstream: "admin/enrollment", - identity: "access", - }, - { - method: "POST", - path: "/admin/enrollment/invites", - upstream: "admin/enrollment/invites", - identity: "access", - }, - { - method: "GET", - path: "/admin/auth-events/integrity", - upstream: "admin/auth-events/integrity", - identity: "access", - }, - { - method: "GET", - path: "/admin/auth-events/export", - upstream: "admin/auth-events/export", - identity: "access", - raw: true, - }, - { - method: "GET", - path: "/admin/reports/authentication-coverage", - upstream: "admin/reports/authentication-coverage", - identity: "access", - raw: true, - }, - { - method: "GET", - path: "/admin/review-accounts", - upstream: "admin/review-accounts", - identity: "access", - }, -]; - -/** - * Fills the `:params` in an upstream template from the route's params, encoding - * each so it stays inside a single path segment. - */ -export function resolveUpstreamPath( - template: string, - params: Record, -): string { - return template.replace(/:([A-Za-z0-9_]+)/g, (_match, name: string) => { - const value = params[name]; - - if (typeof value !== "string") { - throw new Error(`Missing route parameter "${name}"`); - } - - return encodePathParam(value, name); - }); -} diff --git a/packages/nextjs/tests/handler.test.js b/packages/nextjs/tests/handler.test.js index 4d3f351..2044c6a 100644 --- a/packages/nextjs/tests/handler.test.js +++ b/packages/nextjs/tests/handler.test.js @@ -110,13 +110,13 @@ describe("createSeamlessAuthHandler", () => { }); it("matches the WebAuthn routes case-insensitively, as Express does", async () => { - const res = await call("/auth/webauthn/register/start", { + const res = await call("/auth/webAuthn/register/start", { headers: { cookie: accessCookie() }, }); expect(res.status).toBe(200); expect(global.fetch.mock.calls[0][0]).toBe( - "https://auth.example.com/webAuthn/register/start", + "https://auth.example.com/webauthn/register/start", ); }); diff --git a/packages/nextjs/tests/logger.test.js b/packages/nextjs/tests/logger.test.js new file mode 100644 index 0000000..07cd6c2 --- /dev/null +++ b/packages/nextjs/tests/logger.test.js @@ -0,0 +1,32 @@ +import { jest } from "@jest/globals"; + +const { createSeamlessAuthHandler, setSeamlessLogger } = await import( + "../dist/index.js" +); + +describe("setSeamlessLogger (nextjs)", () => { + afterEach(() => { + setSeamlessLogger(); + jest.restoreAllMocks(); + }); + + it("receives the adapter's own warnings instead of console", () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + const logger = { warn: jest.fn(), error: jest.fn() }; + setSeamlessLogger(logger); + + createSeamlessAuthHandler({ + fetchManifest: false, + authServerUrl: "https://auth.example.com", + cookieSecret: "cookie-secret-cookie-secret-cookie-secret", + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + resolveClientIp: () => undefined, + }); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringMatching(/^\[SEAMLESS-AUTH-NEXTJS\] - jwksKid is not set/), + ); + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/nextjs/tests/parity.test.js b/packages/nextjs/tests/parity.test.js index 058122a..ae6b6fb 100644 --- a/packages/nextjs/tests/parity.test.js +++ b/packages/nextjs/tests/parity.test.js @@ -511,18 +511,42 @@ describe("next.js and express adapters agree", () => { expect(next).toEqual(expressResult); }); + it("answers an unknown path with a JSON 404", async () => { + const { next, express: expressResult } = await bothAdapters( + { method: "get", path: "/not-a-route" }, + upstream(200, {}), + ); + + expect(next.status).toBe(404); + expect(next.body).toEqual({ error: "not_found" }); + expect(expressResult).toEqual(next); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + // These were hand-registered passthroughs that returned the upstream body as-is. + it("keeps tokens out of a former passthrough route's cookie-transport body", async () => { + const { next, express: expressResult } = await bothAdapters( + { method: "get", path: "/organizations", cookie: accessCookie() }, + upstream(200, { organizations: [], token: "leaked", refreshToken: "leaked" }), + ); + + expect(next.status).toBe(200); + expect(next.body).toEqual({ organizations: [] }); + expect(expressResult).toEqual(next); + }); + it("answers an upstream redirect as a 502 rather than following it", async () => { const scenario = { method: "post", - path: "/users/update", + path: "/users/credentials", cookie: accessCookie(), - payload: { email: "a@b.c" }, + payload: { credentialId: "c1" }, }; const { next, express: expressResult } = await bothAdapters( scenario, new Response(null, { status: 302, - headers: { location: "https://elsewhere.example.com/users/update" }, + headers: { location: "https://elsewhere.example.com/users/credentials" }, }), ); @@ -805,9 +829,9 @@ describe("next.js and express adapters agree", () => { async (site, origin, status) => { const scenario = { method: "post", - path: "/users/update", + path: "/users/credentials", cookie: accessCookie(), - payload: { email: "a@b.c" }, + payload: { credentialId: "c1" }, headers: { "sec-fetch-site": site, ...(origin ? { origin } : {}) }, options: { allowedOrigins: ["https://app.example.com"] }, }; @@ -2140,6 +2164,7 @@ describe("next.js and express route guards agree", () => { expect(next.status).toBe(status); expect(expressResult.status).toBe(status); expect(next.body).toEqual({ error }); + expect(expressResult.body).toEqual(next.body); }); it("blocks a cross-site state change on a cookie session but not on a bearer one", async () => { diff --git a/scripts/check-adapter-manifest.mjs b/scripts/check-adapter-manifest.mjs new file mode 100644 index 0000000..314d5b4 --- /dev/null +++ b/scripts/check-adapter-manifest.mjs @@ -0,0 +1,67 @@ +// Fails when the bundled adapter manifest lists different routes from the one the +// auth API publishes. `apiVersion` is ignored: a patch release of the API bumps it +// without changing a route. +// +// node scripts/check-adapter-manifest.mjs [path-or-url] +// +// Defaults to the manifest on the auth API's main branch. Fix a failure with +// scripts/sync-adapter-manifest.mjs. +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; + +const DEFAULT_SOURCE = + "https://raw.githubusercontent.com/fells-code/seamless-auth-api/main/adapter-manifest.json"; +const BUNDLED = join( + import.meta.dirname, + "../packages/core/src/manifest/bundledManifest.ts", +); + +async function read(source) { + if (!/^https?:\/\//.test(source)) { + return readFile(source, "utf8"); + } + + const response = await fetch(source); + if (!response.ok) { + throw new Error(`Could not read ${source}: HTTP ${response.status}`); + } + return response.text(); +} + +function routeKeys(manifest) { + return manifest.routes + .map((route) => + JSON.stringify(Object.fromEntries(Object.entries(route).sort())), + ) + .sort(); +} + +const source = process.argv[2] ?? DEFAULT_SOURCE; +const published = JSON.parse(await read(source)); + +const text = await readFile(BUNDLED, "utf8"); +const bundled = JSON.parse( + text.slice(text.indexOf("= {") + 2, text.lastIndexOf("}") + 1), +); + +const want = new Set(routeKeys(published)); +const have = new Set(routeKeys(bundled)); +const missing = [...want].filter((route) => !have.has(route)); +const extra = [...have].filter((route) => !want.has(route)); + +if (bundled.schemaVersion !== published.schemaVersion) { + missing.unshift(`schemaVersion ${published.schemaVersion}`); + extra.unshift(`schemaVersion ${bundled.schemaVersion}`); +} + +if (missing.length === 0 && extra.length === 0) { + console.log( + `Bundled manifest matches ${source}: ${have.size} routes (bundled API ${bundled.apiVersion}, published ${published.apiVersion}).`, + ); +} else { + console.error(`Bundled manifest differs from ${source}.`); + for (const route of missing) console.error(` missing: ${route}`); + for (const route of extra) console.error(` extra: ${route}`); + console.error("Run node scripts/sync-adapter-manifest.mjs and commit the result."); + process.exitCode = 1; +}