From 6931fd95f080ebccbc8ab966af2b8b3fe03e6b34 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Fri, 31 Jul 2026 09:04:56 -0400 Subject: [PATCH] fix(cookies): parse an upstream cookie ttl before it reaches an adapter Registration failed on Fastify with `TypeError: option maxAge is invalid: 300`. The auth API returns ttl as the string "300" on its registration response. Handler results declare ttl as a number but fill it from a parsed JSON body, so nothing caught the mismatch. From there the adapters diverged. Express multiplies the value into milliseconds, which coerces the string to a number and hides it, so registration has always worked there. Fastify passes the value through to `cookie`, whose Number.isInteger check rejects a string and throws. applyCookies now parses the lifetime once, before an adapter sees it, and uses the parsed value for the Max-Age, the Expires, and the signed cookie's own expiry. Anything that is not a positive whole number of seconds throws with the offending value, rather than issuing a session cookie with a lifetime nobody can vouch for. The parity suite hand-wrote ttl as a number in every scenario, so it agreed on input the real upstream does not send. It now covers a string ttl through the registration route, and fails without this change. The API returning a string here is a separate defect, fixed alongside this. Parsing at the boundary is what stops the next untyped upstream value from splitting the adapters the same way. Verified with the core, express, and fastify suites: 225, 150, and 44 passing. --- .changeset/coerce-upstream-cookie-ttl.md | 23 ++++++++++ packages/core/src/applyResult.ts | 36 +++++++++++++-- packages/core/tests/applyResult.test.js | 57 ++++++++++++++++++++++++ packages/fastify/tests/parity.test.js | 29 ++++++++++++ 4 files changed, 142 insertions(+), 3 deletions(-) create mode 100644 .changeset/coerce-upstream-cookie-ttl.md diff --git a/.changeset/coerce-upstream-cookie-ttl.md b/.changeset/coerce-upstream-cookie-ttl.md new file mode 100644 index 0000000..c684aaf --- /dev/null +++ b/.changeset/coerce-upstream-cookie-ttl.md @@ -0,0 +1,23 @@ +--- +'@seamless-auth/core': patch +'@seamless-auth/express': patch +'@seamless-auth/fastify': patch +--- + +Normalize a cookie lifetime arriving from upstream, so the two adapters cannot disagree about it. + +Registration failed on Fastify with `TypeError: option maxAge is invalid: 300`. The auth API returns +`ttl` as the string `"300"` on its registration response. Handler results declare `ttl` as a number +but fill it from a parsed JSON body, so nothing caught the mismatch. + +From there the adapters diverged. Express multiplies the value into milliseconds, which coerces the +string to a number and hides the problem, so it has always worked. Fastify passes the value through +to `cookie`, whose `Number.isInteger` check rejects a string and throws, failing the request. + +`applyCookies` now parses the lifetime once, before it reaches an adapter, and uses the parsed value +for the `Max-Age`, the `Expires`, and the signed cookie's own expiry. Anything that is not a positive +whole number of seconds throws with the offending value, rather than issuing a session cookie with a +lifetime nobody can vouch for. + +The parity suite hand-wrote `ttl` as a number in every scenario, so it agreed on input the real +upstream does not send. It now covers a string `ttl` through the registration route. diff --git a/packages/core/src/applyResult.ts b/packages/core/src/applyResult.ts index 37bacb8..b585f8d 100644 --- a/packages/core/src/applyResult.ts +++ b/packages/core/src/applyResult.ts @@ -106,6 +106,34 @@ export function signSessionCookie( }); } +/** + * Normalizes a cookie lifetime arriving from upstream. + * + * Handler results declare `ttl` as a number, but they populate it from a parsed + * JSON body, which is untyped. When the value arrives as a numeric string the + * declaration is simply wrong, and the two adapters then disagree: Express + * multiplies it into milliseconds, which coerces it to a number and hides the + * problem, while Fastify passes it through to `cookie`, whose `Number.isInteger` + * check rejects it and fails the request. That is the difference between an + * adapter working and an adapter throwing on the same upstream response, so it + * is settled here rather than in either one. + * + * Anything that is not a positive whole number of seconds throws. A cookie is + * the session, and emitting one with a lifetime nobody can vouch for is worse + * than refusing the response. + */ +function toTtlSeconds(ttl: unknown): number { + const seconds = typeof ttl === "string" ? Number(ttl) : ttl; + + if (typeof seconds !== "number" || !Number.isInteger(seconds) || seconds <= 0) { + throw new Error( + `Upstream returned an unusable cookie ttl: ${JSON.stringify(ttl)}`, + ); + } + + return seconds; +} + function requireSecret(opts: CookieSecurityOptions): string { if (!opts.cookieSecret) { throw new Error("Missing cookieSecret"); @@ -150,12 +178,14 @@ export function applyCookies( const now = Date.now(); for (const cookie of result.setCookies) { + const ttlSeconds = toTtlSeconds(cookie.ttl); + adapter.setCookie({ name: cookie.name, - value: signSessionCookie(cookie.value, secret, cookie.ttl), + value: signSessionCookie(cookie.value, secret, ttlSeconds), domain: cookie.domain, - maxAgeSeconds: cookie.ttl, - expires: new Date(now + cookie.ttl * 1000), + maxAgeSeconds: ttlSeconds, + expires: new Date(now + ttlSeconds * 1000), httpOnly: true, secure, sameSite, diff --git a/packages/core/tests/applyResult.test.js b/packages/core/tests/applyResult.test.js index 6499d1f..70cc63e 100644 --- a/packages/core/tests/applyResult.test.js +++ b/packages/core/tests/applyResult.test.js @@ -271,3 +271,60 @@ describe("signSessionCookie", () => { expect(decoded.exp - decoded.iat).toBe(300); }); }); + +// The auth API's registration response has sent `ttl` as the string "300". +// Handler results declare it a number, but they fill it from a parsed JSON body, +// so nothing caught it. Express hid it by multiplying into milliseconds, which +// coerces; Fastify passed it to `cookie`, whose Number.isInteger check threw and +// failed the request. Normalizing here is what stops the two disagreeing. +describe("cookie ttl arriving from an untyped upstream body", () => { + const setCookie = (ttl) => { + const adapter = recorder(); + + applyCookies( + { setCookies: [{ name: "seamless-ephemeral", value: { sub: "u1" }, ttl }] }, + adapter, + { cookieSecret: SECRET }, + ); + + return adapter.calls.set[0]; + }; + + it("treats a numeric string exactly like the number", () => { + const fromString = setCookie("300"); + const fromNumber = setCookie(300); + + expect(fromString.maxAgeSeconds).toBe(300); + expect(fromString.maxAgeSeconds).toBe(fromNumber.maxAgeSeconds); + }); + + it("emits a maxAge the cookie library will accept", () => { + expect(Number.isInteger(setCookie("300").maxAgeSeconds)).toBe(true); + }); + + it("dates the expiry from the parsed seconds", () => { + const before = Date.now(); + const { expires } = setCookie("300"); + + expect(expires.getTime()).toBeGreaterThanOrEqual(before + 300 * 1000); + expect(expires.getTime()).toBeLessThan(before + 301 * 1000); + }); + + it("signs the cookie for the same lifetime either way", () => { + const decoded = jwt.verify(setCookie("300").value, SECRET); + + expect(decoded.exp - decoded.iat).toBe(300); + }); + + it.each([ + ["a non-numeric string", "15m"], + ["an empty string", ""], + ["a fraction", 300.5], + ["zero", 0], + ["a negative", -300], + ["null", null], + ["undefined", undefined], + ])("refuses %s rather than issuing a cookie nobody can vouch for", (_l, ttl) => { + expect(() => setCookie(ttl)).toThrow(/unusable cookie ttl/); + }); +}); diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index 0ba58f7..d772f3e 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -306,6 +306,35 @@ describe("fastify and express adapters agree", () => { ); }); + // The auth API's registration response sends `ttl` as the string "300". Every + // scenario in this file hand-writes a number, so the suite agreed on input the + // real upstream does not send: Express coerced the string by multiplying into + // milliseconds, Fastify handed it to `cookie` and got a TypeError, and + // registration failed on Fastify only. + it("issues identical session cookies when upstream sends ttl as a string", async () => { + const scenario = { + method: "post", + path: "/registration/register", + payload: { email: "user@example.com" }, + }; + const upstreamResponse = upstream(200, { + message: "Registration started", + sub: "user-123", + token: "registration-token", + ttl: "300", + }); + + const { fastify, express: expressResult } = await bothAdapters( + scenario, + upstreamResponse, + ); + + expect(fastify.status).toBe(200); + expect(fastify.status).toBe(expressResult.status); + expect(fastify.cookies).toEqual(expressResult.cookies); + expect(fastify.cookies.length).toBeGreaterThan(0); + }); + it.each([ ["default policy", {}], ["insecure dev", { cookieSecure: false }],