fix(session): stop repeating the credentials in the body that sets the cookies

.changeset/gentle-pandas-hide.md

@@ -0,0 +1,35 @@
+---
+'@seamless-auth/core': minor
+'@seamless-auth/express': minor
+'@seamless-auth/fastify': minor
+---
+
+Stop repeating the session credentials in the body of the response that sets them as
+cookies.
+
+Five handlers that issue session cookies returned the upstream body unchanged, and that body
+carries the access token and the refresh token, because it is the same body
+`issueSessionCookies` reads them out of to build the cookies. So every completed sign-in
+answered with `Set-Cookie: httpOnly` and then handed the same two values to the caller as
+JSON.
+
+The `httpOnly` flag exists to keep those tokens out of reach of page scripts. A response body
+is not: it is readable by anything that can see the response, and it reaches places a cookie
+does not, including a devtools or HAR export shared while debugging, a service worker, a
+browser extension with request access, an APM tool that records payloads, and a proxy
+configured to log bodies. The refresh token is the durable session credential, so it is the
+half that matters.
+
+`finishRegisterHandler` already had this right, answering `204` with no body at all. The five
+that did not are `finishLoginHandler`, `verifyLoginOtpHandler`, `finishOAuthLoginHandler`,
+`pollMagicLinkConfirmationHandler` and `switchOrganizationHandler`.
+
+Only `token` and `refreshToken` are removed, by one exported helper rather than five copies,
+so the handlers cannot drift apart on it again. Everything callers actually read survives:
+`message`, `sub`, `email`, `roles`, `phone`, `organizationId`, `ttl`, `refreshTtl` and
+`returnTo`. The cookies are unchanged, so sessions work exactly as before.
+
+Nothing in `@seamless-auth/react` reads either field off a response, and its result types
+already declare them absent, with the comment that sessions are carried by cookies so
+adopters have no reason to handle raw tokens. An adopter reading `token` or `refreshToken`
+directly off one of these responses, against that guidance, no longer can.

packages/core/src/handlers/finishLogin.ts

@@ -1,5 +1,8 @@
 import { authFetch } from "../authFetch.js";
-import { issueSessionCookies } from "../upstreamSession.js";
+import {
+  issueSessionCookies,
+  withoutSessionMaterial,
+} from "../upstreamSession.js";
 import { readPassthroughFailure } from "../upstreamError.js";
 import type { ResultFailure } from "../result.js";
 import type { CookiePayload } from "../ensureCookies.js";
@@ -53,7 +56,7 @@ export async function finishLoginHandler(
 
   return {
     status: 200,
-    body: data,
+    body: withoutSessionMaterial(data),
     setCookies: await issueSessionCookies(data, {
       authServerUrl: opts.authServerUrl,
       audience: opts.audience,

packages/core/src/handlers/oauthHandlers.ts

@@ -1,5 +1,8 @@
 import { authFetch } from "../authFetch.js";
-import { issueSessionCookies } from "../upstreamSession.js";
+import {
+  issueSessionCookies,
+  withoutSessionMaterial,
+} from "../upstreamSession.js";
 import { readPassthroughFailure } from "../upstreamError.js";
 import type { ResultFailure } from "../result.js";
 import type { CookiePayload } from "../ensureCookies.js";
@@ -92,7 +95,7 @@ export async function finishOAuthLoginHandler(
 
   return {
     status: up.status,
-    body: data,
+    body: withoutSessionMaterial(data),
     setCookies: await issueSessionCookies(data, {
       authServerUrl: opts.authServerUrl,
       audience: opts.audience,

packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts

@@ -1,5 +1,8 @@
 import { authFetch } from "../authFetch.js";
-import { issueSessionCookies } from "../upstreamSession.js";
+import {
+  issueSessionCookies,
+  withoutSessionMaterial,
+} from "../upstreamSession.js";
 import { readPassthroughFailure } from "../upstreamError.js";
 import type { ResultFailure } from "../result.js";
 import type { CookiePayload } from "../ensureCookies.js";
@@ -64,7 +67,7 @@ export async function pollMagicLinkConfirmationHandler(
 
   return {
     status: 200,
-    body: data,
+    body: withoutSessionMaterial(data),
     setCookies: await issueSessionCookies(data, {
       authServerUrl: opts.authServerUrl,
       audience: opts.audience,

packages/core/src/handlers/switchOrganizationHandler.ts

@@ -1,5 +1,8 @@
 import { authFetch } from "../authFetch.js";
-import { issueSessionCookies } from "../upstreamSession.js";
+import {
+  issueSessionCookies,
+  withoutSessionMaterial,
+} from "../upstreamSession.js";
 import { readPassthroughFailure } from "../upstreamError.js";
 import type { ResultFailure } from "../result.js";
 import type { CookiePayload } from "../ensureCookies.js";
@@ -61,7 +64,7 @@ export async function switchOrganizationHandler(
 
   return {
     status: up.status,
-    body: data,
+    body: withoutSessionMaterial(data),
     setCookies: await issueSessionCookies(data, {
       authServerUrl: opts.authServerUrl,
       audience: opts.audience,

packages/core/src/handlers/verifyLoginOtpHandler.ts

@@ -1,5 +1,8 @@
 import { authFetch } from "../authFetch.js";
-import { issueSessionCookies } from "../upstreamSession.js";
+import {
+  issueSessionCookies,
+  withoutSessionMaterial,
+} from "../upstreamSession.js";
 import { readPassthroughFailure } from "../upstreamError.js";
 import type { ResultFailure } from "../result.js";
 import type { CookiePayload } from "../ensureCookies.js";
@@ -67,7 +70,7 @@ async function verifyOtp(
 
   return {
     status: up.status,
-    body: data,
+    body: withoutSessionMaterial(data),
     setCookies: await issueSessionCookies(data, {
       authServerUrl: opts.authServerUrl,
       audience: opts.audience,

packages/core/src/upstreamSession.ts

@@ -25,6 +25,26 @@ export interface UpstreamSessionResponse {
   [key: string]: unknown;
 }
 
+/**
+ * The response body minus the credentials the cookies now carry.
+ *
+ * The access and refresh tokens go out as `httpOnly` cookies precisely so that
+ * page scripts cannot read them. Returning the upstream body unchanged handed the
+ * same two values straight back to the caller in the response that set those
+ * cookies, which put them everywhere a body goes and a cookie does not: a devtools
+ * export, a service worker, an APM tool that records payloads, a proxy logging
+ * bodies.
+ *
+ * Everything else survives. Callers read `message`, `email`, `roles`,
+ * `organizationId` and `returnTo` off these responses, so this removes the
+ * credentials rather than the body.
+ */
+export function withoutSessionMaterial<T extends UpstreamSessionResponse>(data: T) {
+  const { token: _token, refreshToken: _refreshToken, ...rest } = data;
+
+  return rest;
+}
+
 export interface VerifiedUpstreamSession {
   /** The `sid` claim, when the access token carries one. */
   sessionId?: string;

packages/core/tests/sessionBodyMaterial.test.js

@@ -0,0 +1,132 @@
+import { jest } from "@jest/globals";
+
+const verifySignedAuthResponseMock = jest.fn();
+
+jest.unstable_mockModule("../dist/verifySignedAuthResponse.js", () => ({
+  verifySignedAuthResponse: verifySignedAuthResponseMock,
+}));
+
+function createJsonResponse(status, body) {
+  return {
+    ok: status >= 200 && status < 300,
+    status,
+    json: async () => body,
+  };
+}
+
+const UPSTREAM = {
+  message: "Success",
+  token: "access-token",
+  refreshToken: "refresh-token",
+  sub: "user-123",
+  roles: ["user"],
+  email: "person@example.com",
+  phone: null,
+  organizationId: "org-123",
+  returnTo: "https://app.example.com/dashboard",
+  ttl: 900,
+  refreshTtl: 3600,
+};
+
+const OPTS = {
+  authServerUrl: "https://auth.example.com",
+  audience: "https://auth.example.com",
+  accessCookieName: "access",
+  refreshCookieName: "refresh",
+};
+
+/**
+ * Every handler that issues session cookies, and how to call it.
+ *
+ * Enumerated in one place so a handler added later is covered by adding a line
+ * rather than by remembering this invariant exists.
+ */
+const SESSION_HANDLERS = [
+  {
+    name: "finishLoginHandler",
+    module: "../dist/handlers/finishLogin.js",
+    input: { body: {} },
+  },
+  {
+    name: "verifyLoginOtpHandler",
+    module: "../dist/handlers/verifyLoginOtpHandler.js",
+    input: { body: {}, channel: "email" },
+  },
+  {
+    name: "finishOAuthLoginHandler",
+    module: "../dist/handlers/oauthHandlers.js",
+    input: { providerId: "google", body: {} },
+  },
+  {
+    name: "pollMagicLinkConfirmationHandler",
+    module: "../dist/handlers/pollMagicLinkConfirmationHandler.js",
+    input: { body: {} },
+  },
+  {
+    name: "switchOrganizationHandler",
+    module: "../dist/handlers/switchOrganizationHandler.js",
+    input: { organizationId: "org-123" },
+  },
+];
+
+describe("session responses do not repeat the credentials in the body", () => {
+  const originalFetch = global.fetch;
+
+  beforeEach(() => {
+    global.fetch = jest.fn();
+    verifySignedAuthResponseMock.mockReset();
+    verifySignedAuthResponseMock.mockResolvedValue({
+      sub: "user-123",
+      sid: "session-123",
+    });
+  });
+
+  afterEach(() => {
+    global.fetch = originalFetch;
+  });
+
+  // The tokens go out as httpOnly cookies so page scripts cannot read them.
+  // Returning them in the body of the same response handed them straight back.
+  it.each(SESSION_HANDLERS)("$name keeps the tokens out of the body", async entry => {
+    const module = await import(entry.module);
+    global.fetch.mockResolvedValue(createJsonResponse(200, UPSTREAM));
+
+    const result = await module[entry.name](entry.input, OPTS);
+
+    expect(result.body).toBeDefined();
+    expect(result.body).not.toHaveProperty("token");
+    expect(result.body).not.toHaveProperty("refreshToken");
+    expect(JSON.stringify(result.body)).not.toContain("access-token");
+    expect(JSON.stringify(result.body)).not.toContain("refresh-token");
+  });
+
+  it.each(SESSION_HANDLERS)("$name still carries what callers read", async entry => {
+    const module = await import(entry.module);
+    global.fetch.mockResolvedValue(createJsonResponse(200, UPSTREAM));
+
+    const result = await module[entry.name](entry.input, OPTS);
+
+    expect(result.body).toMatchObject({
+      message: "Success",
+      email: "person@example.com",
+      organizationId: "org-123",
+      returnTo: "https://app.example.com/dashboard",
+    });
+  });
+
+  it.each(SESSION_HANDLERS)("$name still puts the access token in a cookie", async entry => {
+    const module = await import(entry.module);
+    global.fetch.mockResolvedValue(createJsonResponse(200, UPSTREAM));
+
+    const result = await module[entry.name](entry.input, OPTS);
+
+    expect(result.setCookies).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          name: "access",
+          value: expect.objectContaining({ token: "access-token" }),
+        }),
+      ]),
+    );
+  });
+});