From f848016b019d9762be2c1b088fd9cce0d247da5e Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Tue, 6 Oct 2026 23:37:44 -0400 Subject: [PATCH] fix(core): check a silently refreshed token against authServerIssuer The Release workflow on main failed in packages/core: two tests in ensureCookies.test.js expected verifySignedAuthResponse to be called with three arguments and received a fourth, undefined. #193 (silent refresh verification) and #192 (authServerIssuer) were written in parallel and merged minutes apart. #192 gave verifySignedAuthResponse and issueSessionCookies an issuer argument and threaded it through every flow that issues a session; #193 routed the silent refresh through issueSessionCookies without it. So the tests were stale, and the silent refresh also ignored a configured authServerIssuer: an app reaching the auth server at another URL (the local Docker stack from the host) got a 401 on every silent refresh and was signed out. Add authServerIssuer to EnsureCookiesOptions and the Express createEnsureCookiesMiddleware options, pass it to issueSessionCookies, and have the Express, Fastify and Next.js adapters pass their configured value. The JWKS verification of the refreshed token is unchanged and still fails closed. Update the core assertions for the issuer argument, and cover the silent refresh with a configured issuer, and from an unexpected one, in the Express middleware tests and the Fastify and Next.js issuer parity suites. --- .../silent-refresh-auth-server-issuer.md | 8 +++ packages/core/src/ensureCookies.ts | 4 +- packages/core/tests/ensureCookies.test.js | 20 +++++++ packages/express/src/createServer.ts | 1 + .../express/src/middleware/ensureCookies.ts | 7 +++ .../tests/ensureCookies.middleware.test.js | 32 +++++++++++- packages/fastify/src/hooks/ensureCookies.ts | 1 + packages/fastify/tests/issuer.parity.test.js | 52 +++++++++++++++++++ packages/nextjs/src/handler.ts | 1 + packages/nextjs/tests/issuer.parity.test.js | 52 +++++++++++++++++++ 10 files changed, 175 insertions(+), 3 deletions(-) create mode 100644 .changeset/silent-refresh-auth-server-issuer.md diff --git a/.changeset/silent-refresh-auth-server-issuer.md b/.changeset/silent-refresh-auth-server-issuer.md new file mode 100644 index 0000000..0e8818b --- /dev/null +++ b/.changeset/silent-refresh-auth-server-issuer.md @@ -0,0 +1,8 @@ +--- +"@seamless-auth/core": patch +"@seamless-auth/express": patch +"@seamless-auth/fastify": patch +"@seamless-auth/nextjs": patch +--- + +Check a silently refreshed access token against `authServerIssuer`. The silent refresh in `ensureCookies` now verifies the token it returns, but it checked `iss` against `authServerUrl` even when `authServerIssuer` was set, so an app reaching the auth server at another URL (the local Docker stack from the host) answered 401 on every silent refresh and signed the user out. `EnsureCookiesOptions` and the Express `createEnsureCookiesMiddleware` take an optional `authServerIssuer`, and the adapters pass their configured one. diff --git a/packages/core/src/ensureCookies.ts b/packages/core/src/ensureCookies.ts index 48cdad5..8cbae87 100644 --- a/packages/core/src/ensureCookies.ts +++ b/packages/core/src/ensureCookies.ts @@ -2,6 +2,7 @@ import { verifyCookieJwt } from "./verifyCookieJwt.js"; import type { ResultFailure } from "./result.js"; import { refreshAccessToken } from "./refreshAccessToken.js"; import { assertSecrets } from "./validateSecrets.js"; +import type { AuthServerIssuerOption } from "./authServerIssuer.js"; import { issueSessionCookies, type UpstreamSessionResponse, @@ -43,7 +44,7 @@ export interface EnsureCookiesResult extends ResultFailure { clearCookies?: string[]; } -export interface EnsureCookiesOptions { +export interface EnsureCookiesOptions extends AuthServerIssuerOption { authServerUrl: string; cookieDomain?: string; accessCookieName: string; @@ -271,6 +272,7 @@ async function refreshRequiredCookie( { authServerUrl: opts.authServerUrl, audience: opts.accessTokenAudience || opts.authServerUrl, + authServerIssuer: opts.authServerIssuer, accessCookieName: cookieName, refreshCookieName: opts.refreshCookieName, cookieDomain: opts.cookieDomain, diff --git a/packages/core/tests/ensureCookies.test.js b/packages/core/tests/ensureCookies.test.js index 43a2d33..c1d519c 100644 --- a/packages/core/tests/ensureCookies.test.js +++ b/packages/core/tests/ensureCookies.test.js @@ -294,6 +294,7 @@ describe("ensureCookies", () => { "new-access", "https://auth.example.com", "https://app.example.com", + undefined, ); }); @@ -306,6 +307,25 @@ describe("ensureCookies", () => { "new-access", "https://auth.example.com", "https://auth.example.com", + undefined, + ); + }); + + it("verifies against the configured auth server issuer", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-123" }); + + await silentRefresh({ + ...BASE_OPTS, + authServerUrl: "http://localhost:5312", + authServerIssuer: "http://auth:5312", + accessTokenAudience: "http://auth:5312", + }); + + expect(verifySignedAuthResponseMock).toHaveBeenCalledWith( + "new-access", + "http://localhost:5312", + "http://auth:5312", + "http://auth:5312", ); }); diff --git a/packages/express/src/createServer.ts b/packages/express/src/createServer.ts index 3927fb8..8f0cb2c 100644 --- a/packages/express/src/createServer.ts +++ b/packages/express/src/createServer.ts @@ -326,6 +326,7 @@ export function createSeamlessAuthServer( issuer: SERVICE_TOKEN_ISSUER, audience: SERVICE_TOKEN_AUDIENCE, accessTokenAudience: resolvedOpts.audience, + authServerIssuer: resolvedOpts.authServerIssuer, keyId: resolvedOpts.jwksKid, resolveClientIp: resolvedOpts.resolveClientIp, }), diff --git a/packages/express/src/middleware/ensureCookies.ts b/packages/express/src/middleware/ensureCookies.ts index 56267df..6be61d9 100644 --- a/packages/express/src/middleware/ensureCookies.ts +++ b/packages/express/src/middleware/ensureCookies.ts @@ -30,6 +30,12 @@ export interface EnsureCookiesMiddlewareOptions { * refreshed token is verified against. Defaults to `authServerUrl`. */ accessTokenAudience?: string; + /** + * Expected `iss` on a silently refreshed access token, when the auth server + * signs under a different issuer from `authServerUrl`. Defaults to + * `authServerUrl`. + */ + authServerIssuer?: string; keyId: string; resolveClientIp?: ClientIpResolver; } @@ -68,6 +74,7 @@ export function createEnsureCookiesMiddleware( issuer: opts.issuer, audience: opts.audience, accessTokenAudience: opts.accessTokenAudience, + authServerIssuer: opts.authServerIssuer, keyId: opts.keyId, forwardedClientIp: buildForwardedClientIp(req, opts.resolveClientIp), forwardedUserAgent: buildForwardedUserAgent(req), diff --git a/packages/express/tests/ensureCookies.middleware.test.js b/packages/express/tests/ensureCookies.middleware.test.js index 8d43984..b3c27af 100644 --- a/packages/express/tests/ensureCookies.middleware.test.js +++ b/packages/express/tests/ensureCookies.middleware.test.js @@ -78,11 +78,11 @@ describe("createEnsureCookiesMiddleware silent refresh", () => { return server; } - async function refreshWithTokenFor(audience, options, refreshToken) { + async function refreshWithTokenFor(audience, options, refreshToken, issuer = AUTH) { const token = jwt.sign({ sub: "user-123", typ: "access", sid: "s-1" }, privateKey, { algorithm: "RS256", keyid: "k1", - issuer: AUTH, + issuer, audience, expiresIn: "5m", }); @@ -131,4 +131,32 @@ describe("createEnsureCookiesMiddleware silent refresh", () => { expect(res.status).toBe(401); expect(res.headers["set-cookie"].join(";")).not.toMatch(/access=ey/); }); + + // On the local Docker stack the auth server signs as http://auth:5312 while + // the app reaches it at authServerUrl (fells-code/seamless-cli#224). + it("verifies the refreshed token against authServerIssuer", async () => { + const res = await refreshWithTokenFor( + "http://auth:5312", + { accessTokenAudience: "http://auth:5312", authServerIssuer: "http://auth:5312" }, + "opaque-3", + "http://auth:5312", + ); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ sub: "user-123", sessionId: "s-1" }); + }); + + it("refuses a refreshed token from an issuer other than the expected one", async () => { + const spy = jest.spyOn(console, "error").mockImplementation(() => {}); + const res = await refreshWithTokenFor( + "http://auth:5312", + { accessTokenAudience: "http://auth:5312" }, + "opaque-4", + "http://auth:5312", + ); + spy.mockRestore(); + + expect(res.status).toBe(401); + expect(res.headers["set-cookie"].join(";")).not.toMatch(/access=ey/); + }); }); diff --git a/packages/fastify/src/hooks/ensureCookies.ts b/packages/fastify/src/hooks/ensureCookies.ts index 12d5dd0..ab6e56d 100644 --- a/packages/fastify/src/hooks/ensureCookies.ts +++ b/packages/fastify/src/hooks/ensureCookies.ts @@ -70,6 +70,7 @@ export function createEnsureCookiesHook(opts: ResolvedOptions, prefix: string) { issuer: SERVICE_TOKEN_ISSUER, audience: SERVICE_TOKEN_AUDIENCE, accessTokenAudience: opts.audience, + authServerIssuer: opts.authServerIssuer, keyId: opts.jwksKid, forwardedClientIp: buildForwardedClientIp(req, opts.resolveClientIp), forwardedUserAgent: buildForwardedUserAgent(req), diff --git a/packages/fastify/tests/issuer.parity.test.js b/packages/fastify/tests/issuer.parity.test.js index 03e6737..89268e8 100644 --- a/packages/fastify/tests/issuer.parity.test.js +++ b/packages/fastify/tests/issuer.parity.test.js @@ -93,6 +93,19 @@ async function mockUpstream(issuer) { refreshTtl: 2592000, }); } + if (href === `${AUTH}/refresh`) { + return Response.json({ + sub: "user-123", + token: access, + refreshToken: "refresh-2", + roles: ["athlete"], + ttl: 1800, + refreshTtl: 2592000, + }); + } + if (href === `${AUTH}/users/me`) { + return Response.json({ user: { id: "user-123" } }); + } throw new Error(`Unexpected upstream call: ${href}`); }); @@ -106,6 +119,19 @@ 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. +let refreshCount = 0; +const silentRefresh = () => ({ + method: "get", + path: "/users/me", + cookie: `seamless-refresh=${jwt.sign( + { sub: "user-123", refreshToken: `refresh-${++refreshCount}` }, + COOKIE_SECRET, + { algorithm: "HS256", expiresIn: "3600s" }, + )}`, +}); + const DOCKER_OPTIONS = { authServerIssuer: DOCKER_ISSUER, audience: DOCKER_ISSUER }; const STEPS = [ @@ -243,6 +269,32 @@ describe("authServerIssuer", () => { } }, ); + + it("accepts a silent refresh signed by a configured issuer distinct from the URL", async () => { + const calls = await mockUpstream(DOCKER_ISSUER); + + const res = await run(silentRefresh(), DOCKER_OPTIONS); + + expect(res.status).toBe(200); + expect(res.cookies).toEqual( + expect.arrayContaining(["seamless-access", "seamless-refresh"]), + ); + expect(calls.every((href) => href.startsWith(`${AUTH}/`))).toBe(true); + }); + + it("rejects a silent refresh from an issuer other than the expected one", async () => { + await mockUpstream(DOCKER_ISSUER); + const unconfigured = await run(silentRefresh(), {}); + + await mockUpstream(AUTH); + const misconfigured = await run(silentRefresh(), DOCKER_OPTIONS); + + // The 401 clears the session cookies, so their names still appear. + for (const res of [unconfigured, misconfigured]) { + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: "Refresh failed" }); + } + }); }); it("sets the same session cookies from both adapters with a configured issuer", async () => { diff --git a/packages/nextjs/src/handler.ts b/packages/nextjs/src/handler.ts index f72c1ed..37b14ae 100644 --- a/packages/nextjs/src/handler.ts +++ b/packages/nextjs/src/handler.ts @@ -231,6 +231,7 @@ async function dispatch( issuer: SERVICE_TOKEN_ISSUER, audience: SERVICE_TOKEN_AUDIENCE, accessTokenAudience: opts.audience, + authServerIssuer: opts.authServerIssuer, keyId: opts.jwksKid, forwardedClientIp: forwardedClientIp(request, opts.resolveClientIp), forwardedUserAgent: forwardedUserAgent(request), diff --git a/packages/nextjs/tests/issuer.parity.test.js b/packages/nextjs/tests/issuer.parity.test.js index 99e2922..aa22353 100644 --- a/packages/nextjs/tests/issuer.parity.test.js +++ b/packages/nextjs/tests/issuer.parity.test.js @@ -93,6 +93,19 @@ async function mockUpstream(issuer) { refreshTtl: 2592000, }); } + if (href === `${AUTH}/refresh`) { + return Response.json({ + sub: "user-123", + token: access, + refreshToken: "refresh-2", + roles: ["athlete"], + ttl: 1800, + refreshTtl: 2592000, + }); + } + if (href === `${AUTH}/users/me`) { + return Response.json({ user: { id: "user-123" } }); + } throw new Error(`Unexpected upstream call: ${href}`); }); @@ -106,6 +119,19 @@ 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. +let refreshCount = 0; +const silentRefresh = () => ({ + method: "get", + path: "/users/me", + cookie: `seamless-refresh=${jwt.sign( + { sub: "user-123", refreshToken: `refresh-${++refreshCount}` }, + COOKIE_SECRET, + { algorithm: "HS256", expiresIn: "3600s" }, + )}`, +}); + const DOCKER_OPTIONS = { authServerIssuer: DOCKER_ISSUER, audience: DOCKER_ISSUER }; const STEPS = [ @@ -243,6 +269,32 @@ describe("authServerIssuer", () => { } }, ); + + it("accepts a silent refresh signed by a configured issuer distinct from the URL", async () => { + const calls = await mockUpstream(DOCKER_ISSUER); + + const res = await run(silentRefresh(), DOCKER_OPTIONS); + + expect(res.status).toBe(200); + expect(res.cookies).toEqual( + expect.arrayContaining(["seamless-access", "seamless-refresh"]), + ); + expect(calls.every((href) => href.startsWith(`${AUTH}/`))).toBe(true); + }); + + it("rejects a silent refresh from an issuer other than the expected one", async () => { + await mockUpstream(DOCKER_ISSUER); + const unconfigured = await run(silentRefresh(), {}); + + await mockUpstream(AUTH); + const misconfigured = await run(silentRefresh(), DOCKER_OPTIONS); + + // The 401 clears the session cookies, so their names still appear. + for (const res of [unconfigured, misconfigured]) { + expect(res.status).toBe(401); + expect(res.body).toEqual({ error: "Refresh failed" }); + } + }); }); it("sets the same session cookies from both adapters with a configured issuer", async () => {