fix(cookies): parse an upstream cookie ttl before it reaches an adapter

.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.

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,

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/);
+  });
+});

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 }],