From 1f640727b03263a99cdcd2e9449f5129539c51e7 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:28:56 -0500 Subject: [PATCH 01/17] test: reproduce body-less form POST CSRF under SameSite=Lax A same-site or fetch-metadata-free form POST carrying a Lax access cookie reaches cookie-authenticated routes on Express, Fastify (text/plain or no content type) and Next.js, and checkOrigin ignores cross-site and allowedOrigins unless cookies are SameSite=None. These tests fail on main. Refs #256 --- packages/core/tests/checkOrigin.test.js | 69 +++++++ packages/express/tests/formPostCsrf.test.js | 191 ++++++++++++++++++++ packages/fastify/tests/formPostCsrf.test.js | 191 ++++++++++++++++++++ packages/nextjs/tests/formPostCsrf.test.js | 186 +++++++++++++++++++ 4 files changed, 637 insertions(+) create mode 100644 packages/core/tests/checkOrigin.test.js create mode 100644 packages/express/tests/formPostCsrf.test.js create mode 100644 packages/fastify/tests/formPostCsrf.test.js create mode 100644 packages/nextjs/tests/formPostCsrf.test.js diff --git a/packages/core/tests/checkOrigin.test.js b/packages/core/tests/checkOrigin.test.js new file mode 100644 index 0000000..9b85127 --- /dev/null +++ b/packages/core/tests/checkOrigin.test.js @@ -0,0 +1,69 @@ +import { checkOrigin } from "../dist/index.js"; + +const BLOCKED = { status: 403, errorCode: "cross_site_request_blocked" }; +const LISTED = "https://app.example.com"; +const SIBLING = "https://sibling.example.com"; + +describe("checkOrigin", () => { + // A Lax or Strict cookie still rides a same-site request, and a browser that + // sends no fetch metadata gives no Sec-Fetch-Site to refuse on (#256). + describe.each(["lax", "strict", "none"])("with SameSite=%s", (cookieSameSite) => { + const check = (input) => + checkOrigin({ method: "POST", cookieSameSite, ...input }); + + it("refuses a cross-site request", () => { + expect(check({ secFetchSite: "cross-site" })).toEqual(BLOCKED); + }); + + it("refuses a literal null Origin without fetch metadata", () => { + expect(check({ origin: "null" })).toEqual(BLOCKED); + }); + + it("refuses a same-site request from an Origin off allowedOrigins", () => { + expect( + check({ secFetchSite: "same-site", origin: SIBLING, allowedOrigins: [LISTED] }), + ).toEqual(BLOCKED); + }); + + it("refuses a same-site request with no Origin when allowedOrigins is set", () => { + expect(check({ secFetchSite: "same-site", allowedOrigins: [LISTED] })).toEqual( + BLOCKED, + ); + }); + + it("refuses an Origin off allowedOrigins without fetch metadata", () => { + expect(check({ origin: SIBLING, allowedOrigins: [LISTED] })).toEqual(BLOCKED); + }); + + it("treats an empty allowedOrigins as configured", () => { + expect( + check({ secFetchSite: "same-site", origin: LISTED, allowedOrigins: [] }), + ).toEqual(BLOCKED); + expect(check({ origin: LISTED, allowedOrigins: [] })).toEqual(BLOCKED); + }); + + it("accepts a same-site request from a listed Origin", () => { + expect( + check({ secFetchSite: "same-site", origin: LISTED, allowedOrigins: [LISTED] }), + ).toBeUndefined(); + }); + + it("accepts a same-origin request whatever the list says", () => { + expect( + check({ secFetchSite: "same-origin", origin: SIBLING, allowedOrigins: [] }), + ).toBeUndefined(); + }); + + it("accepts a caller with neither Origin nor Sec-Fetch-Site", () => { + expect(check({ allowedOrigins: [LISTED] })).toBeUndefined(); + }); + + it("leaves safe methods alone", () => { + for (const method of ["GET", "HEAD", "OPTIONS"]) { + expect( + checkOrigin({ method, cookieSameSite, secFetchSite: "cross-site" }), + ).toBeUndefined(); + } + }); + }); +}); diff --git a/packages/express/tests/formPostCsrf.test.js b/packages/express/tests/formPostCsrf.test.js new file mode 100644 index 0000000..4e84823 --- /dev/null +++ b/packages/express/tests/formPostCsrf.test.js @@ -0,0 +1,191 @@ +// A body-less form POST from a sibling subdomain, or from a browser that sends +// no fetch metadata, carries a SameSite=Lax cookie and must not reach a +// cookie-authenticated route (#256). +import { jest } from "@jest/globals"; +import express from "express"; +import jwt from "jsonwebtoken"; +import request from "supertest"; + +const { default: createSeamlessAuthServer } = await import("../dist/index.js"); + +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; +const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; + +const accessCookie = () => + `seamless-access=${jwt.sign( + { sub: "user-123", roles: ["user"], token: ACCESS_TOKEN }, + COOKIE_SECRET, + { algorithm: "HS256", expiresIn: "300s" }, + )}`; + +function upstream(status, body) { + return { ok: status >= 200 && status < 300, status, json: async () => body }; +} + +function createApp(overrides = {}) { + const app = express(); + app.use( + "/auth", + createSeamlessAuthServer({ + fetchManifest: false, + authServerUrl: "https://auth.example.com", + cookieSecret: COOKIE_SECRET, + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + jwksKid: "test-main", + cookieSameSite: "lax", + ...overrides, + }), + ); + return app; +} + +async function send({ method = "POST", path, headers, payload, options }) { + let req = request(createApp(options))[method.toLowerCase()](`/auth${path}`); + for (const [name, value] of Object.entries(headers)) { + if (value !== undefined) req = req.set(name, value); + } + if (payload !== undefined) req = req.send(payload); + const res = await req; + return { status: res.status, body: res.body }; +} + +function upstreamCalls(path) { + return global.fetch.mock.calls.filter( + ([url]) => String(url) === `https://auth.example.com${path}`, + ); +} + +const SIBLING = "https://sibling.example.com"; + +describe("form POST CSRF with SameSite=Lax cookies (#256)", () => { + const originalFetch = global.fetch; + + beforeEach(() => { + global.fetch = jest.fn(async () => upstream(200, { codes: ["a"] })); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + it.each([ + ["same-site", "application/x-www-form-urlencoded"], + ["same-site", "multipart/form-data; boundary=x"], + ["same-site", "text/plain"], + ["same-site", undefined], + [undefined, "application/x-www-form-urlencoded"], + [undefined, "text/plain"], + [undefined, undefined], + ])( + "refuses a body-less POST with Sec-Fetch-Site %s and Content-Type %s", + async (site, contentType) => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: SIBLING, + "sec-fetch-site": site, + "content-type": contentType, + }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(415); + expect(res.body).toEqual({ error: "unsupported_media_type" }); + }, + ); + + it("refuses a cross-site POST whatever the SameSite setting", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "cross-site", + "content-type": "application/json", + }, + payload: "{}", + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: "cross_site_request_blocked" }); + }); + + it.each([["same-site"], [undefined]])( + "enforces allowedOrigins under Lax for Sec-Fetch-Site %s", + async (site) => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: SIBLING, + "sec-fetch-site": site, + "content-type": "application/json", + }, + payload: "{}", + options: { allowedOrigins: ["https://app.example.com"] }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + }, + ); + + it("treats an empty allowedOrigins as configured", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: "https://app.example.com", + "sec-fetch-site": "same-site", + "content-type": "application/json", + }, + payload: "{}", + options: { allowedOrigins: [] }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + }); + + it("accepts a same-origin body-less POST that declares JSON", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "same-origin", + "content-type": "application/json", + }, + }); + + expect(res.status).toBe(200); + expect(upstreamCalls("/recovery-codes")).toHaveLength(1); + }); + + it("accepts a body-less POST that carries the transport header", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "same-site", + "x-seamless-auth-transport": "cookie", + }, + }); + + expect(res.status).toBe(200); + expect(upstreamCalls("/recovery-codes")).toHaveLength(1); + }); + + it("does not require a content type on DELETE, which always preflights", async () => { + const res = await send({ + method: "DELETE", + path: "/sessions/s-1", + headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, + }); + + expect(res.status).not.toBe(415); + expect(upstreamCalls("/sessions/s-1")).toHaveLength(1); + }); +}); diff --git a/packages/fastify/tests/formPostCsrf.test.js b/packages/fastify/tests/formPostCsrf.test.js new file mode 100644 index 0000000..ca523ea --- /dev/null +++ b/packages/fastify/tests/formPostCsrf.test.js @@ -0,0 +1,191 @@ +// A body-less form POST from a sibling subdomain, or from a browser that sends +// no fetch metadata, carries a SameSite=Lax cookie and must not reach a +// cookie-authenticated route (#256). +import { jest } from "@jest/globals"; +import Fastify from "fastify"; +import jwt from "jsonwebtoken"; + +const { default: seamlessAuth } = await import("../dist/index.js"); + +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; +const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; + +const accessCookie = () => + `seamless-access=${jwt.sign( + { sub: "user-123", roles: ["user"], token: ACCESS_TOKEN }, + COOKIE_SECRET, + { algorithm: "HS256", expiresIn: "300s" }, + )}`; + +function upstream(status, body) { + return { ok: status >= 200 && status < 300, status, json: async () => body }; +} + +const OPTIONS = { + fetchManifest: false, + authServerUrl: "https://auth.example.com", + cookieSecret: COOKIE_SECRET, + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + jwksKid: "test-main", + cookieSameSite: "lax", +}; + +async function send({ method = "POST", path, headers, payload, options }) { + const app = Fastify(); + await app.register(seamlessAuth, { prefix: "/auth", ...OPTIONS, ...options }); + await app.ready(); + try { + const res = await app.inject({ + method, + url: `/auth${path}`, + headers: Object.fromEntries( + Object.entries(headers).filter(([, value]) => value !== undefined), + ), + ...(payload === undefined ? {} : { payload }), + }); + return { status: res.statusCode, body: res.body ? JSON.parse(res.body) : null }; + } finally { + await app.close(); + } +} + +function upstreamCalls(path) { + return global.fetch.mock.calls.filter( + ([url]) => String(url) === `https://auth.example.com${path}`, + ); +} + +const SIBLING = "https://sibling.example.com"; + +describe("form POST CSRF with SameSite=Lax cookies (#256)", () => { + const originalFetch = global.fetch; + + beforeEach(() => { + global.fetch = jest.fn(async () => upstream(200, { codes: ["a"] })); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + it.each([ + ["same-site", "application/x-www-form-urlencoded"], + ["same-site", "multipart/form-data; boundary=x"], + ["same-site", "text/plain"], + ["same-site", undefined], + [undefined, "application/x-www-form-urlencoded"], + [undefined, "text/plain"], + [undefined, undefined], + ])( + "refuses a body-less POST with Sec-Fetch-Site %s and Content-Type %s", + async (site, contentType) => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: SIBLING, + "sec-fetch-site": site, + "content-type": contentType, + }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(415); + expect(res.body).toEqual({ error: "unsupported_media_type" }); + }, + ); + + it("refuses a cross-site POST whatever the SameSite setting", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "cross-site", + "content-type": "application/json", + }, + payload: "{}", + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: "cross_site_request_blocked" }); + }); + + it.each([["same-site"], [undefined]])( + "enforces allowedOrigins under Lax for Sec-Fetch-Site %s", + async (site) => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: SIBLING, + "sec-fetch-site": site, + "content-type": "application/json", + }, + payload: "{}", + options: { allowedOrigins: ["https://app.example.com"] }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + }, + ); + + it("treats an empty allowedOrigins as configured", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: "https://app.example.com", + "sec-fetch-site": "same-site", + "content-type": "application/json", + }, + payload: "{}", + options: { allowedOrigins: [] }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + }); + + it("accepts a same-origin body-less POST that declares JSON", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "same-origin", + "content-type": "application/json", + }, + }); + + expect(res.status).toBe(200); + expect(upstreamCalls("/recovery-codes")).toHaveLength(1); + }); + + it("accepts a body-less POST that carries the transport header", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "same-site", + "x-seamless-auth-transport": "cookie", + }, + }); + + expect(res.status).toBe(200); + expect(upstreamCalls("/recovery-codes")).toHaveLength(1); + }); + + it("does not require a content type on DELETE, which always preflights", async () => { + const res = await send({ + method: "DELETE", + path: "/sessions/s-1", + headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, + }); + + expect(res.status).not.toBe(415); + expect(upstreamCalls("/sessions/s-1")).toHaveLength(1); + }); +}); diff --git a/packages/nextjs/tests/formPostCsrf.test.js b/packages/nextjs/tests/formPostCsrf.test.js new file mode 100644 index 0000000..909db14 --- /dev/null +++ b/packages/nextjs/tests/formPostCsrf.test.js @@ -0,0 +1,186 @@ +// A body-less form POST from a sibling subdomain, or from a browser that sends +// no fetch metadata, carries a SameSite=Lax cookie and must not reach a +// cookie-authenticated route (#256). +import { jest } from "@jest/globals"; +import jwt from "jsonwebtoken"; + +const { createSeamlessAuthHandler } = await import("../dist/index.js"); + +// Unsigned: the adapter only reads the inner token's type, the auth API verifies it. +const ACCESS_TOKEN = `e30.${Buffer.from(JSON.stringify({ typ: "access" })).toString("base64url")}.sig`; +const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; + +const accessCookie = () => + `seamless-access=${jwt.sign( + { sub: "user-123", roles: ["user"], token: ACCESS_TOKEN }, + COOKIE_SECRET, + { algorithm: "HS256", expiresIn: "300s" }, + )}`; + +function upstream(status, body) { + return { ok: status >= 200 && status < 300, status, json: async () => body }; +} + +const OPTIONS = { + fetchManifest: false, + authServerUrl: "https://auth.example.com", + cookieSecret: COOKIE_SECRET, + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + jwksKid: "test-main", + cookieSameSite: "lax", +}; + +async function send({ method = "POST", path, headers, payload, options }) { + const handlers = createSeamlessAuthHandler({ ...OPTIONS, ...options }); + const res = await handlers[method]( + new Request(`http://localhost/auth${path}`, { + method, + headers: Object.fromEntries( + Object.entries(headers).filter(([, value]) => value !== undefined), + ), + body: payload, + }), + ); + const text = await res.text(); + return { status: res.status, body: text ? JSON.parse(text) : null }; +} + +function upstreamCalls(path) { + return global.fetch.mock.calls.filter( + ([url]) => String(url) === `https://auth.example.com${path}`, + ); +} + +const SIBLING = "https://sibling.example.com"; + +describe("form POST CSRF with SameSite=Lax cookies (#256)", () => { + const originalFetch = global.fetch; + + beforeEach(() => { + global.fetch = jest.fn(async () => upstream(200, { codes: ["a"] })); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + it.each([ + ["same-site", "application/x-www-form-urlencoded"], + ["same-site", "multipart/form-data; boundary=x"], + ["same-site", "text/plain"], + ["same-site", undefined], + [undefined, "application/x-www-form-urlencoded"], + [undefined, "text/plain"], + [undefined, undefined], + ])( + "refuses a body-less POST with Sec-Fetch-Site %s and Content-Type %s", + async (site, contentType) => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: SIBLING, + "sec-fetch-site": site, + "content-type": contentType, + }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(415); + expect(res.body).toEqual({ error: "unsupported_media_type" }); + }, + ); + + it("refuses a cross-site POST whatever the SameSite setting", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "cross-site", + "content-type": "application/json", + }, + payload: "{}", + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + expect(res.body).toEqual({ error: "cross_site_request_blocked" }); + }); + + it.each([["same-site"], [undefined]])( + "enforces allowedOrigins under Lax for Sec-Fetch-Site %s", + async (site) => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: SIBLING, + "sec-fetch-site": site, + "content-type": "application/json", + }, + payload: "{}", + options: { allowedOrigins: ["https://app.example.com"] }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + }, + ); + + it("treats an empty allowedOrigins as configured", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + origin: "https://app.example.com", + "sec-fetch-site": "same-site", + "content-type": "application/json", + }, + payload: "{}", + options: { allowedOrigins: [] }, + }); + + expect(upstreamCalls("/recovery-codes")).toHaveLength(0); + expect(res.status).toBe(403); + }); + + it("accepts a same-origin body-less POST that declares JSON", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "same-origin", + "content-type": "application/json", + }, + }); + + expect(res.status).toBe(200); + expect(upstreamCalls("/recovery-codes")).toHaveLength(1); + }); + + it("accepts a body-less POST that carries the transport header", async () => { + const res = await send({ + path: "/recovery-codes", + headers: { + cookie: accessCookie(), + "sec-fetch-site": "same-site", + "x-seamless-auth-transport": "cookie", + }, + }); + + expect(res.status).toBe(200); + expect(upstreamCalls("/recovery-codes")).toHaveLength(1); + }); + + it("does not require a content type on DELETE, which always preflights", async () => { + const res = await send({ + method: "DELETE", + path: "/sessions/s-1", + headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, + }); + + expect(res.status).not.toBe(415); + expect(upstreamCalls("/sessions/s-1")).toHaveLength(1); + }); +}); From 9eb1b632de064a53d2409ef7f6f2421a116e878e Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:36:20 -0500 Subject: [PATCH 02/17] fix: guard unsafe auth requests whatever the SameSite setting The origin check only ran for SameSite=None cookies, and a body-less form POST reached cookie-authenticated routes because Express and Next.js forward urlencoded and multipart as an empty body and Fastify accepts text/plain and a missing content type. checkOrigin now runs whatever the SameSite setting and enforces allowedOrigins (an empty list included) for same-site and metadata-free requests. A new checkContentType refuses a state-changing request to the auth routes with 415 unsupported_media_type unless it declares application/json or carries an x-seamless-auth-transport header, even with no body. POST /refresh, DELETE /logout and DELETE /logout/all are exempt for now because the current browser client sends them bare. Fastify accepts an empty JSON body on the plugin's routes and Express no longer reads one as {}, so a body-less JSON request forwards no body. Refs #256 --- .changeset/unsafe-request-content-type.md | 17 ++++ packages/core/README.md | 3 +- packages/core/src/guards.ts | 88 ++++++++++++++++--- packages/core/tests/checkOrigin.test.js | 64 +++++++++++++- packages/express/README.md | 35 +++++--- packages/express/src/createServer.ts | 14 ++- .../express/src/middleware/originGuard.ts | 35 +++++--- packages/express/tests/formPostCsrf.test.js | 41 ++++++++- packages/express/tests/loginOtpRoutes.test.js | 1 + packages/express/tests/logoutRoutes.test.js | 1 + .../express/tests/magicLinkRedirect.test.js | 1 + .../express/tests/messagingDelivery.test.js | 3 + .../express/tests/oauthProvidersProxy.test.js | 1 + .../express/tests/organizationRoutes.test.js | 18 ++-- packages/express/tests/originGuard.test.js | 26 +++++- packages/express/tests/totpProxy.test.js | 1 + packages/express/tests/usersRoutes.test.js | 6 +- packages/fastify/src/hooks/originGuard.ts | 43 +++++---- packages/fastify/src/plugin.ts | 26 +++++- .../fastify/tests/dotSegmentParams.test.js | 2 +- packages/fastify/tests/formPostCsrf.test.js | 41 ++++++++- packages/fastify/tests/parity.test.js | 15 +++- packages/nextjs/src/guards.ts | 2 - packages/nextjs/src/handler.ts | 28 +++--- packages/nextjs/tests/formPostCsrf.test.js | 41 ++++++++- packages/nextjs/tests/handler.test.js | 12 ++- packages/nextjs/tests/parity.test.js | 16 +++- 27 files changed, 481 insertions(+), 100 deletions(-) create mode 100644 .changeset/unsafe-request-content-type.md diff --git a/.changeset/unsafe-request-content-type.md b/.changeset/unsafe-request-content-type.md new file mode 100644 index 0000000..dc8d554 --- /dev/null +++ b/.changeset/unsafe-request-content-type.md @@ -0,0 +1,17 @@ +--- +"@seamless-auth/core": patch +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Close a form-post CSRF gap on the auth routes (#256). A body-less form `POST` from a sibling subdomain, or from a browser that sends no fetch metadata, could reach cookie-authenticated routes such as `/recovery-codes` when cookies were `SameSite=Lax` or `Strict`. + +Requests now refused: + +- The origin check runs whatever the `SameSite` setting. A state-changing request with `Sec-Fetch-Site: cross-site` answers `403 {"error":"cross_site_request_blocked"}` under `Lax` and `Strict` too, and `allowedOrigins` is enforced for `same-site` and metadata-free requests under every setting. An empty `allowedOrigins: []` counts as configured. This also applies to the Next.js `requireAuth` and `requireRole` guards. +- A state-changing request to the auth routes must declare `Content-Type: application/json` or carry an `x-seamless-auth-transport` header (any value), even with no body, or it answers `415 {"error":"unsupported_media_type"}`. `POST /refresh`, `DELETE /logout` and `DELETE /logout/all` are exempt for now, because the current browser client sends them bare; they still get the origin check. A client of your own that sends a body-less `POST`, `PUT`, `PATCH` or `DELETE` with no content type must add one of the two. + +Fastify accepts an empty body under `Content-Type: application/json` on the plugin's routes, and Express no longer reads one as `{}`, so a body-less JSON request forwards no body on all three adapters. + +`checkContentType` is exported from `@seamless-auth/core`. `checkOrigin` ignores its `cookieSecure` and `cookieSameSite` inputs. diff --git a/packages/core/README.md b/packages/core/README.md index 6cbbfe1..0854fea 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -119,7 +119,8 @@ remain for direct imports. - `authenticateBearer(...)` – verifies a bearer access token into a session - `authenticateRequest(...)` – the cookie when present, otherwise the bearer token when enabled - `authorizeRoles(...)` – role check against an authenticated session -- `checkOrigin(...)` – cross-site request check for `SameSite=None` deployments +- `checkOrigin(...)` – cross-site request check for state-changing requests, whatever the `SameSite` setting +- `checkContentType(...)` – refuses a state-changing request a form or `no-cors` fetch could send without a CORS preflight **Building an adapter** diff --git a/packages/core/src/guards.ts b/packages/core/src/guards.ts index 91fd5d4..abae116 100644 --- a/packages/core/src/guards.ts +++ b/packages/core/src/guards.ts @@ -1,7 +1,7 @@ import type { SeamlessAuthUser } from "@seamless-auth/types"; import { decodeJwt } from "jose"; -import { resolveCookieSameSite, type CookieSameSite } from "./applyResult.js"; +import type { CookieSameSite } from "./applyResult.js"; import { hasScopedRole } from "@seamless-auth/types/role/matching"; import type { AuthServerIssuerOption } from "./authServerIssuer.js"; import { assertSecretStrength } from "./validateSecrets.js"; @@ -47,8 +47,15 @@ export interface OriginCheckInput { secFetchSite?: string; /** `Origin`, already reduced to a single value. */ origin?: string; + /** + * No longer consulted. The check runs whatever the cookie policy, because a + * `Lax` or `Strict` cookie still rides a same-site request. Kept so existing + * callers compile. + */ cookieSecure?: boolean; + /** As `cookieSecure`. */ cookieSameSite?: CookieSameSite; + /** Set, even to `[]`, it is enforced for same-site and metadata-free requests. */ allowedOrigins?: string[]; } @@ -59,24 +66,22 @@ function normalizeOrigin(value: string): string { /** * Decides whether a cross-site state-changing request should be refused. * - * Only matters when the adapter issues `SameSite=None` cookies, which the - * browser would otherwise attach to a forged cross-site request. A `Lax` or - * `Strict` cookie is not sent on one, so the check is inert there. + * Runs whatever the adapter's `SameSite` setting: `Lax` and `Strict` keep a + * cookie off a cross-site request but not off a same-site one, and a sibling + * subdomain is same-site. * * `Sec-Fetch-Site` is the primary signal: current browsers send it and page * JavaScript cannot forge it. `cross-site` is refused and `same-origin` passes. * `same-site` also covers a sibling subdomain, so when the adopter configured - * `allowedOrigins` the `Origin` must be on it as well; without the allowlist a - * same-site request passes, as it always has. When `Sec-Fetch-Site` is absent - * the `Origin` is matched against `allowedOrigins`, again only when the adopter - * opted in, so nothing regresses for callers that predate the guard. + * `allowedOrigins` the `Origin` must be on it as well. When `Sec-Fetch-Site` is + * absent the `Origin` is matched against `allowedOrigins`, again only when the + * adopter configured it. Without the list both pass here, and + * `checkContentType` is what keeps a forged form post out. */ export function checkOrigin( input: OriginCheckInput, ): GuardRejection | undefined { - const active = resolveCookieSameSite(input) === "none"; - - if (!active || SAFE_METHODS.has(input.method)) { + if (SAFE_METHODS.has(input.method)) { return undefined; } @@ -114,8 +119,6 @@ export function checkOrigin( return rejection; } - // Older browser, but the adopter has not opted into an allowlist. Preserve the - // pre-guard behavior rather than start rejecting these. if (!input.allowedOrigins) { return undefined; } @@ -125,6 +128,65 @@ export function checkOrigin( : rejection; } +export interface ContentTypeCheckInput { + method: string; + /** Relative to the adapter's mount point, as the route tables name it. */ + path: string; + /** `Content-Type`, already reduced to a single value. */ + contentType?: string; + /** `x-seamless-auth-transport`, already reduced to a single value. */ + transport?: string; +} + +// The browser client up to now sends these with no body and no header, so +// they stay open until a client that declares JSON on them is the floor. They +// still get the origin check. +const CONTENT_TYPE_EXEMPT = new Set([ + "POST /refresh", + "DELETE /logout", + "DELETE /logout/all", +]); + +function routeKey(method: string, path: string): string { + const normalized = path.toLowerCase().replace(/\/+$/, "") || "/"; + return `${method.toUpperCase()} ${normalized}`; +} + +/** + * Refuses a state-changing request to the adapter's auth routes that declares + * neither `Content-Type: application/json` nor an `x-seamless-auth-transport` + * header (any value). An HTML form can only send urlencoded, multipart or plain + * text, and a `no-cors` fetch can set neither, so requiring one forces a CORS + * preflight the adopter's CORS policy then decides. + * + * Applies even to a request with no body, because the routes act on the cookie + * alone. Not for an adopter's own routes behind a guard: those get the origin + * check only. + */ +export function checkContentType( + input: ContentTypeCheckInput, +): GuardRejection | undefined { + const method = input.method.toUpperCase(); + + if (SAFE_METHODS.has(method)) { + return undefined; + } + + if (CONTENT_TYPE_EXEMPT.has(routeKey(method, input.path))) { + return undefined; + } + + if (input.transport !== undefined) { + return undefined; + } + + const mediaType = input.contentType?.split(";")[0]?.trim().toLowerCase(); + + return mediaType === "application/json" + ? undefined + : { status: 415, errorCode: "unsupported_media_type" }; +} + function isAllowedOrigin(origin: string, allowedOrigins: string[]): boolean { const allowed = new Set(allowedOrigins.map(normalizeOrigin).filter(Boolean)); diff --git a/packages/core/tests/checkOrigin.test.js b/packages/core/tests/checkOrigin.test.js index 9b85127..b019557 100644 --- a/packages/core/tests/checkOrigin.test.js +++ b/packages/core/tests/checkOrigin.test.js @@ -1,4 +1,4 @@ -import { checkOrigin } from "../dist/index.js"; +import { checkContentType, checkOrigin } from "../dist/index.js"; const BLOCKED = { status: 403, errorCode: "cross_site_request_blocked" }; const LISTED = "https://app.example.com"; @@ -67,3 +67,65 @@ describe("checkOrigin", () => { }); }); }); + +describe("checkContentType", () => { + const REFUSED = { status: 415, errorCode: "unsupported_media_type" }; + + it.each([ + ["POST", "application/x-www-form-urlencoded"], + ["POST", "multipart/form-data; boundary=x"], + ["POST", "text/plain"], + ["POST", undefined], + ["POST", "application/json-seq"], + ["DELETE", undefined], + ["PUT", "text/plain"], + ["PATCH", undefined], + ])("refuses %s with Content-Type %s", (method, contentType) => { + expect(checkContentType({ method, path: "/recovery-codes", contentType })).toEqual( + REFUSED, + ); + }); + + it.each([ + "application/json", + "Application/JSON; charset=utf-8", + " application/json ", + ])("accepts Content-Type %s with or without a body", (contentType) => { + expect( + checkContentType({ method: "POST", path: "/recovery-codes", contentType }), + ).toBeUndefined(); + }); + + it.each(["bearer", "cookie", ""])( + "accepts any x-seamless-auth-transport value (%j)", + (transport) => { + expect( + checkContentType({ method: "POST", path: "/recovery-codes", transport }), + ).toBeUndefined(); + }, + ); + + it("leaves safe methods alone", () => { + for (const method of ["GET", "HEAD", "OPTIONS"]) { + expect(checkContentType({ method, path: "/recovery-codes" })).toBeUndefined(); + } + }); + + it.each([ + ["POST", "/refresh"], + ["POST", "/Refresh/"], + ["DELETE", "/logout"], + ["DELETE", "/logout/all"], + ])("exempts %s %s, which the browser client sends bare", (method, path) => { + expect(checkContentType({ method, path })).toBeUndefined(); + }); + + it.each([ + ["POST", "/logout"], + ["DELETE", "/refresh"], + ["POST", "/refresh/extra"], + ["DELETE", "/logout/everything"], + ])("does not stretch the exemption to %s %s", (method, path) => { + expect(checkContentType({ method, path })).toEqual(REFUSED); + }); +}); diff --git a/packages/express/README.md b/packages/express/README.md index 007da9a..d5feca0 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -339,33 +339,42 @@ app.use( #### Cross-site request protection -A `SameSite=None` cookie is attached to cross-site requests, so a page on any -origin could drive a state-changing call to the adapter with the signed-in -user's session. When the effective `sameSite` is `none`, the router rejects -cross-site state-changing requests with `403 { "error": -"cross_site_request_blocked" }`. This runs by default and needs no configuration. +A session cookie rides any request the browser considers allowed by its +`SameSite` setting: every cross-site request for `None`, and every same-site +request (a sibling subdomain included) for `Lax` and `Strict`. So the router +guards state-changing requests whatever the cookie policy. This runs by default +and needs no configuration. - The guard reads `Sec-Fetch-Site`, which current Chrome, Firefox, and Safari send and page JavaScript cannot forge. A non-safe request (anything other than - `GET`, `HEAD`, or `OPTIONS`) is rejected when that header is `cross-site`. - Same-origin single-page-app calls send `same-origin` or `same-site` and pass - with no allowlist; with `allowedOrigins` set, a `same-site` call must also - come from a listed `Origin`. + `GET`, `HEAD`, or `OPTIONS`) is rejected with `403 { "error": + "cross_site_request_blocked" }` when that header is `cross-site`. Same-origin + single-page-app calls send `same-origin` or `same-site` and pass with no + allowlist; with `allowedOrigins` set, a `same-site` call must also come from a + listed `Origin`. +- A non-safe request must also declare `Content-Type: application/json` or carry + an `x-seamless-auth-transport` header (any value), even with no body, or it is + rejected with `415 { "error": "unsupported_media_type" }`. An HTML form or a + `no-cors` fetch can send neither, so this forces a CORS preflight. `POST + /refresh`, `DELETE /logout` and `DELETE /logout/all` are exempt for now, + because the browser client sends them bare; they still get the origin check. - Safe methods are never gated, so `GET /magic-link/verify/:token` and the CORS preflight (`OPTIONS`) are unaffected. `POST /magic-link/verify/:token/confirm` is a state change and is guarded like any other, so serve the page that calls it from your own site. - A caller that sends neither `Origin` nor `Sec-Fetch-Site`, such as a - server-to-server request, passes. + server-to-server request, passes the origin check, but still needs the JSON + content type or the transport header. `allowedOrigins` is an opt-in `Origin` allowlist. When it is set, the request `Origin` must appear in the list for a state-changing request that is `same-site`, and for one from an older browser that sends no `Sec-Fetch-Site`; otherwise the request is rejected. `same-site` covers every subdomain of your registrable domain, so without the list a page on a sibling subdomain can send -a request that carries a `SameSite=None` cookie. A `same-origin` request always -passes. When `allowedOrigins` is unset the guard behaves as before (a -`same-site` request passes, as does one with no `Sec-Fetch-Site`), so setting it +a request that carries your cookie. A `same-origin` request always passes. An +empty list is a configured list: it admits no `same-site` request and no +metadata-free request that names an `Origin`. When `allowedOrigins` is unset a +`same-site` request passes, as does one with no `Sec-Fetch-Site`, so setting it is never required. Set it when you do not trust every subdomain of your site, or support browsers old enough to lack `Sec-Fetch-Site`: diff --git a/packages/express/src/createServer.ts b/packages/express/src/createServer.ts index 0573998..be81334 100644 --- a/packages/express/src/createServer.ts +++ b/packages/express/src/createServer.ts @@ -239,6 +239,18 @@ export function createSeamlessAuthServer( }); r.use(express.json()); + // A body-less request declares JSON to force a CORS preflight, which + // express.json() reads as `{}`. Keep it as no body, as the other adapters do, + // so nothing is forwarded upstream that the client never sent. + r.use((req, _res, next) => { + if ( + req.headers["transfer-encoding"] === undefined && + !Number(req.headers["content-length"] ?? 0) + ) { + req.body = undefined; + } + next(); + }); r.use(cookieParser()); const resolvedOpts: ResolvedSeamlessAuthServerOptions = { @@ -266,8 +278,6 @@ export function createSeamlessAuthServer( // never triggers a token refresh or reaches a handler. r.use( createOriginGuardMiddleware({ - cookieSecure: resolvedOpts.cookieSecure, - cookieSameSite: resolvedOpts.cookieSameSite, allowedOrigins: resolvedOpts.allowedOrigins, }), ); diff --git a/packages/express/src/middleware/originGuard.ts b/packages/express/src/middleware/originGuard.ts index dded4bc..d116965 100644 --- a/packages/express/src/middleware/originGuard.ts +++ b/packages/express/src/middleware/originGuard.ts @@ -1,28 +1,35 @@ import { NextFunction, Request, Response } from "express"; -import { checkOrigin, type CookieSameSite } from "@seamless-auth/core"; +import { + AUTH_TRANSPORT_HEADER, + checkContentType, + checkOrigin, +} from "@seamless-auth/core"; export interface OriginGuardOptions { - cookieSecure?: boolean; - cookieSameSite?: CookieSameSite; allowedOrigins?: string[]; } /** - * Rejects cross-site state-changing requests when the adapter issues - * `SameSite=None` cookies. The decision is core's; this reads the headers and - * writes the response. + * Rejects cross-site state-changing requests, and a `POST` a page on another + * origin could send without a preflight. The decisions are core's; this reads + * the headers and writes the response. */ export function createOriginGuardMiddleware(opts: OriginGuardOptions) { return function originGuard(req: Request, res: Response, next: NextFunction) { - const rejection = checkOrigin({ - method: req.method, - secFetchSite: firstHeader(req.headers["sec-fetch-site"]), - origin: firstHeader(req.headers.origin), - cookieSecure: opts.cookieSecure, - cookieSameSite: opts.cookieSameSite, - allowedOrigins: opts.allowedOrigins, - }); + const rejection = + checkOrigin({ + method: req.method, + secFetchSite: firstHeader(req.headers["sec-fetch-site"]), + origin: firstHeader(req.headers.origin), + allowedOrigins: opts.allowedOrigins, + }) ?? + checkContentType({ + method: req.method, + path: req.path, + contentType: req.headers["content-type"], + transport: firstHeader(req.headers[AUTH_TRANSPORT_HEADER]), + }); if (rejection) { res.status(rejection.status).json({ error: rejection.errorCode }); diff --git a/packages/express/tests/formPostCsrf.test.js b/packages/express/tests/formPostCsrf.test.js index 4e84823..acd1277 100644 --- a/packages/express/tests/formPostCsrf.test.js +++ b/packages/express/tests/formPostCsrf.test.js @@ -48,7 +48,11 @@ async function send({ method = "POST", path, headers, payload, options }) { } if (payload !== undefined) req = req.send(payload); const res = await req; - return { status: res.status, body: res.body }; + return { + status: res.status, + body: res.body, + setCookie: res.headers["set-cookie"], + }; } function upstreamCalls(path) { @@ -178,14 +182,45 @@ describe("form POST CSRF with SameSite=Lax cookies (#256)", () => { expect(upstreamCalls("/recovery-codes")).toHaveLength(1); }); - it("does not require a content type on DELETE, which always preflights", async () => { + it("refuses a body-less DELETE that declares nothing", async () => { const res = await send({ method: "DELETE", path: "/sessions/s-1", headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, }); + expect(res.status).toBe(415); + expect(upstreamCalls("/sessions/s-1")).toHaveLength(0); + }); + + // The browser client up to now sends these bare, so they keep only the + // origin check. + it.each([ + ["POST", "/refresh"], + ["DELETE", "/logout"], + ["DELETE", "/logout/all"], + ])("leaves %s %s out of the content-type gate", async (method, path) => { + const res = await send({ + method, + path, + headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, + }); + expect(res.status).not.toBe(415); - expect(upstreamCalls("/sessions/s-1")).toHaveLength(1); }); + + it.each([["/logout"], ["/logout/all"]])( + "still applies the origin check to DELETE %s, clearing no cookie", + async (path) => { + const res = await send({ + method: "DELETE", + path, + headers: { cookie: accessCookie(), "sec-fetch-site": "cross-site" }, + }); + + expect(res.status).toBe(403); + expect(res.setCookie ?? []).toEqual([]); + expect(global.fetch).not.toHaveBeenCalled(); + }, + ); }); diff --git a/packages/express/tests/loginOtpRoutes.test.js b/packages/express/tests/loginOtpRoutes.test.js index 6811cd9..0c4e2f6 100644 --- a/packages/express/tests/loginOtpRoutes.test.js +++ b/packages/express/tests/loginOtpRoutes.test.js @@ -64,6 +64,7 @@ describe("login OTP routes", () => { const res = await request(createApp()) .post("/auth/otp/generate-login-email-otp") + .set("Content-Type", "application/json") .set("Cookie", createPreAuthCookie()); expect(res.status).toBe(200); diff --git a/packages/express/tests/logoutRoutes.test.js b/packages/express/tests/logoutRoutes.test.js index 1552707..f96e13e 100644 --- a/packages/express/tests/logoutRoutes.test.js +++ b/packages/express/tests/logoutRoutes.test.js @@ -119,6 +119,7 @@ describe("logout routes", () => { const res = await request(createApp()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Cookie", createPreAuthCookie()); expect(res.status).toBe(200); diff --git a/packages/express/tests/magicLinkRedirect.test.js b/packages/express/tests/magicLinkRedirect.test.js index 2f9c9a1..df8ac20 100644 --- a/packages/express/tests/magicLinkRedirect.test.js +++ b/packages/express/tests/magicLinkRedirect.test.js @@ -61,6 +61,7 @@ describe("magic link redirect target", () => { async function post(body) { const req = request(createApp()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Cookie", createPreAuthCookie()); await (body === undefined ? req : req.send(body)); diff --git a/packages/express/tests/messagingDelivery.test.js b/packages/express/tests/messagingDelivery.test.js index d29fec7..dd1969a 100644 --- a/packages/express/tests/messagingDelivery.test.js +++ b/packages/express/tests/messagingDelivery.test.js @@ -86,6 +86,7 @@ describe("messaging delivery routes", () => { const res = await request(createApp(emailTransport)) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Cookie", createPreAuthCookie()); expect(res.status).toBe(200); @@ -170,6 +171,7 @@ describe("messaging delivery routes", () => { const res = await request(createApp(emailTransport)) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Cookie", createPreAuthCookie()); expect(res.status).toBe(200); @@ -209,6 +211,7 @@ describe("messaging delivery routes", () => { const res = await request(app) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Cookie", createPreAuthCookie()); expect(res.status).toBe(200); diff --git a/packages/express/tests/oauthProvidersProxy.test.js b/packages/express/tests/oauthProvidersProxy.test.js index c97b6a7..69c0050 100644 --- a/packages/express/tests/oauthProvidersProxy.test.js +++ b/packages/express/tests/oauthProvidersProxy.test.js @@ -140,6 +140,7 @@ describe("oauth provider admin proxy routes", () => { const res = await request(createApp()) .delete("/auth/system-config/oauth-providers/google") + .set("Content-Type", "application/json") .set("Cookie", createAccessCookie()); expect(res.status).toBe(200); diff --git a/packages/express/tests/organizationRoutes.test.js b/packages/express/tests/organizationRoutes.test.js index 497f75b..163fcef 100644 --- a/packages/express/tests/organizationRoutes.test.js +++ b/packages/express/tests/organizationRoutes.test.js @@ -120,6 +120,7 @@ describe("organization proxy routes", () => { const res = await request(createApp()) .delete("/auth/admin/organizations/org-1") + .set("Content-Type", "application/json") .set("Cookie", createAccessCookie()); expect(res.status).toBe(200); @@ -138,9 +139,11 @@ describe("organization proxy routes", () => { // Without a session the adapter must answer on its own rather than forwarding a // destructive call upstream with no identity attached. it("refuses organization deletion without an access session", async () => { - const res = await request(createApp()).delete( - "/auth/admin/organizations/org-1", - ); + const res = await request(createApp()) + .delete( + "/auth/admin/organizations/org-1", + ) + .set("Content-Type", "application/json"); expect(res.status).toBe(401); expect(global.fetch).not.toHaveBeenCalled(); @@ -190,6 +193,7 @@ describe("organization proxy routes", () => { const res = await request(createApp()) [verb]("/auth/admin/organizations/org-1/oauth-providers/legacy-idp/retirement") + .set("Content-Type", "application/json") .set("Cookie", createAccessCookie()); expect(res.status).toBe(200); @@ -207,9 +211,11 @@ describe("organization proxy routes", () => { ); it("refuses OAuth provider retirement without an access session", async () => { - const res = await request(createApp()).put( - "/auth/admin/organizations/org-1/oauth-providers/legacy-idp/retirement", - ); + const res = await request(createApp()) + .put( + "/auth/admin/organizations/org-1/oauth-providers/legacy-idp/retirement", + ) + .set("Content-Type", "application/json"); expect(res.status).toBe(401); expect(global.fetch).not.toHaveBeenCalled(); diff --git a/packages/express/tests/originGuard.test.js b/packages/express/tests/originGuard.test.js index 4b727d7..6aa0899 100644 --- a/packages/express/tests/originGuard.test.js +++ b/packages/express/tests/originGuard.test.js @@ -47,6 +47,7 @@ describe("origin guard (#104)", () => { it("rejects a cross-site state-changing request", async () => { const res = await request(createApp()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "cross-site"); expect(res.status).toBe(403); @@ -56,6 +57,7 @@ describe("origin guard (#104)", () => { it("accepts a same-origin request", async () => { const res = await request(createApp()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "same-origin"); expect(res.status).not.toBe(403); @@ -65,6 +67,7 @@ describe("origin guard (#104)", () => { it("accepts a same-site request", async () => { const res = await request(createApp()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "same-site"); expect(res.status).not.toBe(403); @@ -72,7 +75,8 @@ describe("origin guard (#104)", () => { }); it("accepts a server-to-server request with no Origin or Sec-Fetch-Site", async () => { - const res = await request(createApp()).post("/auth/magic-link"); + const res = await request(createApp()).post("/auth/magic-link") + .set("Content-Type", "application/json"); expect(res.status).not.toBe(403); expect(res.body).not.toEqual(BLOCKED); @@ -81,6 +85,7 @@ describe("origin guard (#104)", () => { it("rejects a literal null Origin", async () => { const res = await request(createApp()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Origin", "null"); expect(res.status).toBe(403); @@ -108,6 +113,7 @@ describe("origin guard (#104)", () => { it("rejects a cross-site POST to the magic link confirm step", async () => { const res = await request(createApp()) .post("/auth/magic-link/verify/some-token/confirm") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "cross-site"); expect(res.status).toBe(403); @@ -118,6 +124,7 @@ describe("origin guard (#104)", () => { it("accepts a same-origin POST to the magic link confirm step", async () => { const res = await request(createApp()) .post("/auth/magic-link/verify/some-token/confirm") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "same-origin"); expect(res.status).toBe(200); @@ -136,13 +143,16 @@ describe("origin guard (#104)", () => { expect(res.body).not.toEqual(BLOCKED); }); - it("does nothing when cookieSameSite is lax, even cross-site", async () => { + // A Lax cookie still rides a same-site request, so the cookie policy no + // longer switches the guard off (#256). + it("rejects cross-site when cookieSameSite is lax too", async () => { const res = await request(createApp({ cookieSameSite: "lax" })) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "cross-site"); - expect(res.status).not.toBe(403); - expect(res.body).not.toEqual(BLOCKED); + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); }); describe("Sec-Fetch-Site absent fallback", () => { @@ -151,6 +161,7 @@ describe("origin guard (#104)", () => { createApp({ allowedOrigins: ["https://app.example.com"] }), ) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Origin", "https://app.example.com"); expect(res.status).not.toBe(403); @@ -162,6 +173,7 @@ describe("origin guard (#104)", () => { createApp({ allowedOrigins: ["https://app.example.com"] }), ) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Origin", "https://evil.example.com"); expect(res.status).toBe(403); @@ -171,6 +183,7 @@ describe("origin guard (#104)", () => { it("accepts any Origin when allowedOrigins is unset", async () => { const res = await request(createApp()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Origin", "https://evil.example.com"); expect(res.status).not.toBe(403); @@ -187,6 +200,7 @@ describe("origin guard (#104)", () => { it("rejects a same-site request from an Origin not on the list", async () => { const res = await request(allowlisted()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "same-site") .set("Origin", "https://evil.example.com"); @@ -197,6 +211,7 @@ describe("origin guard (#104)", () => { it("rejects a same-site request with no Origin", async () => { const res = await request(allowlisted()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "same-site"); expect(res.status).toBe(403); @@ -206,6 +221,7 @@ describe("origin guard (#104)", () => { it("accepts a same-site request from a listed Origin", async () => { const res = await request(allowlisted()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "same-site") .set("Origin", "https://APP.example.com"); @@ -215,6 +231,7 @@ describe("origin guard (#104)", () => { it("accepts a same-origin request whatever the list says", async () => { const res = await request(allowlisted()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "same-origin") .set("Origin", "https://auth-adapter.example.com"); @@ -224,6 +241,7 @@ describe("origin guard (#104)", () => { it("still rejects cross-site from a listed Origin", async () => { const res = await request(allowlisted()) .post("/auth/magic-link") + .set("Content-Type", "application/json") .set("Sec-Fetch-Site", "cross-site") .set("Origin", "https://app.example.com"); diff --git a/packages/express/tests/totpProxy.test.js b/packages/express/tests/totpProxy.test.js index 5cd3c59..8dbb59d 100644 --- a/packages/express/tests/totpProxy.test.js +++ b/packages/express/tests/totpProxy.test.js @@ -100,6 +100,7 @@ describe("TOTP proxy routes", () => { const res = await request(createApp()) .post("/auth/totp/enroll/start") + .set("Content-Type", "application/json") .set("Cookie", createAccessCookie()); expect(res.status).toBe(200); diff --git a/packages/express/tests/usersRoutes.test.js b/packages/express/tests/usersRoutes.test.js index eec2aee..a9ebd1c 100644 --- a/packages/express/tests/usersRoutes.test.js +++ b/packages/express/tests/usersRoutes.test.js @@ -117,6 +117,7 @@ describe("users proxy routes", () => { const res = await request(createApp()) .delete("/auth/users/delete") + .set("Content-Type", "application/json") .set("Cookie", createAccessCookie()); expect(res.status).toBe(200); @@ -148,6 +149,7 @@ describe("users proxy routes", () => { const res = await request(createApp()) .delete("/auth/users/delete") + .set("Content-Type", "application/json") .set("Cookie", createAccessCookie()); expect(res.status).toBe(404); @@ -158,7 +160,9 @@ describe("users proxy routes", () => { }); it("does not reach the auth API for a deletion without a session", async () => { - const res = await request(createApp()).delete("/auth/users/delete"); + const res = await request(createApp()) + .delete("/auth/users/delete") + .set("Content-Type", "application/json"); expect([400, 401]).toContain(res.status); expect(global.fetch).not.toHaveBeenCalled(); diff --git a/packages/fastify/src/hooks/originGuard.ts b/packages/fastify/src/hooks/originGuard.ts index a5fecd9..54b52af 100644 --- a/packages/fastify/src/hooks/originGuard.ts +++ b/packages/fastify/src/hooks/originGuard.ts @@ -1,27 +1,40 @@ import type { FastifyReply, FastifyRequest } from "fastify"; -import { checkOrigin, type CookieSameSite } from "@seamless-auth/core"; +import { + AUTH_TRANSPORT_HEADER, + checkContentType, + checkOrigin, +} from "@seamless-auth/core"; + +import { mountRelativePath } from "./ensureCookies.js"; export interface OriginGuardOptions { - cookieSecure?: boolean; - cookieSameSite?: CookieSameSite; allowedOrigins?: string[]; } /** - * Rejects cross-site state-changing requests when the plugin issues - * `SameSite=None` cookies. The decision is core's; this reads the headers and - * writes the response. + * Rejects cross-site state-changing requests, and a `POST` a page on another + * origin could send without a preflight. The decisions are core's; this reads + * the headers and writes the response. It runs `onRequest`, ahead of Fastify's + * own body parsing, which accepts `text/plain` and a missing content type. */ -export function createOriginGuardHook(opts: OriginGuardOptions) { +export function createOriginGuardHook( + opts: OriginGuardOptions, + prefix: string, +) { return async function originGuard(req: FastifyRequest, reply: FastifyReply) { - const rejection = checkOrigin({ - method: req.method, - secFetchSite: firstHeader(req.headers["sec-fetch-site"]), - origin: firstHeader(req.headers.origin), - cookieSecure: opts.cookieSecure, - cookieSameSite: opts.cookieSameSite, - allowedOrigins: opts.allowedOrigins, - }); + const rejection = + checkOrigin({ + method: req.method, + secFetchSite: firstHeader(req.headers["sec-fetch-site"]), + origin: firstHeader(req.headers.origin), + allowedOrigins: opts.allowedOrigins, + }) ?? + checkContentType({ + method: req.method, + path: mountRelativePath(req.url, prefix), + contentType: req.headers["content-type"], + transport: firstHeader(req.headers[AUTH_TRANSPORT_HEADER]), + }); if (rejection) { return reply diff --git a/packages/fastify/src/plugin.ts b/packages/fastify/src/plugin.ts index e6ff4f6..6078a08 100644 --- a/packages/fastify/src/plugin.ts +++ b/packages/fastify/src/plugin.ts @@ -91,9 +91,33 @@ export const seamlessAuth: FastifyPluginAsync< await fastify.register(cookie); + // A body-less POST declares JSON so the browser preflights it, which + // express.json() reads as no body. Fastify's own parser refuses an empty one. + // This plugin is encapsulated, so the adopter's other routes keep the default. + const parseJson = fastify.getDefaultJsonParser( + fastify.initialConfig.onProtoPoisoning ?? "error", + fastify.initialConfig.onConstructorPoisoning ?? "error", + ); + fastify.removeContentTypeParser("application/json"); + fastify.addContentTypeParser( + "application/json", + { parseAs: "string" }, + (request, body, done) => { + const text = body.toString(); + if (text === "") { + done(null, undefined); + return; + } + parseJson(request, text, done); + }, + ); + // Ordering matches the Express adapter: a blocked cross-site request must // never trigger a token refresh or reach a handler. - fastify.addHook("onRequest", createOriginGuardHook(resolved)); + fastify.addHook( + "onRequest", + createOriginGuardHook(resolved, fastify.prefix), + ); fastify.addHook( "onRequest", createEnsureCookiesHook(resolved, fastify.prefix, manifestSource), diff --git a/packages/fastify/tests/dotSegmentParams.test.js b/packages/fastify/tests/dotSegmentParams.test.js index bf79b7d..92f47d4 100644 --- a/packages/fastify/tests/dotSegmentParams.test.js +++ b/packages/fastify/tests/dotSegmentParams.test.js @@ -47,7 +47,7 @@ async function send(handler, method, path) { path: `/auth${path}`, headers: { cookie: accessCookie(), - ...(body ? { "content-type": "application/json" } : {}), + ...(method === "get" ? {} : { "content-type": "application/json" }), }, }, (res) => { diff --git a/packages/fastify/tests/formPostCsrf.test.js b/packages/fastify/tests/formPostCsrf.test.js index ca523ea..168273b 100644 --- a/packages/fastify/tests/formPostCsrf.test.js +++ b/packages/fastify/tests/formPostCsrf.test.js @@ -45,7 +45,11 @@ async function send({ method = "POST", path, headers, payload, options }) { ), ...(payload === undefined ? {} : { payload }), }); - return { status: res.statusCode, body: res.body ? JSON.parse(res.body) : null }; + return { + status: res.statusCode, + body: res.body ? JSON.parse(res.body) : null, + setCookie: res.headers["set-cookie"], + }; } finally { await app.close(); } @@ -178,14 +182,45 @@ describe("form POST CSRF with SameSite=Lax cookies (#256)", () => { expect(upstreamCalls("/recovery-codes")).toHaveLength(1); }); - it("does not require a content type on DELETE, which always preflights", async () => { + it("refuses a body-less DELETE that declares nothing", async () => { const res = await send({ method: "DELETE", path: "/sessions/s-1", headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, }); + expect(res.status).toBe(415); + expect(upstreamCalls("/sessions/s-1")).toHaveLength(0); + }); + + // The browser client up to now sends these bare, so they keep only the + // origin check. + it.each([ + ["POST", "/refresh"], + ["DELETE", "/logout"], + ["DELETE", "/logout/all"], + ])("leaves %s %s out of the content-type gate", async (method, path) => { + const res = await send({ + method, + path, + headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, + }); + expect(res.status).not.toBe(415); - expect(upstreamCalls("/sessions/s-1")).toHaveLength(1); }); + + it.each([["/logout"], ["/logout/all"]])( + "still applies the origin check to DELETE %s, clearing no cookie", + async (path) => { + const res = await send({ + method: "DELETE", + path, + headers: { cookie: accessCookie(), "sec-fetch-site": "cross-site" }, + }); + + expect(res.status).toBe(403); + expect(res.setCookie ?? []).toEqual([]); + expect(global.fetch).not.toHaveBeenCalled(); + }, + ); }); diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index 264d590..6bda665 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -117,13 +117,24 @@ function parseBody(text) { } } +// What the browser client sends: a state-changing request declares JSON even +// with no body, which forces a CORS preflight (#256). +function clientHeaders(method, headers) { + const declared = Object.keys(headers ?? {}).some( + (name) => name.toLowerCase() === "content-type", + ); + return method.toLowerCase() === "get" || declared + ? (headers ?? {}) + : { "content-type": "application/json", ...(headers ?? {}) }; +} + async function viaFastify({ method, path, cookie, headers, payload, options }) { const app = await buildFastify(options); try { const res = await app.inject({ method: method.toUpperCase(), url: `/auth${path}`, - headers: { ...(headers ?? {}), ...(cookie ? { cookie } : {}) }, + headers: { ...clientHeaders(method, headers), ...(cookie ? { cookie } : {}) }, ...(payload === undefined ? {} : { payload }), }); @@ -142,7 +153,7 @@ async function viaFastify({ method, path, cookie, headers, payload, options }) { async function viaExpress({ method, path, cookie, headers, payload, options }) { let req = request(buildExpress(options))[method](`/auth${path}`); - if (headers) req = req.set(headers); + req = req.set(clientHeaders(method, headers)); if (cookie) req = req.set("Cookie", cookie); if (payload !== undefined) req = req.send(payload); diff --git a/packages/nextjs/src/guards.ts b/packages/nextjs/src/guards.ts index 1f21b41..4a50eb8 100644 --- a/packages/nextjs/src/guards.ts +++ b/packages/nextjs/src/guards.ts @@ -91,8 +91,6 @@ function prepare(name: string, opts: SeamlessGuardOptions) { method: request.method, secFetchSite: request.headers.get("sec-fetch-site") ?? undefined, origin: request.headers.get("origin") ?? undefined, - cookieSecure: opts.cookieSecure, - cookieSameSite: opts.cookieSameSite, allowedOrigins: opts.allowedOrigins, }); diff --git a/packages/nextjs/src/handler.ts b/packages/nextjs/src/handler.ts index 5400968..7306549 100644 --- a/packages/nextjs/src/handler.ts +++ b/packages/nextjs/src/handler.ts @@ -4,6 +4,8 @@ import { type AppliableResult, applyCookies, assertSecrets, + AUTH_TRANSPORT_HEADER, + checkContentType, checkOrigin, createAdapterManifestSource, DEV_JWKS_KID, @@ -159,16 +161,24 @@ async function dispatch( collector: ResponseCollector, manifestSource: AdapterManifestSource, ): Promise { + const url = new URL(request.url); + const path = mountRelativePath(url.pathname, opts.basePath); + // Ordering matches the other adapters: a blocked cross-site request must // never trigger a token refresh or reach a handler. - const originRejection = checkOrigin({ - method: request.method, - secFetchSite: request.headers.get("sec-fetch-site") ?? undefined, - origin: request.headers.get("origin") ?? undefined, - cookieSecure: opts.cookieSecure, - cookieSameSite: opts.cookieSameSite, - allowedOrigins: opts.allowedOrigins, - }); + const originRejection = + checkOrigin({ + method: request.method, + secFetchSite: request.headers.get("sec-fetch-site") ?? undefined, + origin: request.headers.get("origin") ?? undefined, + allowedOrigins: opts.allowedOrigins, + }) ?? + checkContentType({ + method: request.method, + path, + contentType: request.headers.get("content-type") ?? undefined, + transport: request.headers.get(AUTH_TRANSPORT_HEADER) ?? undefined, + }); if (originRejection) { return collector.json(originRejection.status, { @@ -176,8 +186,6 @@ async function dispatch( }); } - const url = new URL(request.url); - const path = mountRelativePath(url.pathname, opts.basePath); const manifest = await manifestSource.get(); const target = resolveTarget(manifest, request, path, opts); diff --git a/packages/nextjs/tests/formPostCsrf.test.js b/packages/nextjs/tests/formPostCsrf.test.js index 909db14..8bd814d 100644 --- a/packages/nextjs/tests/formPostCsrf.test.js +++ b/packages/nextjs/tests/formPostCsrf.test.js @@ -43,7 +43,11 @@ async function send({ method = "POST", path, headers, payload, options }) { }), ); const text = await res.text(); - return { status: res.status, body: text ? JSON.parse(text) : null }; + return { + status: res.status, + body: text ? JSON.parse(text) : null, + setCookie: res.headers.getSetCookie(), + }; } function upstreamCalls(path) { @@ -173,14 +177,45 @@ describe("form POST CSRF with SameSite=Lax cookies (#256)", () => { expect(upstreamCalls("/recovery-codes")).toHaveLength(1); }); - it("does not require a content type on DELETE, which always preflights", async () => { + it("refuses a body-less DELETE that declares nothing", async () => { const res = await send({ method: "DELETE", path: "/sessions/s-1", headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, }); + expect(res.status).toBe(415); + expect(upstreamCalls("/sessions/s-1")).toHaveLength(0); + }); + + // The browser client up to now sends these bare, so they keep only the + // origin check. + it.each([ + ["POST", "/refresh"], + ["DELETE", "/logout"], + ["DELETE", "/logout/all"], + ])("leaves %s %s out of the content-type gate", async (method, path) => { + const res = await send({ + method, + path, + headers: { cookie: accessCookie(), "sec-fetch-site": "same-origin" }, + }); + expect(res.status).not.toBe(415); - expect(upstreamCalls("/sessions/s-1")).toHaveLength(1); }); + + it.each([["/logout"], ["/logout/all"]])( + "still applies the origin check to DELETE %s, clearing no cookie", + async (path) => { + const res = await send({ + method: "DELETE", + path, + headers: { cookie: accessCookie(), "sec-fetch-site": "cross-site" }, + }); + + expect(res.status).toBe(403); + expect(res.setCookie ?? []).toEqual([]); + expect(global.fetch).not.toHaveBeenCalled(); + }, + ); }); diff --git a/packages/nextjs/tests/handler.test.js b/packages/nextjs/tests/handler.test.js index 2044c6a..3c7441c 100644 --- a/packages/nextjs/tests/handler.test.js +++ b/packages/nextjs/tests/handler.test.js @@ -104,7 +104,10 @@ describe("createSeamlessAuthHandler", () => { }); it("answers 404 for a known path under the wrong method", async () => { - const res = await call("/auth/oauth/providers", { method: "DELETE" }); + const res = await call("/auth/oauth/providers", { + method: "DELETE", + headers: { "content-type": "application/json" }, + }); expect(res.status).toBe(404); }); @@ -174,12 +177,17 @@ describe("createSeamlessAuthHandler", () => { expect(await res.json()).toEqual({ error: "payload_too_large" }); }); + // The transport header gets it past the content-type gate (#256), so this + // reaches the body parser. it("ignores a body that is not JSON", async () => { global.fetch = jest.fn(async () => upstream(400, { error: "invalid" })); await call("/auth/login", { method: "POST", - headers: { "content-type": "text/plain" }, + headers: { + "content-type": "text/plain", + "x-seamless-auth-transport": "cookie", + }, body: "identifier=a@b.c", }); diff --git a/packages/nextjs/tests/parity.test.js b/packages/nextjs/tests/parity.test.js index ae6b6fb..7411858 100644 --- a/packages/nextjs/tests/parity.test.js +++ b/packages/nextjs/tests/parity.test.js @@ -119,13 +119,23 @@ function parseBody(text) { } } +// What the browser client sends: a state-changing request declares JSON even +// with no body, which forces a CORS preflight (#256). +function clientHeaders(method, headers) { + const declared = Object.keys(headers ?? {}).some( + (name) => name.toLowerCase() === "content-type", + ); + return method.toLowerCase() === "get" || declared + ? (headers ?? {}) + : { "content-type": "application/json", ...(headers ?? {}) }; +} + async function viaNext({ method, path, cookie, headers, payload, options }) { const handlers = createSeamlessAuthHandler({ ...OPTIONS, ...options }); const verb = method.toUpperCase(); const requestHeaders = { - ...(headers ?? {}), + ...clientHeaders(method, headers), ...(cookie ? { cookie } : {}), - ...(payload === undefined ? {} : { "content-type": "application/json" }), }; const res = await handlers[verb]( @@ -145,7 +155,7 @@ async function viaNext({ method, path, cookie, headers, payload, options }) { async function viaExpress({ method, path, cookie, headers, payload, options }) { let req = request(buildExpress(options))[method](`/auth${path}`); - if (headers) req = req.set(headers); + req = req.set(clientHeaders(method, headers)); if (cookie) req = req.set("Cookie", cookie); if (payload !== undefined) req = req.send(payload); From 00468d9ffbc879a6e5d8db8fcf2c4427210119e6 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:36:59 -0500 Subject: [PATCH 03/17] test(core): reproduce replay of a completed refresh rotation A second caller presenting the same refresh cookie, session token or bearer refresh token after a completed rotation gets the new session from the 5 second completed-result window, and the auth API is never called. These tests fail on main. Refs #258 --- packages/core/tests/refreshReplay.test.js | 109 ++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 packages/core/tests/refreshReplay.test.js diff --git a/packages/core/tests/refreshReplay.test.js b/packages/core/tests/refreshReplay.test.js new file mode 100644 index 0000000..224f87d --- /dev/null +++ b/packages/core/tests/refreshReplay.test.js @@ -0,0 +1,109 @@ +// A refresh token presented again after its rotation completed is a replay. It +// must reach the auth API, whose reuse detection revokes the session family, +// rather than be handed the session the first caller got (#258). +import { jest } from "@jest/globals"; + +const verifyRefreshCookieMock = jest.fn(); +const authFetchMock = jest.fn(); + +jest.unstable_mockModule("../dist/verifyRefreshCookie.js", () => ({ + verifyRefreshCookie: verifyRefreshCookieMock, +})); + +jest.unstable_mockModule("../dist/authFetch.js", () => ({ + authFetch: authFetchMock, +})); + +const { refreshAccessToken, refreshSessionToken, refreshBearerSession } = + await import("../dist/refreshAccessToken.js"); + +const OPTS = { + authServerUrl: "https://auth.example.com", + cookieSecret: "cookie-secret-cookie-secret-cookie-secret", + serviceSecret: "service-secret-service-secret-service-secret", + issuer: "https://frontend.example.com", + audience: "https://auth.example.com", + keyId: "dev-main", +}; + +const ROTATED = { + sub: "user-123", + token: "new-access", + refreshToken: "new-refresh", + ttl: 300, + refreshTtl: 3600, +}; + +const rotated = () => ({ ok: true, status: 200, json: async () => ROTATED }); +const reused = () => ({ + ok: false, + status: 401, + json: async () => ({ error: "refresh_token_reused" }), +}); + +let counter = 0; +const unique = (label) => `${label}-${(counter += 1)}`; + +describe("a completed refresh rotation is not replayed (#258)", () => { + beforeEach(() => { + verifyRefreshCookieMock.mockReset(); + authFetchMock.mockReset(); + authFetchMock.mockResolvedValueOnce(rotated()).mockResolvedValue(reused()); + }); + + it("sends a stolen refresh cookie to the auth API after the rotation", async () => { + const cookie = unique("cookie"); + verifyRefreshCookieMock.mockReturnValue({ sub: "user-123", refreshToken: "old" }); + + const victim = await refreshAccessToken(cookie, OPTS); + const attacker = await refreshAccessToken(cookie, OPTS); + + expect(victim).toEqual(ROTATED); + expect(authFetchMock).toHaveBeenCalledTimes(2); + expect(attacker).toBeNull(); + }); + + it("sends a spent session token to the auth API after the rotation", async () => { + const payload = { sub: "user-123", refreshToken: unique("token") }; + + const victim = await refreshSessionToken(payload, OPTS); + const attacker = await refreshSessionToken(payload, OPTS); + + expect(victim).toEqual(ROTATED); + expect(authFetchMock).toHaveBeenCalledTimes(2); + expect(attacker).toBeNull(); + }); + + it("sends a stolen bearer refresh token to the auth API after the rotation", async () => { + const token = unique("bearer"); + const opts = { authServerUrl: OPTS.authServerUrl }; + + const victim = await refreshBearerSession(token, opts); + const attacker = await refreshBearerSession(token, opts); + + expect(victim).toEqual({ status: 200, body: ROTATED }); + expect(authFetchMock).toHaveBeenCalledTimes(2); + expect(attacker).toEqual({ + status: 401, + body: { error: "refresh_token_reused" }, + }); + }); + + it("still collapses callers that race one rotation in flight", async () => { + const token = unique("bearer"); + const opts = { authServerUrl: OPTS.authServerUrl }; + let release; + authFetchMock.mockReset(); + authFetchMock.mockImplementationOnce( + () => new Promise((resolve) => (release = () => resolve(rotated()))), + ); + + const first = refreshBearerSession(token, opts); + const second = refreshBearerSession(token, opts); + release(); + + expect(await first).toEqual({ status: 200, body: ROTATED }); + expect(await second).toEqual({ status: 200, body: ROTATED }); + expect(authFetchMock).toHaveBeenCalledTimes(1); + }); +}); From 41bc08b7fbe4d9bdfd5176f68d29d4043f0a940a Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:40:23 -0500 Subject: [PATCH 04/17] fix(core): share a refresh rotation only while it is in flight dedupeRefresh kept a completed rotation for 5 seconds and handed it to any caller presenting the same refresh cookie, session token or bearer refresh token, without calling the auth API. A stolen refresh token replayed in that window got the new session, and the API's reuse detection never saw it. Drop the completed-result window for every transport. Callers that race one rotation still share it; a caller that arrives after it completed goes to the auth API as a replay. Refs #258 --- .changeset/refresh-in-flight-only.md | 10 +++ packages/core/src/refreshAccessToken.ts | 61 +++---------------- .../core/tests/refreshAccessToken.test.js | 6 +- packages/core/tests/transport.test.js | 11 ++-- packages/express/README.md | 2 +- packages/fastify/tests/issuer.parity.test.js | 4 +- packages/fastify/tests/parity.test.js | 4 +- packages/nextjs/tests/issuer.parity.test.js | 4 +- packages/nextjs/tests/parity.test.js | 4 +- 9 files changed, 38 insertions(+), 68 deletions(-) create mode 100644 .changeset/refresh-in-flight-only.md diff --git a/.changeset/refresh-in-flight-only.md b/.changeset/refresh-in-flight-only.md new file mode 100644 index 0000000..876246e --- /dev/null +++ b/.changeset/refresh-in-flight-only.md @@ -0,0 +1,10 @@ +--- +"@seamless-auth/core": patch +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Stop replaying a completed refresh rotation to later callers (#258). After a successful rotation, the adapter used to hand the new session to any caller presenting the same old refresh cookie or bearer refresh token for the next 5 seconds, without calling the auth API, so a stolen refresh token replayed in that window got the new tokens and the auth API's reuse detection never saw it. + +Only a rotation still in flight is shared now. A request that presents a refresh token after its rotation completed goes to the auth API, which treats it as a replay: the cookie path answers as an expired session, and a bearer client gets the API's `401 {"error":"refresh_token_reused"}`. Two browser tabs that send the same old refresh cookie a moment apart, rather than at the same time, can now trip that reuse detection and sign the user out. diff --git a/packages/core/src/refreshAccessToken.ts b/packages/core/src/refreshAccessToken.ts index dbd1d18..0bdc7bc 100644 --- a/packages/core/src/refreshAccessToken.ts +++ b/packages/core/src/refreshAccessToken.ts @@ -31,53 +31,21 @@ const inFlightRefreshes = new Map< string, Promise >(); -const recentRefreshResults = new Map< - string, - { result: RefreshAccessTokenResult; expiresAt: number } ->(); -const RECENT_REFRESH_RESULT_TTL_MS = 5000; -// Refresh cookies rotate, so a completed entry's key is never looked up again and -// would live forever without this. Sweep expired entries (throttled so it stays off -// the hot path) and cap total size as a backstop against a burst of distinct cookies. -const MAX_RECENT_REFRESH_RESULTS = 10_000; -let lastPruneAt = 0; - -function pruneRecentRefreshResults(now: number): void { - if (now - lastPruneAt >= RECENT_REFRESH_RESULT_TTL_MS) { - for (const [key, entry] of recentRefreshResults) { - if (entry.expiresAt <= now) { - recentRefreshResults.delete(key); - } - } - lastPruneAt = now; - } - - while (recentRefreshResults.size > MAX_RECENT_REFRESH_RESULTS) { - const oldest = recentRefreshResults.keys().next().value; - if (oldest === undefined) break; - recentRefreshResults.delete(oldest); - } -} /** - * Runs one refresh per key at a time and replays a recent success to callers - * arriving just after it. The auth API rotates refresh tokens and treats a - * replay as theft, revoking the whole chain, so two requests that race the same - * token must be collapsed here rather than both reach it. + * Runs one refresh per key at a time. The auth API rotates refresh tokens and + * treats a replay as theft, revoking the whole chain, so two requests that race + * the same token must be collapsed here rather than both reach it. + * + * Only a rotation still in flight is shared. Once it completes, a caller that + * presents the same token again goes to the auth API, whose reuse detection is + * what catches a stolen token; handing that caller the new session would hide + * the replay. */ async function dedupeRefresh( key: string, run: () => Promise, ): Promise { - const now = Date.now(); - const recentRefresh = recentRefreshResults.get(key); - if (recentRefresh && recentRefresh.expiresAt > now) { - return recentRefresh.result as T; - } - if (recentRefresh) { - recentRefreshResults.delete(key); - } - const existingRefresh = inFlightRefreshes.get(key); if (existingRefresh) { return existingRefresh as Promise; @@ -87,16 +55,7 @@ async function dedupeRefresh( inFlightRefreshes.set(key, refreshPromise); try { - const result = await refreshPromise; - if (result) { - const insertedAt = Date.now(); - pruneRecentRefreshResults(insertedAt); - recentRefreshResults.set(key, { - result, - expiresAt: insertedAt + RECENT_REFRESH_RESULT_TTL_MS, - }); - } - return result; + return await refreshPromise; } finally { inFlightRefreshes.delete(key); } @@ -202,8 +161,6 @@ export async function refreshBearerSession( }); if (!response.ok) { - // Not cached, so a failed rotation is reported to every caller that - // asked for it rather than replayed as a success. throw new UpstreamRefreshFailure(response.status, await response.json()); } diff --git a/packages/core/tests/refreshAccessToken.test.js b/packages/core/tests/refreshAccessToken.test.js index fc483ef..6d490bc 100644 --- a/packages/core/tests/refreshAccessToken.test.js +++ b/packages/core/tests/refreshAccessToken.test.js @@ -138,7 +138,9 @@ describe("refreshAccessToken", () => { expect(authFetchMock).toHaveBeenCalledTimes(1); }); - it("reuses the freshly-rotated result for an immediate follow-up call with the stale cookie", async () => { + // A completed rotation is not shared: the stale cookie is a replay the auth + // API must see (#258). + it("sends a follow-up call with the stale cookie to the auth API", async () => { const { refreshAccessToken } = await import("../dist/refreshAccessToken.js"); @@ -180,6 +182,6 @@ describe("refreshAccessToken", () => { }); expect(first).toEqual(second); - expect(authFetchMock).toHaveBeenCalledTimes(1); + expect(authFetchMock).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/core/tests/transport.test.js b/packages/core/tests/transport.test.js index 2b50abe..2d2db47 100644 --- a/packages/core/tests/transport.test.js +++ b/packages/core/tests/transport.test.js @@ -237,11 +237,12 @@ describe("refreshBearerSession", () => { expect(b.body).toEqual(a.body); expect(global.fetch).toHaveBeenCalledTimes(1); - // A straggler just after completion gets the same rotated pair rather than - // replaying a token the auth API has already retired. - const third = await refreshBearerSession("same-token", opts); - expect(third.body).toEqual(a.body); - expect(global.fetch).toHaveBeenCalledTimes(1); + // Once the rotation completes, the same token is a replay for the auth API + // to judge, not a share of the new pair (#258). + const third = refreshBearerSession("same-token", opts); + resolveUpstream(); + await third; + expect(global.fetch).toHaveBeenCalledTimes(2); }); it("passes the auth API's failure through and does not cache it", async () => { diff --git a/packages/express/README.md b/packages/express/README.md index d5feca0..3e7b08e 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -541,7 +541,7 @@ routes serve a second contract when the request carries the header the pair in the response. The auth API rotates refresh tokens and treats a replayed one as theft (revoking the whole chain, `401 { "error": "refresh_token_reused" }`), so refresh once at a time. Concurrent rotations of the same token that reach the adapter together are collapsed into one - upstream call. + upstream call. One that arrives after that rotation completed goes to the auth API as a replay. - Message delivery, client IP and user agent forwarding, and the service token behave exactly as in cookie transport, since the adapter attaches them regardless of how the client carries its session. diff --git a/packages/fastify/tests/issuer.parity.test.js b/packages/fastify/tests/issuer.parity.test.js index 3b0e925..a138792 100644 --- a/packages/fastify/tests/issuer.parity.test.js +++ b/packages/fastify/tests/issuer.parity.test.js @@ -120,8 +120,8 @@ const preAuthCookie = () => { algorithm: "HS256", expiresIn: "300s" }, )}`; -// Each silent refresh needs its own refresh token: core replays a recent -// refresh result for the same token rather than calling the auth API again. +// Each silent refresh uses its own refresh token, as a real rotation chain does, +// so no two tests share a rotation core still has in flight. let refreshCount = 0; const silentRefresh = () => ({ method: "get", diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index 6bda665..6c4c922 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -1847,8 +1847,8 @@ describe("fastify and express serve the OAuth provider discovery refresh", () => }); // A recovery session that enrolls a factor is rotated at once, so its new access -// cookie drops the recovery claim (#235). Refresh tokens are unique per run: the -// adapters replay a recent refresh for the same token, which would hide the call. +// cookie drops the recovery claim (#235). Refresh tokens are unique per run, as +// in a real rotation chain, so each run's refresh call is its own. describe("fastify and express rotate a recovery session after enrollment", () => { const originalFetch = global.fetch; afterEach(() => { diff --git a/packages/nextjs/tests/issuer.parity.test.js b/packages/nextjs/tests/issuer.parity.test.js index 87c44ea..3fad36d 100644 --- a/packages/nextjs/tests/issuer.parity.test.js +++ b/packages/nextjs/tests/issuer.parity.test.js @@ -120,8 +120,8 @@ const preAuthCookie = () => { algorithm: "HS256", expiresIn: "300s" }, )}`; -// Each silent refresh needs its own refresh token: core replays a recent -// refresh result for the same token rather than calling the auth API again. +// Each silent refresh uses its own refresh token, as a real rotation chain does, +// so no two tests share a rotation core still has in flight. let refreshCount = 0; const silentRefresh = () => ({ method: "get", diff --git a/packages/nextjs/tests/parity.test.js b/packages/nextjs/tests/parity.test.js index 7411858..859356a 100644 --- a/packages/nextjs/tests/parity.test.js +++ b/packages/nextjs/tests/parity.test.js @@ -1815,8 +1815,8 @@ describe("next and express serve the OAuth provider discovery refresh", () => { }); // A recovery session that enrolls a factor is rotated at once, so its new access -// cookie drops the recovery claim (#235). Refresh tokens are unique per run: the -// adapters replay a recent refresh for the same token, which would hide the call. +// cookie drops the recovery claim (#235). Refresh tokens are unique per run, as +// in a real rotation chain, so each run's refresh call is its own. describe("next and express rotate a recovery session after enrollment", () => { const originalFetch = global.fetch; afterEach(() => { From 09f599b60b41eb2bc54c5f8323ba5c0dab2136c4 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:42:43 -0500 Subject: [PATCH 05/17] test: reproduce guards that skip the origin check Express and Fastify requireAuth let a cross-site, unlisted same-site or null-Origin state change through on a cookie session, where the Next.js guard refuses it. checkOrigin also passes a literal null Origin when Sec-Fetch-Site is same-origin, same-site or none. These tests fail on the branch. Refs #256 --- packages/core/tests/checkOrigin.test.js | 16 +++ .../express/tests/requireAuth.origin.test.js | 105 +++++++++++++++++ packages/fastify/tests/guards.origin.test.js | 108 ++++++++++++++++++ 3 files changed, 229 insertions(+) create mode 100644 packages/express/tests/requireAuth.origin.test.js create mode 100644 packages/fastify/tests/guards.origin.test.js diff --git a/packages/core/tests/checkOrigin.test.js b/packages/core/tests/checkOrigin.test.js index b019557..9b4b770 100644 --- a/packages/core/tests/checkOrigin.test.js +++ b/packages/core/tests/checkOrigin.test.js @@ -19,6 +19,22 @@ describe("checkOrigin", () => { expect(check({ origin: "null" })).toEqual(BLOCKED); }); + // An opaque origin is never the adopter's own page, whatever else the + // request says. + it.each(["same-origin", "same-site", "none", "cross-site"])( + "refuses a literal null Origin with Sec-Fetch-Site %s", + (secFetchSite) => { + expect(check({ secFetchSite, origin: "null" })).toEqual(BLOCKED); + expect( + check({ secFetchSite, origin: "null", allowedOrigins: ["null", LISTED] }), + ).toEqual(BLOCKED); + }, + ); + + it("refuses a literal null Origin even when allowedOrigins lists it", () => { + expect(check({ origin: "null", allowedOrigins: ["null"] })).toEqual(BLOCKED); + }); + it("refuses a same-site request from an Origin off allowedOrigins", () => { expect( check({ secFetchSite: "same-site", origin: SIBLING, allowedOrigins: [LISTED] }), diff --git a/packages/express/tests/requireAuth.origin.test.js b/packages/express/tests/requireAuth.origin.test.js new file mode 100644 index 0000000..776a8ad --- /dev/null +++ b/packages/express/tests/requireAuth.origin.test.js @@ -0,0 +1,105 @@ +// The guard on an adopter's own route runs the same origin check as the auth +// router and the Next.js guard, for a cookie session only (#256). +import express from "express"; +import cookieParser from "cookie-parser"; +import jwt from "jsonwebtoken"; +import request from "supertest"; + +const { requireAuth, requireRole } = await import("../dist/index.js"); + +const SECRET = "cookie-secret-cookie-secret-cookie-secret"; +const INNER = jwt.sign({ sub: "user-1", typ: "access" }, "upstream-signing-key"); +const cookie = () => + `seamless-access=${jwt.sign({ sub: "user-1", roles: ["admin"], token: INNER }, SECRET, { expiresIn: "1h" })}`; + +const BLOCKED = { error: "cross_site_request_blocked" }; +const LISTED = "https://app.example.com"; + +function createApp(options = {}, { role } = {}) { + const app = express(); + app.use(cookieParser()); + const chain = [ + requireAuth({ cookieSecret: SECRET, ...options }), + ...(role ? [requireRole(role)] : []), + ]; + app.all("/api/thing", ...chain, (req, res) => res.json({ id: req.user.id })); + return app; +} + +function send(app, method, headers) { + let req = request(app)[method]("/api/thing").set("Cookie", cookie()); + for (const [name, value] of Object.entries(headers)) req = req.set(name, value); + return req; +} + +describe("requireAuth origin check (express)", () => { + it.each([["post"], ["put"], ["patch"], ["delete"]])( + "refuses a cross-site %s on a cookie session", + async (method) => { + const res = await send(createApp(), method, { "Sec-Fetch-Site": "cross-site" }); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }, + ); + + it("refuses a cross-site state change behind requireRole too", async () => { + const res = await send(createApp({}, { role: "admin" }), "post", { + "Sec-Fetch-Site": "cross-site", + }); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }); + + it("refuses a same-site state change from an unlisted Origin", async () => { + const res = await send(createApp({ allowedOrigins: [LISTED] }), "post", { + "Sec-Fetch-Site": "same-site", + Origin: "https://sibling.example.com", + }); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }); + + it("refuses a literal null Origin", async () => { + const res = await send(createApp(), "post", { Origin: "null" }); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }); + + // Bearer and cookieless callers are not riding an ambient credential. + it("skips the check when the request carries no session cookie", async () => { + const res = await request(createApp()) + .post("/api/thing") + .set("Sec-Fetch-Site", "cross-site"); + + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: "unauthenticated" }); + }); + + it("accepts a same-site state change from a listed Origin", async () => { + const res = await send(createApp({ allowedOrigins: [LISTED] }), "post", { + "Sec-Fetch-Site": "same-site", + Origin: LISTED, + }); + + expect(res.status).toBe(200); + }); + + it("leaves a cross-site read alone", async () => { + const res = await send(createApp(), "get", { "Sec-Fetch-Site": "cross-site" }); + + expect(res.status).toBe(200); + }); + + it("asks for no JSON content type on the adopter's route", async () => { + const res = await send(createApp(), "post", { + "Sec-Fetch-Site": "same-origin", + "Content-Type": "text/plain", + }); + + expect(res.status).toBe(200); + }); +}); diff --git a/packages/fastify/tests/guards.origin.test.js b/packages/fastify/tests/guards.origin.test.js new file mode 100644 index 0000000..9318936 --- /dev/null +++ b/packages/fastify/tests/guards.origin.test.js @@ -0,0 +1,108 @@ +// The guard on an adopter's own route runs the same origin check as the auth +// plugin and the Next.js guard, for a cookie session only (#256). +import cookiePlugin from "@fastify/cookie"; +import Fastify from "fastify"; +import jwt from "jsonwebtoken"; + +const { requireAuth, requireRole } = await import("../dist/index.js"); + +const SECRET = "cookie-secret-cookie-secret-cookie-secret"; +const INNER = jwt.sign({ sub: "user-1", typ: "access" }, "upstream-signing-key"); +const cookie = () => + `seamless-access=${jwt.sign({ sub: "user-1", roles: ["admin"], token: INNER }, SECRET, { expiresIn: "1h" })}`; + +const BLOCKED = { error: "cross_site_request_blocked" }; +const LISTED = "https://app.example.com"; + +async function send(method, headers, options = {}, { role } = {}) { + const app = Fastify(); + await app.register(cookiePlugin); + const guard = requireAuth({ cookieSecret: SECRET, ...options }); + app.route({ + method: ["GET", "POST", "PUT", "PATCH", "DELETE"], + url: "/api/thing", + preHandler: role ? [guard, requireRole(role)] : guard, + handler: async (req) => ({ id: req.user.id }), + }); + await app.ready(); + try { + const res = await app.inject({ + method, + url: "/api/thing", + headers: { cookie: cookie(), ...headers }, + }); + return { status: res.statusCode, body: res.json() }; + } finally { + await app.close(); + } +} + +describe("requireAuth origin check (fastify)", () => { + it.each([["POST"], ["PUT"], ["PATCH"], ["DELETE"]])( + "refuses a cross-site %s on a cookie session", + async (method) => { + const res = await send(method, { "sec-fetch-site": "cross-site" }); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }, + ); + + it("refuses a cross-site state change behind requireRole too", async () => { + const res = await send("POST", { "sec-fetch-site": "cross-site" }, {}, { role: "admin" }); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }); + + it("refuses a same-site state change from an unlisted Origin", async () => { + const res = await send( + "POST", + { "sec-fetch-site": "same-site", origin: "https://sibling.example.com" }, + { allowedOrigins: [LISTED] }, + ); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }); + + it("refuses a literal null Origin", async () => { + const res = await send("POST", { origin: "null" }); + + expect(res.status).toBe(403); + expect(res.body).toEqual(BLOCKED); + }); + + // Bearer and cookieless callers are not riding an ambient credential. + it("skips the check when the request carries no session cookie", async () => { + const res = await send("POST", { "sec-fetch-site": "cross-site", cookie: "" }); + + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: "unauthenticated" }); + }); + + it("accepts a same-site state change from a listed Origin", async () => { + const res = await send( + "POST", + { "sec-fetch-site": "same-site", origin: LISTED }, + { allowedOrigins: [LISTED] }, + ); + + expect(res.status).toBe(200); + }); + + it("leaves a cross-site read alone", async () => { + const res = await send("GET", { "sec-fetch-site": "cross-site" }); + + expect(res.status).toBe(200); + }); + + it("asks for no JSON content type on the adopter's route", async () => { + const res = await send("POST", { + "sec-fetch-site": "same-origin", + "content-type": "text/plain", + }); + + expect(res.status).toBe(200); + }); +}); From e5608a536b2bc04f727e53eff0a83982c1131623 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:44:04 -0500 Subject: [PATCH 06/17] fix: run the origin check in the Express and Fastify guards requireAuth in Express and Fastify never called checkOrigin, so a cross-site, unlisted same-site or null-Origin state change on a cookie session reached the adopter's route, where the Next.js guard and the Go, Python and Rust adapters refuse it. Run the origin check on a cookie session, before authentication, with a new allowedOrigins option. Bearer requests are not checked and no content type is required. checkOrigin now refuses a literal null Origin whatever Sec-Fetch-Site and allowedOrigins say, instead of only when Sec-Fetch-Site is absent. Refs #256 --- .changeset/guard-origin-check.md | 12 +++++++ packages/core/src/guards.ts | 14 ++++---- .../express/src/middleware/requireAuth.ts | 33 ++++++++++++++++++- packages/fastify/src/guards.ts | 32 +++++++++++++++++- 4 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 .changeset/guard-origin-check.md diff --git a/.changeset/guard-origin-check.md b/.changeset/guard-origin-check.md new file mode 100644 index 0000000..cc0b88a --- /dev/null +++ b/.changeset/guard-origin-check.md @@ -0,0 +1,12 @@ +--- +"@seamless-auth/core": patch +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +--- + +Run the origin check in the Express and Fastify `requireAuth` guards, as the Next.js guards and the Go, Python and Rust adapters already do (#256), and refuse a literal `null` `Origin` in every case. + +Requests now refused: + +- On a cookie session, Express and Fastify `requireAuth` (and so `requireRole` behind it) answer a state-changing request (anything but `GET`, `HEAD` and `OPTIONS`) with `403 {"error":"cross_site_request_blocked"}` when `Sec-Fetch-Site` is `cross-site` or the `Origin` is a literal `null`. A new `allowedOrigins` option, which takes the same list as the auth router, also requires a listed `Origin` for a `same-site` request or one with no `Sec-Fetch-Site`; an empty list is enforced. A bearer request is not checked, and the guards ask for no JSON content type. +- `checkOrigin` refuses a literal `null` `Origin` whatever `Sec-Fetch-Site` and `allowedOrigins` say. It used to pass one sent with `Sec-Fetch-Site: same-origin`, `same-site` or `none`. This applies to the auth routes of every adapter and to the Next.js guards. diff --git a/packages/core/src/guards.ts b/packages/core/src/guards.ts index abae116..49d52bb 100644 --- a/packages/core/src/guards.ts +++ b/packages/core/src/guards.ts @@ -70,6 +70,8 @@ function normalizeOrigin(value: string): string { * cookie off a cross-site request but not off a same-site one, and a sibling * subdomain is same-site. * + * A literal `null` `Origin` is refused first, in every case. + * * `Sec-Fetch-Site` is the primary signal: current browsers send it and page * JavaScript cannot forge it. `cross-site` is refused and `same-origin` passes. * `same-site` also covers a sibling subdomain, so when the adopter configured @@ -90,6 +92,12 @@ export function checkOrigin( errorCode: "cross_site_request_blocked", }; + // A literal `null` origin is opaque or sandboxed, never the adopter's own + // page, so it is refused whatever `Sec-Fetch-Site` and the allowlist say. + if (input.origin?.trim().toLowerCase() === "null") { + return rejection; + } + if (input.secFetchSite !== undefined) { const site = input.secFetchSite.toLowerCase(); @@ -113,12 +121,6 @@ export function checkOrigin( return undefined; } - // A literal `null` origin is opaque or sandboxed, which is cross-site - // regardless of the allowlist. - if (input.origin === "null") { - return rejection; - } - if (!input.allowedOrigins) { return undefined; } diff --git a/packages/express/src/middleware/requireAuth.ts b/packages/express/src/middleware/requireAuth.ts index effcf54..848cf5e 100644 --- a/packages/express/src/middleware/requireAuth.ts +++ b/packages/express/src/middleware/requireAuth.ts @@ -2,6 +2,7 @@ import { Request, Response, NextFunction } from "express"; import { assertSecretStrength, authenticateRequest, + checkOrigin, getSeamlessLogger, } from "@seamless-auth/core"; @@ -36,6 +37,14 @@ export interface RequireAuthOptions { * 403 `enrollment_required`; turn it on only for routes that drive that enrollment. */ allowRecoverySession?: boolean; + /** + * `Origin` allowlist for a state-changing request on a cookie session that is + * `same-site` or carries no `Sec-Fetch-Site`, as on the auth router. Pass the + * same list. Set, even to `[]`, it is enforced; left out, such a request + * passes. A `cross-site` one or a literal `null` `Origin` is refused either + * way. + */ + allowedOrigins?: string[]; } /** @@ -78,6 +87,7 @@ export function requireAuth(opts: RequireAuthOptions) { audience, authServerIssuer, allowRecoverySession, + allowedOrigins, } = opts; // Eagerly, so a weak secret fails at setup rather than on the first request. @@ -108,8 +118,25 @@ export function requireAuth(opts: RequireAuthOptions) { : "Ensure you are using `cookieParser` in your express server"; return async function (req: Request, res: Response, next: NextFunction) { + const token = req.cookies?.[cookieName]; + + // A cookie rides a forged request on its own; a bearer token never does. + if (token) { + const blocked = checkOrigin({ + method: req.method, + secFetchSite: firstHeader(req.headers["sec-fetch-site"]), + origin: firstHeader(req.headers.origin), + allowedOrigins, + }); + + if (blocked) { + res.status(blocked.status).json({ error: blocked.errorCode }); + return; + } + } + const { user, rejection } = await authenticateRequest({ - token: req.cookies?.[cookieName], + token, cookieSecret, authorization: req.headers.authorization, bearer, @@ -131,3 +158,7 @@ export function requireAuth(opts: RequireAuthOptions) { next(); }; } + +function firstHeader(value: string | string[] | undefined): string | undefined { + return Array.isArray(value) ? value[0] : value; +} diff --git a/packages/fastify/src/guards.ts b/packages/fastify/src/guards.ts index f161121..d6f0074 100644 --- a/packages/fastify/src/guards.ts +++ b/packages/fastify/src/guards.ts @@ -3,6 +3,7 @@ import { assertSecretStrength, authenticateRequest, authorizeRoles, + checkOrigin, } from "@seamless-auth/core"; import { requestCookies } from "./internal/requestCookies.js"; @@ -38,6 +39,14 @@ export interface RequireAuthOptions { * 403 `enrollment_required`; turn it on only for routes that drive that enrollment. */ allowRecoverySession?: boolean; + /** + * `Origin` allowlist for a state-changing request on a cookie session that is + * `same-site` or carries no `Sec-Fetch-Site`, as on the auth router. Pass the + * same list. Set, even to `[]`, it is enforced; left out, such a request + * passes. A `cross-site` one or a literal `null` `Origin` is refused either + * way. + */ + allowedOrigins?: string[]; } /** @@ -71,6 +80,7 @@ export function requireAuth(opts: RequireAuthOptions) { audience, authServerIssuer, allowRecoverySession, + allowedOrigins, } = opts; // Eagerly, so a weak secret fails at setup rather than on the first request. @@ -104,8 +114,24 @@ export function requireAuth(opts: RequireAuthOptions) { req: FastifyRequest, reply: FastifyReply, ) { + const token = requestCookies(req)[cookieName]; + + // A cookie rides a forged request on its own; a bearer token never does. + if (token) { + const blocked = checkOrigin({ + method: req.method, + secFetchSite: firstHeader(req.headers["sec-fetch-site"]), + origin: firstHeader(req.headers.origin), + allowedOrigins, + }); + + if (blocked) { + return reply.status(blocked.status).send({ error: blocked.errorCode }); + } + } + const { user, rejection } = await authenticateRequest({ - token: requestCookies(req)[cookieName], + token, cookieSecret, authorization: req.headers.authorization, bearer, @@ -156,3 +182,7 @@ export function requireRole(requiredRoles: string | string[]) { } }; } + +function firstHeader(value: string | string[] | undefined): string | undefined { + return Array.isArray(value) ? value[0] : value; +} From 674d4141669f5728a7b37d79c4ed7007ba19a649 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 21:44:04 -0500 Subject: [PATCH 07/17] docs: describe the content-type gate and guard origin check The Fastify and Next.js option tables gain the 415 gate, its exemptions and the null-Origin rule, matching the Express README. The Express and Fastify guard sections describe the origin check and allowedOrigins. Refs #256 --- packages/express/README.md | 12 ++++++++++++ packages/fastify/README.md | 20 +++++++++++++++++++- packages/nextjs/README.md | 12 +++++++++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/packages/express/README.md b/packages/express/README.md index 3e7b08e..909bc64 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -362,6 +362,8 @@ and needs no configuration. preflight (`OPTIONS`) are unaffected. `POST /magic-link/verify/:token/confirm` is a state change and is guarded like any other, so serve the page that calls it from your own site. +- A literal `null` `Origin` (a sandboxed or opaque document) is refused + whatever `Sec-Fetch-Site` and `allowedOrigins` say. - A caller that sends neither `Origin` nor `Sec-Fetch-Site`, such as a server-to-server request, passes the origin check, but still needs the JSON content type or the transport header. @@ -633,9 +635,18 @@ app.get("/api/profile", guard, (req, res) => { audience?: string; // expected `aud` on a bearer token: the auth server's ISSUER authServerIssuer?: string; // expected `iss` on a bearer token (defaults to authServerUrl) allowRecoverySession?: boolean; // accept a recovery session (default false) + allowedOrigins?: string[]; // origin allowlist, as on createSeamlessAuthServer } ``` +On a cookie session the guard runs the same origin check as the auth router on a +state-changing request: a `cross-site` one or a literal `null` `Origin` is +refused with `403 { "error": "cross_site_request_blocked" }`, and with +`allowedOrigins` set (pass the router's list) a `same-site` one or one with no +`Sec-Fetch-Site` must come from a listed `Origin`. A bearer request is not +checked, since no browser attaches its token on its own. Your own routes need no +JSON content type. + A recovery session comes from account recovery or a recovery code, and its access token carries `recovery: true`. It can only enroll a new passkey or authenticator app, so the guard refuses it with `403 { "error": "enrollment_required" }` unless `allowRecoverySession` is set. Set it only on @@ -706,6 +717,7 @@ Compatibility rules: | --- | --- | --- | | 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"}` | +| A cookie request the origin check refuses (see `allowedOrigins`) | 403 | `{"error":"cross_site_request_blocked"}` | | 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 diff --git a/packages/fastify/README.md b/packages/fastify/README.md index fb7b944..385c95f 100644 --- a/packages/fastify/README.md +++ b/packages/fastify/README.md @@ -102,10 +102,18 @@ 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. +On a cookie session, `requireAuth` runs the same origin check as the plugin on a +state-changing request: a `cross-site` one or a literal `null` `Origin` is +refused, and with `allowedOrigins` set (pass the plugin's list) a `same-site` one +or one with no `Sec-Fetch-Site` must come from a listed `Origin`. A bearer +request is not checked, since no browser attaches its token on its own. Your own +routes need no JSON content type. + | 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"}` | +| A cookie request the origin check refuses (see `allowedOrigins`) | 403 | `{"error":"cross_site_request_blocked"}` | | 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 @@ -261,7 +269,7 @@ into the dashboard work: the upstream answers them with the SPA shell. | `cookieDomain` | none | Domain attribute for the auth cookies | | `cookieSecure` | `true` | Set `false` only for local HTTP development | | `cookieSameSite` | `none` when secure, else `lax` | SameSite policy | -| `allowedOrigins` | none | Origin allowlist for state-changing requests that are `same-site` or come from browsers without `Sec-Fetch-Site` | +| `allowedOrigins` | none | Origin allowlist for state-changing requests that are `same-site` or come from browsers without `Sec-Fetch-Site`. Set, even to `[]`, it is enforced. Whatever it says, a `cross-site` request or a literal `null` `Origin` is refused with `403 cross_site_request_blocked` | | `accessCookieName` | `seamless-access` | Session cookie name | | `registrationCookieName` | `seamless-ephemeral` | Registration cookie name | | `refreshCookieName` | `seamless-refresh` | Refresh cookie name | @@ -269,6 +277,16 @@ into the dashboard work: the upstream answers them with the SPA shell. | `messaging` | none | Adopter-supplied delivery transports and overrides | | `resolveClientIp` | none | Resolver for the end user's IP | +Every state-changing request to the auth routes (anything but `GET`, `HEAD` and +`OPTIONS`) must declare `Content-Type: application/json` or carry an +`x-seamless-auth-transport` header (any value), even with no body, or it is +refused with `415 {"error":"unsupported_media_type"}`. An HTML form or a +`no-cors` fetch can send neither, so this forces a CORS preflight. `POST +/refresh`, `DELETE /logout` and `DELETE /logout/all` are exempt for now, because +the browser client sends them bare; they still get the origin check, which runs +whatever `cookieSameSite` is. Guards on your own routes get the origin check +only. + ### Client IP The adapter forwards the end user's IP so the auth API can rate limit and audit diff --git a/packages/nextjs/README.md b/packages/nextjs/README.md index e367286..ed6f2b8 100644 --- a/packages/nextjs/README.md +++ b/packages/nextjs/README.md @@ -335,7 +335,7 @@ adapters, plus `basePath`. | `cookieDomain` | none | Domain attribute for the auth cookies | | `cookieSecure` | `true` | Set `false` only for local HTTP development | | `cookieSameSite` | `none` when secure, else `lax` | SameSite policy | -| `allowedOrigins` | none | Origin allowlist for state-changing requests that are `same-site` or come from browsers without `Sec-Fetch-Site` | +| `allowedOrigins` | none | Origin allowlist for state-changing requests that are `same-site` or come from browsers without `Sec-Fetch-Site`. Set, even to `[]`, it is enforced. Whatever it says, a `cross-site` request or a literal `null` `Origin` is refused with `403 cross_site_request_blocked` | | `accessCookieName` | `seamless-access` | Session cookie name | | `registrationCookieName` | `seamless-ephemeral` | Registration cookie name | | `refreshCookieName` | `seamless-refresh` | Refresh cookie name | @@ -344,6 +344,16 @@ adapters, plus `basePath`. | `resolveClientIp` | none | Picks the end user's IP to forward | | `trustedProxies` | none | Addresses or CIDR ranges of your proxies; the client IP is read from `X-Forwarded-For` past them | +Every state-changing request to the auth routes (anything but `GET`, `HEAD` and +`OPTIONS`) must declare `Content-Type: application/json` or carry an +`x-seamless-auth-transport` header (any value), even with no body, or it is +refused with `415 {"error":"unsupported_media_type"}`. An HTML form or a +`no-cors` fetch can send neither, so this forces a CORS preflight. `POST +/refresh`, `DELETE /logout` and `DELETE /logout/all` are exempt for now, because +the browser client sends them bare; they still get the origin check, which runs +whatever `cookieSameSite` is. Guards on your own routes get the origin check +only. + The session helpers take `authServerUrl`, `cookieSecret`, and optionally `serviceSecret`, `jwksKid`, the cookie names, `userAgent`, `headers`, `trustedProxies` and `resolveClientIp`. They also accept From 30f1301f0b7bf4fc1bdbec4decbac030a58a1269 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:07:37 -0500 Subject: [PATCH 08/17] test(express): reproduce forwarding a client IP under a trust-everything proxy setting Refs #257 --- .../express/tests/proxyServiceToken.test.js | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/express/tests/proxyServiceToken.test.js b/packages/express/tests/proxyServiceToken.test.js index 6b545bd..9cca243 100644 --- a/packages/express/tests/proxyServiceToken.test.js +++ b/packages/express/tests/proxyServiceToken.test.js @@ -147,6 +147,38 @@ describe("proxied service token", () => { warn.mockRestore(); }); + // Each of these trusts every address as a proxy, so req.ip is the leftmost + // X-Forwarded-For entry the client wrote (#257). Express refuses a /0 range, so + // the whole IPv4 space is written as its two halves. + it.each([ + ["two subnets covering every IPv4 address", "0.0.0.0/1, 128.0.0.0/1"], + ["a function that trusts every address", () => true], + ])("drops the client IP when trust proxy is %s", async (_label, setting) => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + const app = createApp((a) => a.set("trust proxy", setting)); + + await request(app) + .delete("/auth/admin/users") + .set("Cookie", createAccessCookie()) + .set("X-Forwarded-For", "1.2.3.4") + .send({ userId: "user-1" }); + + expect(lastHeaders()).not.toHaveProperty("x-seamless-client-ip"); + warn.mockRestore(); + }); + + it("forwards the client IP when trust proxy names the proxy address", async () => { + const app = createApp((a) => a.set("trust proxy", "loopback")); + + await request(app) + .delete("/auth/admin/users") + .set("Cookie", createAccessCookie()) + .set("X-Forwarded-For", "203.0.113.44") + .send({ userId: "user-1" }); + + expect(lastHeaders()["x-seamless-client-ip"]).toBe("203.0.113.44"); + }); + // The auth API records the user agent on every audit row and folds it into a // device class. Without this header it only ever sees this adapter's own. it("forwards the browser user agent", async () => { From a63c4f655e261f3015f083cdb9777acb0b574395 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:08:59 -0500 Subject: [PATCH 09/17] fix(express): detect any trust-everything proxy setting before forwarding the client IP Probe the compiled trust proxy function with documentation addresses, as the Fastify adapter does, so subnets covering every address or a function that always trusts no longer let a caller choose x-seamless-client-ip. Refs #257 --- .changeset/express-trust-everything-proxy.md | 5 +++ packages/express/README.md | 8 ++-- .../src/internal/buildForwardedClientIp.ts | 43 ++++++++++++++++--- .../express/tests/proxyServiceToken.test.js | 12 ++++++ 4 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 .changeset/express-trust-everything-proxy.md diff --git a/.changeset/express-trust-everything-proxy.md b/.changeset/express-trust-everything-proxy.md new file mode 100644 index 0000000..81912c8 --- /dev/null +++ b/.changeset/express-trust-everything-proxy.md @@ -0,0 +1,5 @@ +--- +"@seamless-auth/express": patch +--- + +Stop forwarding a client-chosen IP when Express `trust proxy` trusts every address in a form other than `true` (#257). The adapter used to refuse only `trust proxy === true`, so a setting such as `"0.0.0.0/1, 128.0.0.0/1"` or a function that always returns `true` let a caller pick the `x-seamless-client-ip` the auth API uses for per-IP rate limits and audit records. The adapter now asks the compiled trust function about documentation addresses, as the Fastify adapter does, and in that case logs a warning and forwards no client IP. A hop count or a named proxy address or subnet forwards as before. diff --git a/packages/express/README.md b/packages/express/README.md index 909bc64..eece0b0 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -287,9 +287,11 @@ app.set("trust proxy", 1); app.set("trust proxy", "10.0.0.0/8"); ``` -With `trust proxy` set to `true`, Express takes `req.ip` from the leftmost `X-Forwarded-For` entry, -which any client can set. The adapter detects that case, logs a startup warning, and forwards no -client IP at all rather than forwarding an attacker-chosen one. +With `trust proxy` set to `true`, or to anything else that trusts every address (subnets that +together cover the whole address space, or a function that always returns `true`), Express takes +`req.ip` from the leftmost `X-Forwarded-For` entry, which any client can set. The adapter detects +that case, logs a warning, and forwards no client IP at all rather than forwarding an +attacker-chosen one. If your topology cannot be expressed as a hop count or subnet, supply `resolveClientIp` and derive the address yourself, for example from a load-balancer header you know is stripped at the edge: diff --git a/packages/express/src/internal/buildForwardedClientIp.ts b/packages/express/src/internal/buildForwardedClientIp.ts index b027fbe..fe30fcd 100644 --- a/packages/express/src/internal/buildForwardedClientIp.ts +++ b/packages/express/src/internal/buildForwardedClientIp.ts @@ -5,19 +5,50 @@ import { Request } from "express"; export type ClientIpResolver = (req: Request) => string | undefined; +// Documentation addresses (RFC 5737), which no real proxy has. A trust policy that +// accepts them as proxies accepts any address, so req.ip is whatever the client put +// first in X-Forwarded-For. The hop index is far past any real proxy chain, so a hop count, which trusts by +// position whatever the address, is not mistaken for trusting every address. +const PROBES: ReadonlyArray = [ + ["203.0.113.9", 0], + ["198.51.100.8", 1], + ["198.51.100.9", 99], +]; + let warnedBlanketTrustProxy = false; -// With `trust proxy` set to blanket true, Express derives req.ip from the leftmost -// X-Forwarded-For entry, which any client can set. Forwarding that upstream would let -// a caller pick its own rate-limit and audit identity, so drop it instead. +type TrustProxyFn = (addr: string, hop: number) => boolean; + +/** + * Whether the application's `trust proxy` trusts any address as a proxy. + * + * Asks Express's compiled trust function whether it trusts documentation addresses + * as the peer and as hops behind it, which would make req.ip a client-written + * X-Forwarded-For entry. A hop count or a named proxy address or subnet does not, so + * those still forward. + */ +function trustsAnyAddress(req: Request): boolean { + const setting = req.app?.get?.("trust proxy"); + if (setting === true) return true; + + const trust = req.app?.get?.("trust proxy fn") as TrustProxyFn | undefined; + if (typeof trust !== "function") return false; + + try { + return PROBES.every(([addr, hop]) => trust(addr, hop) === true); + } catch { + return false; + } +} + function derivedFromTrustedHop(req: Request): string | undefined { - if (req.app?.get?.("trust proxy") === true) { + if (trustsAnyAddress(req)) { if (!warnedBlanketTrustProxy) { warnedBlanketTrustProxy = true; getSeamlessLogger().warn( - "[seamless-auth] Express 'trust proxy' is set to true, so req.ip is client-controlled. " + + "[seamless-auth] Express 'trust proxy' trusts every address (for example `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.", + "hop count or the proxy's address or subnet, or pass resolveClientIp to createSeamlessAuthServer.", ); } diff --git a/packages/express/tests/proxyServiceToken.test.js b/packages/express/tests/proxyServiceToken.test.js index 9cca243..89ba551 100644 --- a/packages/express/tests/proxyServiceToken.test.js +++ b/packages/express/tests/proxyServiceToken.test.js @@ -167,6 +167,18 @@ describe("proxied service token", () => { warn.mockRestore(); }); + it("forwards the client IP behind two trusted hops", async () => { + const app = createApp((a) => a.set("trust proxy", 2)); + + await request(app) + .delete("/auth/admin/users") + .set("Cookie", createAccessCookie()) + .set("X-Forwarded-For", "1.2.3.4, 203.0.113.44, 10.0.0.2") + .send({ userId: "user-1" }); + + expect(lastHeaders()["x-seamless-client-ip"]).toBe("203.0.113.44"); + }); + it("forwards the client IP when trust proxy names the proxy address", async () => { const app = createApp((a) => a.set("trust proxy", "loopback")); From 0ccda2dff1383847b50a21548ac46f477c7c466d Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:09:32 -0500 Subject: [PATCH 10/17] test(core): keep keys served at the host root from verifying prefixed auth server tokens The JWKS prefix was already kept by #267, so this does not reproduce on the current branch. Against the pre-#267 URL resolution it fails: a token signed with the host root key verifies. Refs #259 --- packages/core/tests/jwks.test.js | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/core/tests/jwks.test.js b/packages/core/tests/jwks.test.js index 6baf0ed..b63837d 100644 --- a/packages/core/tests/jwks.test.js +++ b/packages/core/tests/jwks.test.js @@ -36,4 +36,48 @@ describe("auth server JWKS", () => { expect(payload.sub).toBe("user-123"); }); + + // The security half of #259: keys served at the root of a shared host must not + // verify tokens for an auth server mounted under a prefix. + it("does not trust keys served at the host root", async () => { + const { verifyWithAuthServerJwks } = await import("../dist/jwks.js"); + const apiKeys = await generateKeyPair("RS256"); + const rootKeys = await generateKeyPair("RS256"); + const apiJwk = { + ...(await exportJWK(apiKeys.publicKey)), + alg: "RS256", + kid: "k1", + }; + const rootJwk = { + ...(await exportJWK(rootKeys.publicKey)), + alg: "RS256", + kid: "k1", + }; + const forged = await new SignJWT({ sub: "attacker" }) + .setProtectedHeader({ alg: "RS256", kid: "k1" }) + .setExpirationTime("5m") + .sign(rootKeys.privateKey); + + global.fetch = jest.fn(async (url) => { + const keys = { + "https://shared.example.com/.well-known/jwks.json": [rootJwk], + "https://shared.example.com/tenant-api/.well-known/jwks.json": [apiJwk], + }[url.toString()]; + return new Response(JSON.stringify({ keys: keys ?? [] }), { + status: keys ? 200 : 404, + headers: { "Content-Type": "application/json" }, + }); + }); + + await expect( + verifyWithAuthServerJwks( + forged, + "https://shared.example.com/tenant-api", + {}, + ), + ).rejects.toThrow(); + expect( + global.fetch.mock.calls.map(([url]) => url.toString()), + ).not.toContain("https://shared.example.com/.well-known/jwks.json"); + }); }); From cdab867da29d1c0ac56a0c707a8bd55e3806130e Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:10:32 -0500 Subject: [PATCH 11/17] test(core): reproduce client credentials tokens that look like users Refs #260 --- packages/core/tests/resourceServer.test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/core/tests/resourceServer.test.js b/packages/core/tests/resourceServer.test.js index e5f74d1..93191a5 100644 --- a/packages/core/tests/resourceServer.test.js +++ b/packages/core/tests/resourceServer.test.js @@ -91,6 +91,19 @@ describe("shared verify-only vectors", () => { }); describe("createResourceServer", () => { + // A client credentials token's `sub` is the client id, so without a marker an + // adopter reading `user.id` takes a machine for a user (#260). + it("marks a client credentials token as a client, and a user token as not", async () => { + const server = vectorServer(); + + const client = await server.verify(vectors.tokens.client_credentials); + const user = await server.verify(vectors.tokens.user_passkey); + + expect(client.id).toBe(client.clientId); + expect(client.isClient).toBe(true); + expect(user.isClient).toBe(false); + }); + it("refuses an empty audience or a non-URL issuer at setup", () => { expect(() => createResourceServer({ issuer: vectors.issuer, audience: "" }), From 1fc3b9ca90f1741e202db4ced9d52112377eb516 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:11:37 -0500 Subject: [PATCH 12/17] fix(core): mark client credentials tokens on the resource server user A verified user now carries isClient, true when sub equals client_id, so adopters can tell a machine acting for itself from a user before reading id as a user id. Refs #260 --- .changeset/resource-server-is-client.md | 8 ++++++++ packages/core/src/resourceServer.ts | 6 ++++++ packages/express/README.md | 5 +++-- packages/fastify/README.md | 5 +++-- packages/nextjs/README.md | 5 +++-- 5 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 .changeset/resource-server-is-client.md diff --git a/.changeset/resource-server-is-client.md b/.changeset/resource-server-is-client.md new file mode 100644 index 0000000..8b85abb --- /dev/null +++ b/.changeset/resource-server-is-client.md @@ -0,0 +1,8 @@ +--- +"@seamless-auth/core": patch +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Mark client credentials tokens on the resource server user (#260). A user verified by `createResourceServer` now carries `isClient`, `true` when `sub` equals `client_id` (a machine acting for itself, whose `id` is a client id rather than a user id) and `false` otherwise. Code that builds a `ResourceServerUser` by hand, such as a test double, needs the new field. diff --git a/packages/core/src/resourceServer.ts b/packages/core/src/resourceServer.ts index f646891..94c39d0 100644 --- a/packages/core/src/resourceServer.ts +++ b/packages/core/src/resourceServer.ts @@ -19,6 +19,11 @@ import { extractBearerToken } from "./verifyAccessToken.js"; */ export interface ResourceServerClaims { clientId: string; + /** + * A client credentials token, where `sub` is the client id: a machine acting + * for itself, with no user behind it. `id` is then a client id, not a user id. + */ + isClient: boolean; scopes: string[]; sid?: string; orgId?: string; @@ -243,6 +248,7 @@ function toUser(claims: JWTPayload, token: string): ResourceServerUser | null { return { id: sub, clientId, + isClient: sub === clientId, scopes: scope ? scope.split(" ").filter(Boolean) : [], roles: roles ?? [], amr: amr ?? [], diff --git a/packages/express/README.md b/packages/express/README.md index eece0b0..d8b58e3 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -803,8 +803,9 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai `max_age` to send the user back through the provider with. The verified user has `id` (`sub`), `clientId`, `scopes`, `roles`, `sid`, `orgId`, `acr`, `amr` -and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, and fails -role and step-up checks. +and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, with +`isClient` set to `true` and the client id in `id`, and fails role and step-up checks. Check +`isClient` before treating `id` as a user id. --- diff --git a/packages/fastify/README.md b/packages/fastify/README.md index 385c95f..c5f92fc 100644 --- a/packages/fastify/README.md +++ b/packages/fastify/README.md @@ -189,8 +189,9 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai `max_age` to send the user back through the provider with. The verified user has `id` (`sub`), `clientId`, `scopes`, `roles`, `sid`, `orgId`, `acr`, `amr` -and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, and fails -role and step-up checks. +and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, with +`isClient` set to `true` and the client id in `id`, and fails role and step-up checks. Check +`isClient` before treating `id` as a user id. ## Adopter-supplied message delivery diff --git a/packages/nextjs/README.md b/packages/nextjs/README.md index ed6f2b8..02248c2 100644 --- a/packages/nextjs/README.md +++ b/packages/nextjs/README.md @@ -315,8 +315,9 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai `max_age` to send the user back through the provider with. The verified user has `id` (`sub`), `clientId`, `scopes`, `roles`, `sid`, `orgId`, `acr`, `amr` -and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, and fails -role and step-up checks. +and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, with +`isClient` set to `true` and the client id in `id`, and fails role and step-up checks. Check +`isClient` before treating `id` as a user id. ## Options From 29ce8fc3609c7a17de84bf1ed5fd19e62fd011ca Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:21:27 -0500 Subject: [PATCH 13/17] test(core): reproduce resource server guards that cannot require a user Adds shared vectors for isClient on verified tokens and for an auth guard that refuses client credentials tokens with 403 user_required. Both are additive: is_client sits beside each verify entry's user, and the new requests live in their own user_required_requests list, so the Go, Python and Rust copies keep passing until those adapters take them up. Refs #260 --- .../tests/fixtures/verify-only/generate.mjs | 69 ++++++++- .../tests/fixtures/verify-only/vectors.json | 137 ++++++++++++++++-- packages/core/tests/resourceServer.test.js | 30 +++- .../tests/resourceServer.parity.test.js | 12 +- .../tests/resourceServer.parity.test.js | 12 +- .../tests/resourceServer.parity.test.js | 17 ++- 6 files changed, 252 insertions(+), 25 deletions(-) diff --git a/packages/core/tests/fixtures/verify-only/generate.mjs b/packages/core/tests/fixtures/verify-only/generate.mjs index 8621ac5..d712946 100644 --- a/packages/core/tests/fixtures/verify-only/generate.mjs +++ b/packages/core/tests/fixtures/verify-only/generate.mjs @@ -2,6 +2,15 @@ // adapter's verify-only mode is tested against (Express, Fastify and Next.js // here, and the Go, Python and Rust adapters, which copy vectors.json). // +// A consumer reads only the keys it knows, so additions stay out of the way of +// an adapter that has not implemented them yet: +// +// - `verify[].is_client` sits beside `user`, not in it, because consumers +// compare `user` whole. +// - `user_required_requests` holds requests for an auth guard set to refuse +// client credentials tokens (`auth.allow_clients` false). It is its own list +// because an adapter without the option would answer them as `requests`. +// // RS256 signatures are deterministic, so rerunning with the committed keys // reproduces the file byte for byte. The keys are test-only. // @@ -235,7 +244,12 @@ const accepted = { const verify = Object.keys(tokens).map((name) => name in accepted - ? { name, valid: true, user: user(accepted[name]) } + ? { + name, + valid: true, + user: user(accepted[name]), + is_client: accepted[name].sub === accepted[name].client_id, + } : { name, valid: false }, ); @@ -263,6 +277,11 @@ const roleDenied = { body: { error: "insufficient_role" }, www_authenticate: null, }; +const userRequired = { + status: 403, + body: { error: "user_required" }, + www_authenticate: null, +}; function stepUpDenied(description, guard) { const body = { @@ -471,6 +490,53 @@ const requests = [ }, ]; +const usersOnly = { allow_clients: false }; + +const userRequiredRequests = [ + { + name: "user_required_client_credentials", + authorization: bearer("client_credentials"), + auth: usersOnly, + guard: null, + expect: userRequired, + }, + { + name: "user_required_user", + authorization: bearer("user_otp"), + auth: usersOnly, + guard: null, + expect: ok, + }, + { + name: "user_required_missing_token", + authorization: null, + auth: usersOnly, + guard: null, + expect: missing, + }, + { + name: "user_required_invalid_token_first", + authorization: bearer("expired"), + auth: usersOnly, + guard: null, + expect: invalid, + }, + { + name: "user_required_before_scope", + authorization: bearer("client_credentials"), + auth: usersOnly, + guard: { kind: "scope", scopes: ["reports:read"] }, + expect: userRequired, + }, + { + name: "user_required_user_with_scope", + authorization: bearer("user_passkey"), + auth: usersOnly, + guard: { kind: "scope", scopes: ["orders:write"] }, + expect: ok, + }, +]; + const vectors = { version: 1, issuer: ISSUER, @@ -481,6 +547,7 @@ const vectors = { tokens, verify, requests, + user_required_requests: userRequiredRequests, }; writeFileSync( diff --git a/packages/core/tests/fixtures/verify-only/vectors.json b/packages/core/tests/fixtures/verify-only/vectors.json index 8c0a71c..6ba46f4 100644 --- a/packages/core/tests/fixtures/verify-only/vectors.json +++ b/packages/core/tests/fixtures/verify-only/vectors.json @@ -115,7 +115,8 @@ "mfa" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "user_otp", @@ -136,7 +137,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "user_stale_passkey", @@ -159,7 +161,8 @@ "mfa" ], "auth_time": 1798761000 - } + }, + "is_client": false }, { "name": "user_no_auth_time", @@ -182,7 +185,8 @@ "mfa" ], "auth_time": null - } + }, + "is_client": false }, { "name": "client_credentials", @@ -199,7 +203,8 @@ "acr": null, "amr": [], "auth_time": null - } + }, + "is_client": true }, { "name": "aud_array", @@ -220,7 +225,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "typ_application", @@ -241,7 +247,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "typ_uppercase", @@ -262,7 +269,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "exp_within_skew", @@ -283,7 +291,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "nbf_within_skew", @@ -304,7 +313,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "rotated_key", @@ -325,7 +335,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "org_id_null", @@ -346,7 +357,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "unknown_claims", @@ -367,7 +379,8 @@ "otp" ], "auth_time": 1798761480 - } + }, + "is_client": false }, { "name": "wrong_aud", @@ -1007,5 +1020,103 @@ "www_authenticate": "Bearer error=\"insufficient_user_authentication\", error_description=\"More recent authentication is required\", max_age=\"0\"" } } + ], + "user_required_requests": [ + { + "name": "user_required_client_credentials", + "authorization": "Bearer eyJhbGciOiJSUzI1NiIsImtpZCI6InZvLTEiLCJ0eXAiOiJhdCtqd3QifQ.eyJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUudGVzdCIsImF1ZCI6Imh0dHBzOi8vYXBpLmV4YW1wbGUudGVzdCIsImV4cCI6MTc5ODc2NTIwMCwiaWF0IjoxNzk4NzYxNDgwLCJqdGkiOiJqdGktMSIsInN1YiI6InN2Yy1yZXBvcnRzIiwiY2xpZW50X2lkIjoic3ZjLXJlcG9ydHMiLCJzY29wZSI6InJlcG9ydHM6cmVhZCJ9.PaT0ELDOlyKU5q_6CMxkjZYyz2jYhB1djlGIB0ebLcDBZO2gJDna2AxJTa4QKmREIcJOZAMGsyzAp8ojLNeGdJXuClbGNxHaStrPyLdu5sYyypWqTdy3sHBXaf04YjE6CmaREcNt6TymXF6n52BdgxCjVDfsmDJ30JCCQ-RCCf1HfLZHVnf8Y_I19R4lzJ1SuDVENi6x_LVIGxg0FGF4WkVq0_6lXItF01nAGb1Rg-GrfaYBBf-SEWiYedUHd7tUXH3btZeZ20g4XlBqifHonax9MBJpOeyOUG_NNDHYoSBj7tV3Qk5Vwb9exuSrJMroW33mjvqSakRc9PD67X5tqw", + "auth": { + "allow_clients": false + }, + "guard": null, + "expect": { + "status": 403, + "body": { + "error": "user_required" + }, + "www_authenticate": null + } + }, + { + "name": "user_required_user", + "authorization": "Bearer eyJhbGciOiJSUzI1NiIsImtpZCI6InZvLTEiLCJ0eXAiOiJhdCtqd3QifQ.eyJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUudGVzdCIsImF1ZCI6Imh0dHBzOi8vYXBpLmV4YW1wbGUudGVzdCIsImV4cCI6MTc5ODc2NTIwMCwiaWF0IjoxNzk4NzYxNDgwLCJqdGkiOiJqdGktMSIsInN1YiI6InVzZXItNDU2IiwiY2xpZW50X2lkIjoiYXBwLXdlYiIsInNjb3BlIjoib3JkZXJzOnJlYWQiLCJzaWQiOiJzZXNzLTIiLCJyb2xlcyI6WyJtZW1iZXIiXSwiYW1yIjpbIm90cCJdLCJhdXRoX3RpbWUiOjE3OTg3NjE0ODB9.Q9__hloZFJqysviQX_klUPKyZjdLSWcV0YQyFMMRzneBiHjEKMEuAVH3MLNMOZV1kp3SY_BrPb9ihRQULVYNIJu9og-kv0G0Wcd7at6RfNwfr41mY5BXA4r35FUgMOwdHvuSG0MMWUWeStzKCUgxlz7MV5S5Ye9ahl8G0Gb0dQHKV7x0W2t3a098oRseQvu2PjA67ad5W7HBSIArRSp0Z5mdq-adYYmcTIf-kZtEyFsp25gNUqoHrLZVoXyIfGwdMhWF9BVEtVAa47QiK13L5kulko4tJbTSA5gEDqfyer4Fcp8KXAcEr92VASYMqm-y50rZHYoOV7NCPaKjL3AhUQ", + "auth": { + "allow_clients": false + }, + "guard": null, + "expect": { + "status": 200, + "body": null, + "www_authenticate": null + } + }, + { + "name": "user_required_missing_token", + "authorization": null, + "auth": { + "allow_clients": false + }, + "guard": null, + "expect": { + "status": 401, + "body": { + "error": "missing_token" + }, + "www_authenticate": "Bearer" + } + }, + { + "name": "user_required_invalid_token_first", + "authorization": "Bearer eyJhbGciOiJSUzI1NiIsImtpZCI6InZvLTEiLCJ0eXAiOiJhdCtqd3QifQ.eyJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUudGVzdCIsImF1ZCI6Imh0dHBzOi8vYXBpLmV4YW1wbGUudGVzdCIsImV4cCI6MTc5ODc2MTU0MCwiaWF0IjoxNzk4NzYxNDgwLCJqdGkiOiJqdGktMSIsInN1YiI6InVzZXItNDU2IiwiY2xpZW50X2lkIjoiYXBwLXdlYiIsInNjb3BlIjoib3JkZXJzOnJlYWQiLCJzaWQiOiJzZXNzLTIiLCJyb2xlcyI6WyJtZW1iZXIiXSwiYW1yIjpbIm90cCJdLCJhdXRoX3RpbWUiOjE3OTg3NjE0ODB9.EHGExwM8o7uCLrFolhwJ4P9mU4V6aW_wR2O3Ezt75jMjhkzBqGivaX5L5xHQOsMdbU_yg_54OxGjBEDoQ4RLz3nqfBf-VhjgJNyGjzfQaJ-KlMUFN0ZtM5zf_8-vRBy1zfTJF6ebzDs8ILxMzJTcrQq5U0DOp0xXO20rzYR84Us46Gs4sprpxHlFJrnEeoQl6LoHrPiMl19jarNHAFmiy-K4TooZ8Zhl0Pb6ML29FJucLc0dL8pBZ8C5PeWVBQ6ZO8jnvfR-_v7gxytMaH9W-8QbF7dzDHz3XSmZUhAEY1vpP676j6idsccG3TY_Q9KKaw47Px3bV7Hv3ubzrwXaRQ", + "auth": { + "allow_clients": false + }, + "guard": null, + "expect": { + "status": 401, + "body": { + "error": "invalid_token" + }, + "www_authenticate": "Bearer error=\"invalid_token\"" + } + }, + { + "name": "user_required_before_scope", + "authorization": "Bearer eyJhbGciOiJSUzI1NiIsImtpZCI6InZvLTEiLCJ0eXAiOiJhdCtqd3QifQ.eyJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUudGVzdCIsImF1ZCI6Imh0dHBzOi8vYXBpLmV4YW1wbGUudGVzdCIsImV4cCI6MTc5ODc2NTIwMCwiaWF0IjoxNzk4NzYxNDgwLCJqdGkiOiJqdGktMSIsInN1YiI6InN2Yy1yZXBvcnRzIiwiY2xpZW50X2lkIjoic3ZjLXJlcG9ydHMiLCJzY29wZSI6InJlcG9ydHM6cmVhZCJ9.PaT0ELDOlyKU5q_6CMxkjZYyz2jYhB1djlGIB0ebLcDBZO2gJDna2AxJTa4QKmREIcJOZAMGsyzAp8ojLNeGdJXuClbGNxHaStrPyLdu5sYyypWqTdy3sHBXaf04YjE6CmaREcNt6TymXF6n52BdgxCjVDfsmDJ30JCCQ-RCCf1HfLZHVnf8Y_I19R4lzJ1SuDVENi6x_LVIGxg0FGF4WkVq0_6lXItF01nAGb1Rg-GrfaYBBf-SEWiYedUHd7tUXH3btZeZ20g4XlBqifHonax9MBJpOeyOUG_NNDHYoSBj7tV3Qk5Vwb9exuSrJMroW33mjvqSakRc9PD67X5tqw", + "auth": { + "allow_clients": false + }, + "guard": { + "kind": "scope", + "scopes": [ + "reports:read" + ] + }, + "expect": { + "status": 403, + "body": { + "error": "user_required" + }, + "www_authenticate": null + } + }, + { + "name": "user_required_user_with_scope", + "authorization": "Bearer eyJhbGciOiJSUzI1NiIsImtpZCI6InZvLTEiLCJ0eXAiOiJhdCtqd3QifQ.eyJpc3MiOiJodHRwczovL2xvZ2luLmV4YW1wbGUudGVzdCIsImF1ZCI6Imh0dHBzOi8vYXBpLmV4YW1wbGUudGVzdCIsImV4cCI6MTc5ODc2NTIwMCwiaWF0IjoxNzk4NzYxNDgwLCJqdGkiOiJqdGktMSIsInN1YiI6InVzZXItMTIzIiwiY2xpZW50X2lkIjoiYXBwLXdlYiIsInNjb3BlIjoib3BlbmlkIG9yZGVyczpyZWFkIG9yZGVyczp3cml0ZSIsInNpZCI6InNlc3MtMSIsIm9yZ19pZCI6Im9yZy05Iiwicm9sZXMiOlsib3JkZXJzOndyaXRlIiwibWVtYmVyIl0sImFjciI6InBocmgiLCJhbXIiOlsiaHdrIiwidXNlciIsIm1mYSJdLCJhdXRoX3RpbWUiOjE3OTg3NjE0ODB9.KfHFZpgrLcSwnOn81Wu49u5ihVB8BhLjf2DnUCtN4BOsOkUOQRtYV31DCevRzcN_FBFMpxt-b5WXOKAGml6nouXOAntr26DKBkeHuFU9LsVmZHOtxtNGy3VpuLK2qhVizzrD-Ol8MffP0fuPhV827itGV9Aea9XD6Mdi1sM54wYgvWjvpd0BChdTHPFQY4eq6GpsmTUSOnee-k0WiwbgKnML0kFlejQ_d8Qo4M1De10zV6d87s12h5MP8VTm4RJWgVmEzIhH_Och5sx77GIS-0IrLV6aKV8sil3HbKvKzKsWKwTCpIvBnyY2hEC98q5vv07074-O5r-Q4aMGPWJahA", + "auth": { + "allow_clients": false + }, + "guard": { + "kind": "scope", + "scopes": [ + "orders:write" + ] + }, + "expect": { + "status": 200, + "body": null, + "www_authenticate": null + } + } ] } diff --git a/packages/core/tests/resourceServer.test.js b/packages/core/tests/resourceServer.test.js index 93191a5..81eb488 100644 --- a/packages/core/tests/resourceServer.test.js +++ b/packages/core/tests/resourceServer.test.js @@ -58,6 +58,7 @@ describe("shared verify-only vectors", () => { if (entry.valid) { expect(user).not.toBeNull(); expect(asVectorUser(user)).toEqual(entry.user); + expect(user.isClient).toBe(entry.is_client); expect(user.token).toBe(vectors.tokens[entry.name]); } else { expect(user).toBeNull(); @@ -65,12 +66,18 @@ describe("shared verify-only vectors", () => { }, ); - it.each(vectors.requests.map((entry) => [entry.name, entry]))( + it.each( + [...vectors.requests, ...vectors.user_required_requests].map((entry) => [ + entry.name, + entry, + ]), + )( "request %s", async (_, entry) => { const check = guardFor(server, entry.guard); const { user, rejection: authRejection } = await server.authenticate( entry.authorization ?? undefined, + entry.auth && { allowClients: entry.auth.allow_clients }, ); const rejection = authRejection ?? check(user); @@ -104,6 +111,27 @@ describe("createResourceServer", () => { expect(user.isClient).toBe(false); }); + it("lets a client credentials token through authenticate unless told not to", async () => { + const server = vectorServer(); + const authorization = `Bearer ${vectors.tokens.client_credentials}`; + + const byDefault = await server.authenticate(authorization); + const allowed = await server.authenticate(authorization, { + allowClients: true, + }); + const refused = await server.authenticate(authorization, { + allowClients: false, + }); + + expect(byDefault.user.isClient).toBe(true); + expect(allowed.user.isClient).toBe(true); + expect(refused.user).toBeUndefined(); + expect(refused.rejection).toEqual({ + status: 403, + errorCode: "user_required", + }); + }); + it("refuses an empty audience or a non-URL issuer at setup", () => { expect(() => createResourceServer({ issuer: vectors.issuer, audience: "" }), diff --git a/packages/express/tests/resourceServer.parity.test.js b/packages/express/tests/resourceServer.parity.test.js index f36827b..08f9308 100644 --- a/packages/express/tests/resourceServer.parity.test.js +++ b/packages/express/tests/resourceServer.parity.test.js @@ -31,16 +31,22 @@ function guardFor(guard) { }); } +// `user_required_requests` run with the auth guard refusing client tokens. +const entries = [...vectors.requests, ...vectors.user_required_requests]; + const app = express(); -vectors.requests.forEach((entry, index) => { +entries.forEach((entry, index) => { const guards = entry.guard ? [guardFor(entry.guard)] : []; - app.get(`/vector/${index}`, api.requireAuth(), ...guards, (req, res) => { + const auth = entry.auth + ? api.requireAuth({ allowClients: entry.auth.allow_clients }) + : api.requireAuth(); + app.get(`/vector/${index}`, auth, ...guards, (req, res) => { res.json({ sub: req.user.id, clientId: req.user.clientId }); }); }); describe("Express resource server against the shared vectors", () => { - it.each(vectors.requests.map((entry, index) => [entry.name, entry, index]))( + it.each(entries.map((entry, index) => [entry.name, entry, index]))( "%s", async (_, entry, index) => { let call = request(app).get(`/vector/${index}`); diff --git a/packages/fastify/tests/resourceServer.parity.test.js b/packages/fastify/tests/resourceServer.parity.test.js index 37b6d4b..01d1128 100644 --- a/packages/fastify/tests/resourceServer.parity.test.js +++ b/packages/fastify/tests/resourceServer.parity.test.js @@ -30,12 +30,18 @@ function guardFor(guard) { }); } +// `user_required_requests` run with the auth guard refusing client tokens. +const entries = [...vectors.requests, ...vectors.user_required_requests]; + const app = Fastify(); -vectors.requests.forEach((entry, index) => { +entries.forEach((entry, index) => { const guards = entry.guard ? [guardFor(entry.guard)] : []; + const auth = entry.auth + ? api.requireAuth({ allowClients: entry.auth.allow_clients }) + : api.requireAuth(); app.get( `/vector/${index}`, - { preHandler: [api.requireAuth(), ...guards] }, + { preHandler: [auth, ...guards] }, async (req) => ({ sub: req.user.id, clientId: req.user.clientId }), ); }); @@ -43,7 +49,7 @@ vectors.requests.forEach((entry, index) => { afterAll(() => app.close()); describe("Fastify resource server against the shared vectors", () => { - it.each(vectors.requests.map((entry, index) => [entry.name, entry, index]))( + it.each(entries.map((entry, index) => [entry.name, entry, index]))( "%s", async (_, entry, index) => { const res = await app.inject({ diff --git a/packages/nextjs/tests/resourceServer.parity.test.js b/packages/nextjs/tests/resourceServer.parity.test.js index 9ac3cba..8a4658b 100644 --- a/packages/nextjs/tests/resourceServer.parity.test.js +++ b/packages/nextjs/tests/resourceServer.parity.test.js @@ -19,8 +19,8 @@ const api = createResourceServer({ currentDate: () => new Date(vectors.now * 1000), }); -function requirementsFor(guard) { - if (!guard) return undefined; +function guardRequirements(guard) { + if (!guard) return {}; if (guard.kind === "scope") return { scopes: guard.scopes }; if (guard.kind === "role") return { roles: guard.roles }; return { @@ -32,6 +32,15 @@ function requirementsFor(guard) { }; } +function requirementsFor(entry) { + const requirements = guardRequirements(entry.guard); + if (entry.auth) requirements.allowClients = entry.auth.allow_clients; + return Object.keys(requirements).length ? requirements : undefined; +} + +// `user_required_requests` run with the auth check refusing client tokens. +const entries = [...vectors.requests, ...vectors.user_required_requests]; + function get(authorization) { return new Request("https://api.example.test/orders", { headers: authorization === null ? {} : { authorization }, @@ -39,12 +48,12 @@ function get(authorization) { } describe("Next.js resource server against the shared vectors", () => { - it.each(vectors.requests.map((entry) => [entry.name, entry]))( + it.each(entries.map((entry) => [entry.name, entry]))( "%s", async (_, entry) => { const handler = api.protect( (_request, _context, user) => Response.json({ sub: user.id }), - requirementsFor(entry.guard), + requirementsFor(entry), ); const res = await handler(get(entry.authorization), {}); From 8aa4bbecd10589cce42a668ad4d4eaee2ce46a69 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 2 Nov 2026 22:22:37 -0500 Subject: [PATCH 14/17] feat: let verify-only resource servers require a user requireAuth from createResourceServer (and the Next.js requirements, and core authenticate) take allowClients. With false, a verified client credentials token is refused with 403 user_required and no challenge, the same shape as insufficient_role. Invalid tokens still get 401 first, and the default keeps today's behavior. Refs #260 --- .changeset/resource-server-allow-clients.md | 10 ++++++++++ packages/core/README.md | 4 +++- packages/core/src/resourceServer.ts | 20 ++++++++++++++++++- packages/express/README.md | 12 ++++++++++- packages/express/src/index.ts | 1 + .../express/src/middleware/resourceServer.ts | 12 ++++++++--- packages/fastify/README.md | 16 ++++++++++++++- packages/fastify/src/index.ts | 1 + packages/fastify/src/resourceServer.ts | 12 ++++++++--- packages/nextjs/README.md | 14 ++++++++++++- packages/nextjs/src/index.ts | 1 + packages/nextjs/src/resourceServer.ts | 6 ++++++ 12 files changed, 98 insertions(+), 11 deletions(-) create mode 100644 .changeset/resource-server-allow-clients.md diff --git a/.changeset/resource-server-allow-clients.md b/.changeset/resource-server-allow-clients.md new file mode 100644 index 0000000..f6c452f --- /dev/null +++ b/.changeset/resource-server-allow-clients.md @@ -0,0 +1,10 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": minor +"@seamless-auth/fastify": minor +"@seamless-auth/nextjs": minor +--- + +Let a verify-only resource server require a user rather than a client credentials token (#260). The Express and Fastify `requireAuth` guards from `createResourceServer` take `{ allowClients: false }`, the Next.js `protect` and `authenticate` requirements take `allowClients: false`, and core's `authenticate(authorization, { allowClients: false })` does the same. A verified token whose user has `isClient` set is then refused with `403 {"error":"user_required"}` and no `WWW-Authenticate` header, the same shape as `insufficient_role`. A token that fails verification still gets `401` first. `allowClients` defaults to `true`, so existing routes are unchanged. + +The shared verify-only vectors gain `is_client` on each accepted token and a `user_required_requests` list. Both are additive, so the Go, Python and Rust copies keep passing until those adapters implement the option. diff --git a/packages/core/README.md b/packages/core/README.md index 0854fea..7cf49b9 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -287,7 +287,9 @@ provider identity linking. Core and adapter code never store provider access tok `createResourceServer({ issuer, audience })` verifies the OpenID Provider's RFC 9068 access tokens for an API that only receives them, with `authenticate`, `authorizeScopes`, `authorizeRoles` and `stepUp` returning a `GuardRejection` (status, `errorCode`, `detail` and -`headers`, including the `WWW-Authenticate` challenge). The Express, Fastify and Next.js +`headers`, including the `WWW-Authenticate` challenge). `authenticate(authorization, +{ allowClients: false })` refuses a client credentials token (`isClient`) with +`403 user_required` and no challenge, the same shape as `insufficient_role`. The Express, Fastify and Next.js adapters wrap it; their READMEs list the rules and responses. `tests/fixtures/verify-only` holds the signed vectors every adapter, including Go, Python and Rust, is tested against, with the script that generates them. diff --git a/packages/core/src/resourceServer.ts b/packages/core/src/resourceServer.ts index 94c39d0..2fc1d37 100644 --- a/packages/core/src/resourceServer.ts +++ b/packages/core/src/resourceServer.ts @@ -58,6 +58,15 @@ export type ResourceServerAuthResult = | { user: ResourceServerUser; rejection?: undefined } | { user?: undefined; rejection: GuardRejection }; +export interface ResourceServerAuthOptions { + /** + * Accept a client credentials token (`isClient`). Defaults to `true`. With + * `false`, such a token is refused with `403 user_required`, so only a token + * issued to a user gets through. + */ + allowClients?: boolean; +} + export interface StepUpRequirement { /** Any of these `acr` values. `phr` is also met by `phrh`. */ acrValues?: string[]; @@ -73,6 +82,7 @@ export interface ResourceServer { /** Verifies the raw `Authorization` header. */ authenticate( authorization: string | undefined, + options?: ResourceServerAuthOptions, ): Promise; /** All of the scopes. */ authorizeScopes( @@ -117,6 +127,11 @@ const INSUFFICIENT_ROLE: GuardRejection = { errorCode: "insufficient_role", }; +const USER_REQUIRED: GuardRejection = { + status: 403, + errorCode: "user_required", +}; + function requiredValues(guard: string, values: string | string[]): string[] { const list = Array.isArray(values) ? values : [values]; if ( @@ -387,12 +402,15 @@ export function createResourceServer( async function authenticate( authorization: string | undefined, + { allowClients = true }: ResourceServerAuthOptions = {}, ): Promise { const token = extractBearerToken(authorization); if (!token) return { rejection: MISSING_TOKEN }; const user = await verify(token); - return user ? { user } : { rejection: INVALID_TOKEN }; + if (!user) return { rejection: INVALID_TOKEN }; + if (user.isClient && !allowClients) return { rejection: USER_REQUIRED }; + return { user }; } function scopeCheck(scopes: string | string[]) { diff --git a/packages/express/README.md b/packages/express/README.md index d8b58e3..8b4cff7 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -775,6 +775,15 @@ app.post( `requireAuth()` sets `req.user`. The other guards only trust a user it verified, so they cannot be satisfied by a cookie session or anything else that set `req.user`. +`requireAuth({ allowClients: false })` refuses a client credentials token with +`403 { "error": "user_required" }`, after the token itself has verified (a bad token still gets +`401`). The guards mounted after it then only ever see a user, so put the option on +`requireAuth` rather than on `requireScope`, `requireRole` or `requireStepUp`: + +```ts +app.get("/me/orders", api.requireAuth({ allowClients: false }), api.requireRole("member"), listMine); +``` + Tokens are verified the same way in every Seamless adapter (Express, Fastify, Next.js, Go, Python and Rust), held to shared signed test vectors: @@ -794,6 +803,7 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai | No bearer token | 401 | `missing_token` | `Bearer` | | Token fails verification | 401 | `invalid_token` | `Bearer error="invalid_token"` | | Missing scope (all needed) | 403 | `insufficient_scope`, plus `scope` | `Bearer error="insufficient_scope", scope="..."` | +| Client token, users only | 403 | `user_required` | none | | Missing role (any one) | 403 | `insufficient_role` | none | | Step-up not met | 401 | `insufficient_user_authentication` | RFC 9470 challenge with `error_description`, and `acr_values` / `max_age` if set | @@ -805,7 +815,7 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai The verified user has `id` (`sub`), `clientId`, `scopes`, `roles`, `sid`, `orgId`, `acr`, `amr` and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, with `isClient` set to `true` and the client id in `id`, and fails role and step-up checks. Check -`isClient` before treating `id` as a user id. +`isClient` before treating `id` as a user id, or pass `allowClients: false` to `requireAuth`. --- diff --git a/packages/express/src/index.ts b/packages/express/src/index.ts index e90cb4c..d5a6a4e 100644 --- a/packages/express/src/index.ts +++ b/packages/express/src/index.ts @@ -22,6 +22,7 @@ export type { DeliveryResult, EmailMessage, EmailTransport, + ResourceServerAuthOptions, ResourceServerClaims, ResourceServerOptions, ResourceServerUser, diff --git a/packages/express/src/middleware/resourceServer.ts b/packages/express/src/middleware/resourceServer.ts index f96dc2b..2412bd5 100644 --- a/packages/express/src/middleware/resourceServer.ts +++ b/packages/express/src/middleware/resourceServer.ts @@ -3,14 +3,19 @@ import { createResourceServer as createCoreResourceServer, type GuardRejection, type ResourceServer, + type ResourceServerAuthOptions, type ResourceServerOptions, type ResourceServerUser, type StepUpRequirement, } from "@seamless-auth/core"; export interface ExpressResourceServer { - /** Verifies `Authorization: Bearer` and sets `req.user`. Mount before the other guards. */ - requireAuth(): RequestHandler; + /** + * Verifies `Authorization: Bearer` and sets `req.user`. Mount before the other + * guards. `{ allowClients: false }` refuses a client credentials token with + * `403 user_required`, and the guards after it then only see users. + */ + requireAuth(options?: ResourceServerAuthOptions): RequestHandler; /** Every one of the scopes. */ requireScope(scopes: string | string[]): RequestHandler; /** Any one of the roles. Scoped names match as in `requireRole`. */ @@ -74,10 +79,11 @@ export function createResourceServer( return { verifier, - requireAuth() { + requireAuth(options) { return async (req: Request, res: Response, next: NextFunction) => { const { user, rejection } = await verifier.authenticate( req.headers.authorization, + options, ); if (rejection) { reject(res, rejection); diff --git a/packages/fastify/README.md b/packages/fastify/README.md index c5f92fc..688552d 100644 --- a/packages/fastify/README.md +++ b/packages/fastify/README.md @@ -161,6 +161,19 @@ app.get("/orders", { preHandler: [api.requireAuth(), api.requireScope("orders:re `requireAuth()` sets `request.user`. `requireScope`, `requireRole` and `requireStepUp` only trust a user it verified. +`requireAuth({ allowClients: false })` refuses a client credentials token with +`403 { "error": "user_required" }`, after the token itself has verified (a bad token still gets +`401`). The guards after it then only ever see a user, so put the option on `requireAuth` +rather than on `requireScope`, `requireRole` or `requireStepUp`: + +```ts +app.get( + "/me/orders", + { preHandler: [api.requireAuth({ allowClients: false }), api.requireRole("member")] }, + listMine, +); +``` + Tokens are verified the same way in every Seamless adapter (Express, Fastify, Next.js, Go, Python and Rust), held to shared signed test vectors: @@ -180,6 +193,7 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai | No bearer token | 401 | `missing_token` | `Bearer` | | Token fails verification | 401 | `invalid_token` | `Bearer error="invalid_token"` | | Missing scope (all needed) | 403 | `insufficient_scope`, plus `scope` | `Bearer error="insufficient_scope", scope="..."` | +| Client token, users only | 403 | `user_required` | none | | Missing role (any one) | 403 | `insufficient_role` | none | | Step-up not met | 401 | `insufficient_user_authentication` | RFC 9470 challenge with `error_description`, and `acr_values` / `max_age` if set | @@ -191,7 +205,7 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai The verified user has `id` (`sub`), `clientId`, `scopes`, `roles`, `sid`, `orgId`, `acr`, `amr` and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, with `isClient` set to `true` and the client id in `id`, and fails role and step-up checks. Check -`isClient` before treating `id` as a user id. +`isClient` before treating `id` as a user id, or pass `allowClients: false` to `requireAuth`. ## Adopter-supplied message delivery diff --git a/packages/fastify/src/index.ts b/packages/fastify/src/index.ts index 6352718..6fc0c85 100644 --- a/packages/fastify/src/index.ts +++ b/packages/fastify/src/index.ts @@ -20,6 +20,7 @@ export type { DeliveryResult, EmailMessage, EmailTransport, + ResourceServerAuthOptions, ResourceServerClaims, ResourceServerOptions, ResourceServerUser, diff --git a/packages/fastify/src/resourceServer.ts b/packages/fastify/src/resourceServer.ts index d210914..0f26b10 100644 --- a/packages/fastify/src/resourceServer.ts +++ b/packages/fastify/src/resourceServer.ts @@ -3,6 +3,7 @@ import { createResourceServer as createCoreResourceServer, type GuardRejection, type ResourceServer, + type ResourceServerAuthOptions, type ResourceServerOptions, type ResourceServerUser, type StepUpRequirement, @@ -14,8 +15,12 @@ type PreHandler = ( ) => Promise; export interface FastifyResourceServer { - /** Verifies `Authorization: Bearer` and sets `request.user`. Run before the other guards. */ - requireAuth(): PreHandler; + /** + * Verifies `Authorization: Bearer` and sets `request.user`. Run before the + * other guards. `{ allowClients: false }` refuses a client credentials token + * with `403 user_required`, and the guards after it then only see users. + */ + requireAuth(options?: ResourceServerAuthOptions): PreHandler; /** Every one of the scopes. */ requireScope(scopes: string | string[]): PreHandler; /** Any one of the roles. Scoped names match as in `requireRole`. */ @@ -72,13 +77,14 @@ export function createResourceServer( return { verifier, - requireAuth() { + requireAuth(options) { return async function resourceServerAuth( req: FastifyRequest, reply: FastifyReply, ) { const { user, rejection } = await verifier.authenticate( req.headers.authorization, + options, ); if (rejection) return reject(reply, rejection); diff --git a/packages/nextjs/README.md b/packages/nextjs/README.md index 02248c2..48490ee 100644 --- a/packages/nextjs/README.md +++ b/packages/nextjs/README.md @@ -287,6 +287,17 @@ export const POST = api.protect( `protect` checks `scopes`, then `roles`, then `stepUp`. `api.authenticate(request, requirements)` returns `{ user }` or the `{ response }` to send, for handlers that branch on it. +`allowClients: false` in the requirements refuses a client credentials token with +`403 { "error": "user_required" }`. It is checked once the token has verified (a bad token still +gets `401`) and before `scopes`, `roles` and `stepUp`: + +```ts +export const GET = api.protect( + async (_request, _context, user) => Response.json(await listOrdersFor(user.id)), + { allowClients: false, roles: "member" }, +); +``` + Tokens are verified the same way in every Seamless adapter (Express, Fastify, Next.js, Go, Python and Rust), held to shared signed test vectors: @@ -306,6 +317,7 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai | No bearer token | 401 | `missing_token` | `Bearer` | | Token fails verification | 401 | `invalid_token` | `Bearer error="invalid_token"` | | Missing scope (all needed) | 403 | `insufficient_scope`, plus `scope` | `Bearer error="insufficient_scope", scope="..."` | +| Client token, users only | 403 | `user_required` | none | | Missing role (any one) | 403 | `insufficient_role` | none | | Step-up not met | 401 | `insufficient_user_authentication` | RFC 9470 challenge with `error_description`, and `acr_values` / `max_age` if set | @@ -317,7 +329,7 @@ the key set cannot be fetched, the request gets `401 invalid_token`; nothing fai The verified user has `id` (`sub`), `clientId`, `scopes`, `roles`, `sid`, `orgId`, `acr`, `amr` and `authTime`. A client credentials token (`sub` equal to `client_id`) verifies too, with `isClient` set to `true` and the client id in `id`, and fails role and step-up checks. Check -`isClient` before treating `id` as a user id. +`isClient` before treating `id` as a user id, or pass `allowClients: false`. ## Options diff --git a/packages/nextjs/src/index.ts b/packages/nextjs/src/index.ts index ab9974c..3312879 100644 --- a/packages/nextjs/src/index.ts +++ b/packages/nextjs/src/index.ts @@ -40,6 +40,7 @@ export type { DeliveryResult, EmailMessage, EmailTransport, + ResourceServerAuthOptions, ResourceServerClaims, ResourceServerOptions, ResourceServerUser, diff --git a/packages/nextjs/src/resourceServer.ts b/packages/nextjs/src/resourceServer.ts index 4675685..bfcecdf 100644 --- a/packages/nextjs/src/resourceServer.ts +++ b/packages/nextjs/src/resourceServer.ts @@ -9,6 +9,11 @@ import { /** What a route requires beyond a valid token. Checked in this order. */ export interface ResourceServerRequirements { + /** + * Accept a client credentials token (`isClient`). Defaults to `true`. With + * `false`, such a token is refused with `403 user_required`. + */ + allowClients?: boolean; /** Every one of the scopes. */ scopes?: string | string[]; /** Any one of the roles, with scoped role matching. */ @@ -92,6 +97,7 @@ export function createResourceServer( ): Promise { const { user, rejection } = await verifier.authenticate( request.headers.get("authorization") ?? undefined, + { allowClients: requirements.allowClients }, ); if (rejection) return { response: toResponse(rejection) }; From e054f5bc17b8e01a206fe6174fc4b535dd6896e2 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 10:57:31 -0500 Subject: [PATCH 15/17] ci: run conformance from seamless-cli v0.27.2 and hold Next.js to the content-type gate --- .github/workflows/conformance.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml index 071c00f..c0d1890 100644 --- a/.github/workflows/conformance.yml +++ b/.github/workflows/conformance.yml @@ -19,7 +19,7 @@ concurrency: jobs: verify: - uses: fells-code/seamless-cli/.github/workflows/verify-conformance.yml@1ff9552d68d3d74096992a83d931acc0233edab2 # v0.26.0 + uses: fells-code/seamless-cli/.github/workflows/verify-conformance.yml@28a363fcd6ba6c2ea3930c7172093638d8c41fe2 # v0.27.2 with: server-ref: ${{ github.event.pull_request.head.sha }} @@ -43,7 +43,7 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: repository: fells-code/seamless-cli - ref: 1ff9552d68d3d74096992a83d931acc0233edab2 # v0.26.0 + ref: 28a363fcd6ba6c2ea3930c7172093638d8c41fe2 # v0.27.2 path: seamless-cli - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -89,6 +89,10 @@ jobs: - name: Conformance env: SEAMLESS_API_DIR: ${{ github.workspace }}/seamless-auth-api + # This adapter ships the content-type gate, so hold it to the pending + # contract (seamless-cli verify/harness/lib/pendingContracts.ts). The + # reusable workflow above has no input for it yet. + SEAMLESS_VERIFY_CONTRACTS: content-type-gate run: node seamless-cli/dist/index.js verify --adapter-url=http://localhost:3002 - name: Reference app log From fa9c5b9e928fcfc3fbf0cda338d63fd26ca12730 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 11:39:04 -0500 Subject: [PATCH 16/17] test(core): reproduce a second tab losing its session on a late refresh cookie Browser tabs share one cookie jar, so a second tab can send the old refresh cookie a moment after the first tab's rotation completed. That replay should get the session the first tab got for 5 seconds, without calling the auth API. After 5 seconds, or for a failed rotation, it goes to the auth API. A bearer replay always goes to the auth API, even inside the 5 seconds. The two cookie window tests fail on this branch. Refs #258 --- .../core/tests/refreshAccessToken.test.js | 8 +-- packages/core/tests/refreshReplay.test.js | 67 ++++++++++++++++++- 2 files changed, 69 insertions(+), 6 deletions(-) diff --git a/packages/core/tests/refreshAccessToken.test.js b/packages/core/tests/refreshAccessToken.test.js index 6d490bc..a1010ed 100644 --- a/packages/core/tests/refreshAccessToken.test.js +++ b/packages/core/tests/refreshAccessToken.test.js @@ -138,9 +138,9 @@ describe("refreshAccessToken", () => { expect(authFetchMock).toHaveBeenCalledTimes(1); }); - // A completed rotation is not shared: the stale cookie is a replay the auth - // API must see (#258). - it("sends a follow-up call with the stale cookie to the auth API", async () => { + // Another tab sending the stale cookie a moment later shares the completed + // rotation rather than tripping reuse detection (#258). + it("reuses the freshly-rotated result for an immediate follow-up call with the stale cookie", async () => { const { refreshAccessToken } = await import("../dist/refreshAccessToken.js"); @@ -182,6 +182,6 @@ describe("refreshAccessToken", () => { }); expect(first).toEqual(second); - expect(authFetchMock).toHaveBeenCalledTimes(2); + expect(authFetchMock).toHaveBeenCalledTimes(1); }); }); diff --git a/packages/core/tests/refreshReplay.test.js b/packages/core/tests/refreshReplay.test.js index 224f87d..aedf517 100644 --- a/packages/core/tests/refreshReplay.test.js +++ b/packages/core/tests/refreshReplay.test.js @@ -1,6 +1,9 @@ // A refresh token presented again after its rotation completed is a replay. It // must reach the auth API, whose reuse detection revokes the session family, -// rather than be handed the session the first caller got (#258). +// rather than be handed the session the first caller got (#258). The one +// exception is a refresh cookie presented again within 5 seconds: browser tabs +// share one cookie jar, so a second tab sending the old cookie a moment late is +// expected, and it gets the session the first tab got. import { jest } from "@jest/globals"; const verifyRefreshCookieMock = jest.fn(); @@ -44,18 +47,48 @@ const reused = () => ({ let counter = 0; const unique = (label) => `${label}-${(counter += 1)}`; +const clock = { + now: 0, + install() { + this.now = Date.now(); + jest.spyOn(Date, "now").mockImplementation(() => this.now); + }, + advance(ms) { + this.now += ms; + }, +}; + describe("a completed refresh rotation is not replayed (#258)", () => { beforeEach(() => { verifyRefreshCookieMock.mockReset(); authFetchMock.mockReset(); authFetchMock.mockResolvedValueOnce(rotated()).mockResolvedValue(reused()); + clock.install(); }); - it("sends a stolen refresh cookie to the auth API after the rotation", async () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("shares a completed cookie rotation with a replay inside 5 seconds", async () => { + const cookie = unique("cookie"); + verifyRefreshCookieMock.mockReturnValue({ sub: "user-123", refreshToken: "old" }); + + const firstTab = await refreshAccessToken(cookie, OPTS); + clock.advance(4_999); + const secondTab = await refreshAccessToken(cookie, OPTS); + + expect(firstTab).toEqual(ROTATED); + expect(secondTab).toEqual(firstTab); + expect(authFetchMock).toHaveBeenCalledTimes(1); + }); + + it("sends a refresh cookie replayed after 5 seconds to the auth API", async () => { const cookie = unique("cookie"); verifyRefreshCookieMock.mockReturnValue({ sub: "user-123", refreshToken: "old" }); const victim = await refreshAccessToken(cookie, OPTS); + clock.advance(5_000); const attacker = await refreshAccessToken(cookie, OPTS); expect(victim).toEqual(ROTATED); @@ -63,6 +96,20 @@ describe("a completed refresh rotation is not replayed (#258)", () => { expect(attacker).toBeNull(); }); + it("does not keep a failed cookie rotation", async () => { + const cookie = unique("cookie"); + verifyRefreshCookieMock.mockReturnValue({ sub: "user-123", refreshToken: "old" }); + authFetchMock.mockReset(); + authFetchMock.mockResolvedValueOnce(reused()).mockResolvedValue(rotated()); + + const first = await refreshAccessToken(cookie, OPTS); + const second = await refreshAccessToken(cookie, OPTS); + + expect(first).toBeNull(); + expect(second).toEqual(ROTATED); + expect(authFetchMock).toHaveBeenCalledTimes(2); + }); + it("sends a spent session token to the auth API after the rotation", async () => { const payload = { sub: "user-123", refreshToken: unique("token") }; @@ -89,6 +136,22 @@ describe("a completed refresh rotation is not replayed (#258)", () => { }); }); + it("sends a bearer replay inside 5 seconds to the auth API", async () => { + const token = unique("bearer"); + const opts = { authServerUrl: OPTS.authServerUrl }; + + const victim = await refreshBearerSession(token, opts); + clock.advance(1_000); + const attacker = await refreshBearerSession(token, opts); + + expect(victim).toEqual({ status: 200, body: ROTATED }); + expect(authFetchMock).toHaveBeenCalledTimes(2); + expect(attacker).toEqual({ + status: 401, + body: { error: "refresh_token_reused" }, + }); + }); + it("still collapses callers that race one rotation in flight", async () => { const token = unique("bearer"); const opts = { authServerUrl: OPTS.authServerUrl }; From 595dfaae931645367222bbae151d87d840ff8649 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 3 Nov 2026 11:39:58 -0500 Subject: [PATCH 17/17] fix: keep a completed refresh for cookie transport for five seconds Browser tabs share one refresh cookie, so a second tab can send the old cookie just after the first tab's rotation completed. Sending it to the auth API trips reuse detection and signs both tabs out, which breaks the multi-tab conformance case. Keep a successful cookie rotation for 5 seconds, keyed on a hash of the old refresh cookie, and hand it to a caller presenting that cookie without calling the auth API. Failed rotations are not kept, expired entries are pruned on insert and the map is capped. Bearer and session token refreshes keep sharing only a rotation in flight, so a late bearer replay still reaches the auth API. Refs #258 --- .changeset/refresh-in-flight-only.md | 6 ++- packages/core/src/refreshAccessToken.ts | 72 ++++++++++++++++++++++++- packages/express/README.md | 7 ++- 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/.changeset/refresh-in-flight-only.md b/.changeset/refresh-in-flight-only.md index 876246e..6a72176 100644 --- a/.changeset/refresh-in-flight-only.md +++ b/.changeset/refresh-in-flight-only.md @@ -5,6 +5,8 @@ "@seamless-auth/nextjs": patch --- -Stop replaying a completed refresh rotation to later callers (#258). After a successful rotation, the adapter used to hand the new session to any caller presenting the same old refresh cookie or bearer refresh token for the next 5 seconds, without calling the auth API, so a stolen refresh token replayed in that window got the new tokens and the auth API's reuse detection never saw it. +Stop replaying a completed bearer refresh rotation to later callers (#258). After a successful rotation, the adapter used to hand the new session to any caller presenting the same old refresh cookie or bearer refresh token for the next 5 seconds, without calling the auth API, so a stolen bearer refresh token replayed in that window got the new tokens and the auth API's reuse detection never saw it. -Only a rotation still in flight is shared now. A request that presents a refresh token after its rotation completed goes to the auth API, which treats it as a replay: the cookie path answers as an expired session, and a bearer client gets the API's `401 {"error":"refresh_token_reused"}`. Two browser tabs that send the same old refresh cookie a moment apart, rather than at the same time, can now trip that reuse detection and sign the user out. +Bearer transport now shares only a rotation still in flight. A bearer client that presents a refresh token after its rotation completed goes to the auth API, which treats it as a replay and answers `401 {"error":"refresh_token_reused"}`. + +Cookie transport keeps the 5 second window, because browser tabs share one refresh cookie and a second tab sending the old cookie just after the first tab's rotation is expected. Within 5 seconds of a successful rotation the old refresh cookie gets the same rotated session; after that it goes to the auth API as a replay. A failed rotation is never kept, and the window is keyed on a hash of the cookie. diff --git a/packages/core/src/refreshAccessToken.ts b/packages/core/src/refreshAccessToken.ts index 0bdc7bc..98ece2f 100644 --- a/packages/core/src/refreshAccessToken.ts +++ b/packages/core/src/refreshAccessToken.ts @@ -1,3 +1,5 @@ +import { createHash } from "node:crypto"; + import { authFetch } from "./authFetch.js"; import { createServiceToken } from "./createServiceToken.js"; import { verifyRefreshCookie } from "./verifyRefreshCookie.js"; @@ -40,7 +42,8 @@ const inFlightRefreshes = new Map< * Only a rotation still in flight is shared. Once it completes, a caller that * presents the same token again goes to the auth API, whose reuse detection is * what catches a stolen token; handing that caller the new session would hide - * the replay. + * the replay. The refresh cookie is the one exception, see + * `completedCookieRefreshes`. */ async function dedupeRefresh( key: string, @@ -61,13 +64,78 @@ async function dedupeRefresh( } } +/** + * Browser tabs share one cookie jar, so a second tab can send the old refresh + * cookie just after the first tab's rotation completed, before its response set + * the new one. Sending that to the auth API would revoke the session for both + * tabs, so a successful cookie rotation is kept for a few seconds and handed to + * a caller presenting the same old cookie. Bearer and session token refreshes + * never use this: their clients hold the token themselves, so a late repeat is + * a replay for the auth API to judge. + * + * Keyed on a hash of the cookie so the map never holds a live refresh token. + */ +const COMPLETED_COOKIE_REFRESH_TTL_MS = 5000; +// Rotated cookies are never looked up again, so expired entries are swept on +// insert (throttled to once per TTL) and the size is capped as a backstop +// against a burst of distinct cookies. +const MAX_COMPLETED_COOKIE_REFRESHES = 10_000; +const completedCookieRefreshes = new Map< + string, + { result: RefreshAccessTokenResult; expiresAt: number } +>(); +let lastCompletedPruneAt = 0; + +function pruneCompletedCookieRefreshes(now: number): void { + if (now - lastCompletedPruneAt >= COMPLETED_COOKIE_REFRESH_TTL_MS) { + for (const [key, entry] of completedCookieRefreshes) { + if (entry.expiresAt <= now) completedCookieRefreshes.delete(key); + } + lastCompletedPruneAt = now; + } + + while (completedCookieRefreshes.size >= MAX_COMPLETED_COOKIE_REFRESHES) { + const oldest = completedCookieRefreshes.keys().next().value; + if (oldest === undefined) break; + completedCookieRefreshes.delete(oldest); + } +} + +async function dedupeCookieRefresh( + refreshCookie: string, + run: () => Promise, +): Promise { + const key = createHash("sha256").update(refreshCookie).digest("base64url"); + + const completed = completedCookieRefreshes.get(key); + if (completed) { + if (completed.expiresAt > Date.now()) return completed.result; + completedCookieRefreshes.delete(key); + } + + const result = await dedupeRefresh(`cookie:${key}`, run); + + // Every caller sharing the in-flight rotation reaches this; only the first + // insert matters, and a later one must not extend the window. + if (result && !completedCookieRefreshes.has(key)) { + const now = Date.now(); + pruneCompletedCookieRefreshes(now); + completedCookieRefreshes.set(key, { + result, + expiresAt: now + COMPLETED_COOKIE_REFRESH_TTL_MS, + }); + } + + return result; +} + export async function refreshAccessToken( refreshCookie: string, opts: RefreshAccessTokenOptions, ): Promise { assertSecrets(opts, { allowPlaceholder: true }); - return dedupeRefresh(`cookie:${refreshCookie}`, async () => { + return dedupeCookieRefresh(refreshCookie, async () => { const payload = verifyRefreshCookie(refreshCookie, opts.cookieSecret); if (!payload) return null; return rotate(payload, opts); diff --git a/packages/express/README.md b/packages/express/README.md index 8b4cff7..e5a8d47 100644 --- a/packages/express/README.md +++ b/packages/express/README.md @@ -545,7 +545,8 @@ routes serve a second contract when the request carries the header the pair in the response. The auth API rotates refresh tokens and treats a replayed one as theft (revoking the whole chain, `401 { "error": "refresh_token_reused" }`), so refresh once at a time. Concurrent rotations of the same token that reach the adapter together are collapsed into one - upstream call. One that arrives after that rotation completed goes to the auth API as a replay. + upstream call. One that arrives after that rotation completed goes to the auth API as a replay, + however soon after. - Message delivery, client IP and user agent forwarding, and the service token behave exactly as in cookie transport, since the adapter attaches them regardless of how the client carries its session. @@ -582,6 +583,10 @@ To accept the same access token on your own routes, configure `POST /auth/refresh` also works in cookie transport, rotating the refresh cookie into fresh session cookies, for a browser client that wants an explicit refresh rather than the silent one. +Browser tabs share one refresh cookie, so cookie transport also shares a completed rotation for 5 +seconds: a request presenting the old refresh cookie within 5 seconds of a successful rotation gets +that rotated session without calling the auth API. After 5 seconds the old cookie goes to the auth +API as a replay. A failed rotation is never shared. ---