Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/cookie-transport-ephemeral-tokens.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
"@seamless-auth/core": patch
"@seamless-auth/express": patch
"@seamless-auth/fastify": patch
"@seamless-auth/nextjs": patch
---

Keep ephemeral tokens out of cookie-transport response bodies (#202).

The OTP send routes (`POST /otp/generate-email-otp`, `/otp/generate-phone-otp`, `/otp/generate-login-email-otp`, `/otp/generate-login-phone-otp`) returned the ephemeral token the auth API re-mints on every send, and `POST /registration/register` returned the registration token it had just stored in the cookie. Page scripts could read both, which the httpOnly cookie exists to prevent. Under cookie transport these bodies no longer carry `token` or `refreshToken`. Bearer transport is unchanged, because the client holds its own tokens there.

`requestOtpHandler` takes an optional `transport`. Without it the token is dropped, so a caller serving bearer clients through this handler directly should pass `transport: "bearer"`.
22 changes: 22 additions & 0 deletions packages/core/src/bodyTokens.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* A response body with the auth API's tokens removed.
*
* In cookie transport the cookies carry every token, so the browser must never
* see one in a body: a page script could read it, and bodies end up where cookies
* do not (devtools exports, APM payload capture, service workers, proxy logs).
* That covers ephemeral tokens as well as sessions, because the auth API also
* returns the token it re-mints on an OTP send and the one it issues on
* registration.
*/
export function withoutTokens(data: unknown): unknown {
if (typeof data !== "object" || data === null || Array.isArray(data)) {
return data;
}

const { token: _token, refreshToken: _refreshToken, ...rest } = data as Record<
string,
unknown
>;

return rest;
}
3 changes: 2 additions & 1 deletion packages/core/src/handlers/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { EXTERNAL_DELIVERY_HEADERS } from "../apiContract.js";
import type { ResultFailure } from "../result.js";
import type { CookiePayload } from "../ensureCookies.js";
import type { AuthTransport } from "../transport.js";
import { withoutTokens } from "../bodyTokens.js";

export interface RegisterInput {
body: unknown;
Expand Down Expand Up @@ -63,7 +64,7 @@ export async function registerHandler(

return {
status: 200,
body: data,
body: withoutTokens(data),
setCookies: [
{
name: opts.registrationCookieName,
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/handlers/requestOtpHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { authFetch } from "../authFetch.js";
import { readPassthroughFailure } from "../upstreamError.js";
import { EXTERNAL_DELIVERY_HEADERS } from "../apiContract.js";
import type { ResultFailure } from "../result.js";
import type { AuthTransport } from "../transport.js";
import { withoutTokens } from "../bodyTokens.js";

export interface RequestOtpInput {
authorization?: string;
Expand All @@ -15,6 +17,8 @@ export interface RequestOtpOptions {
forwardedClientIp?: string;
forwardedUserAgent?: string;
serviceAuthorization?: string;
/** Cookie transport (the default) drops the re-minted token from the body. */
transport?: AuthTransport;
}

export interface RequestOtpResult extends ResultFailure {
Expand Down Expand Up @@ -60,6 +64,6 @@ export async function requestOtpHandler(

return {
status: up.status,
body: data,
body: opts.transport === "bearer" ? data : withoutTokens(data),
};
}
22 changes: 6 additions & 16 deletions packages/core/src/manifestProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { authFetch } from "./authFetch.js";
import type { AuthServerIssuerOption } from "./authServerIssuer.js";
import type { SeamlessAuthMessagingOptions } from "./authMessaging.js";
import { EXTERNAL_DELIVERY_HEADERS } from "./apiContract.js";
import { withoutTokens } from "./bodyTokens.js";
import { applyExternalDelivery } from "./deliverAuthMessage.js";
import {
type AdapterHeld,
Expand Down Expand Up @@ -80,24 +81,13 @@ function isUpstreamSession(value: unknown): value is UpstreamSessionResponse {
);
}

/**
* The body a cookie-transport caller receives.
*
* The browser never sees a token: the cookies carry them. That holds for every
* route, not only session-issuing ones, because the auth API also returns the
* ephemeral token it re-mints on an OTP send.
*/
/** The body a cookie-transport caller receives: a `pick`, or everything but tokens. */
function cookieTransportBody(route: AdapterManifestRoute, data: unknown) {
if (!isObject(data)) return data;
if (!isObject(data) || !route.body) return withoutTokens(data);

if (route.body) {
return Object.fromEntries(
route.body.pick.filter((key) => key in data).map((key) => [key, data[key]]),
);
}

const { token: _token, refreshToken: _refreshToken, ...rest } = data;
return rest;
return Object.fromEntries(
route.body.pick.filter((key) => key in data).map((key) => [key, data[key]]),
);
}

async function cookiesFor(
Expand Down
42 changes: 42 additions & 0 deletions packages/core/tests/registerHandler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,46 @@ describe("registerHandler", () => {
},
]);
});

const REGISTERED = {
message: "Success",
sub: "user-123",
token: "ephemeral-token",
ttl: 300,
};

// The cookie holds the registration token. Returning it in the body as well
// let page scripts read the token the httpOnly cookie exists to hide (#202).
it("keeps the registration token out of a cookie-transport body", async () => {
const { registerHandler } = await import("../dist/handlers/register.js");
global.fetch.mockResolvedValue(createJsonResponse(200, REGISTERED));

const result = await registerHandler(
{ body: { email: "user@example.com" } },
{
authServerUrl: "https://auth.example.com",
registrationCookieName: "registration",
},
);

expect(result.body).toEqual({ message: "Success", sub: "user-123", ttl: 300 });
expect(result.setCookies[0].value.token).toBe("ephemeral-token");
});

it("returns the whole body in bearer transport", async () => {
const { registerHandler } = await import("../dist/handlers/register.js");
global.fetch.mockResolvedValue(createJsonResponse(200, REGISTERED));

const result = await registerHandler(
{ body: { email: "user@example.com" } },
{
authServerUrl: "https://auth.example.com",
registrationCookieName: "registration",
transport: "bearer",
},
);

expect(result.body).toEqual(REGISTERED);
expect(result.setCookies).toBeUndefined();
});
});
67 changes: 67 additions & 0 deletions packages/core/tests/requestOtpHandler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,71 @@ describe("requestOtpHandler", () => {
expect.objectContaining({ method: "GET" }),
);
});

// The auth API re-mints the caller's ephemeral token on every send. The cookie
// already holds one, and page scripts must not be able to read it (#202).
it("drops the re-minted token from the body by default", async () => {
const { requestOtpHandler } = await import(
"../dist/handlers/requestOtpHandler.js"
);
global.fetch.mockResolvedValue(
jsonResponse(200, { message: "success", token: "re-minted" }),
);

const result = await requestOtpHandler(
{ kind: "email", flow: "login", authorization: "Bearer pre-auth" },
{ authServerUrl: "https://auth.example.com", transport: "cookie" },
);

expect(result.body).toEqual({ message: "success" });
});

it("drops it when no transport is given", async () => {
const { requestOtpHandler } = await import(
"../dist/handlers/requestOtpHandler.js"
);
global.fetch.mockResolvedValue(
jsonResponse(200, { message: "success", token: "re-minted" }),
);

const result = await requestOtpHandler(
{ kind: "email", authorization: "Bearer pre-auth" },
{ authServerUrl: "https://auth.example.com" },
);

expect(result.body).toEqual({ message: "success" });
});

it("keeps it for a bearer client, which holds its own token", async () => {
const { requestOtpHandler } = await import(
"../dist/handlers/requestOtpHandler.js"
);
global.fetch.mockResolvedValue(
jsonResponse(200, { message: "success", token: "re-minted" }),
);

const result = await requestOtpHandler(
{ kind: "email", flow: "login", authorization: "Bearer pre-auth" },
{ authServerUrl: "https://auth.example.com", transport: "bearer" },
);

expect(result.body).toEqual({ message: "success", token: "re-minted" });
});

it("keeps the delivery payload for the adapter to send", async () => {
const { requestOtpHandler } = await import(
"../dist/handlers/requestOtpHandler.js"
);
const delivery = { kind: "otp_email", to: "a@b.c", token: "123456" };
global.fetch.mockResolvedValue(
jsonResponse(200, { message: "success", token: "re-minted", delivery }),
);

const result = await requestOtpHandler(
{ kind: "email", authorization: "Bearer pre-auth" },
{ authServerUrl: "https://auth.example.com", externalDelivery: true },
);

expect(result.body).toEqual({ message: "success", delivery });
});
});
2 changes: 2 additions & 0 deletions packages/express/src/handlers/requestOtp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from "../internal/buildAuthorization";
import { buildForwardedClientIp } from "../internal/buildForwardedClientIp";
import { buildForwardedUserAgent } from "../internal/buildForwardedUserAgent";
import { transportOf } from "../internal/transport";
import { applyExternalDelivery } from "@seamless-auth/core";
import { SeamlessAuthServerOptions } from "../createServer";

Expand All @@ -26,6 +27,7 @@ export async function requestOtp(
},
{
authServerUrl: opts.authServerUrl,
transport: transportOf(req),
externalDelivery: Boolean(opts.messaging),
forwardedClientIp: buildForwardedClientIp(req, opts.resolveClientIp),
forwardedUserAgent: buildForwardedUserAgent(req),
Expand Down
1 change: 1 addition & 0 deletions packages/fastify/src/routes/authRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ export function registerAuthRoutes(
{ kind, flow, authorization: buildServiceAuthorization(req) },
{
...common(req),
transport: transportOf(req),
externalDelivery: Boolean(opts.messaging),
serviceAuthorization: deliveryServiceAuthorization(),
},
Expand Down
73 changes: 73 additions & 0 deletions packages/fastify/tests/parity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1200,3 +1200,76 @@ describe("fastify and express agree on routes served from the manifest", () => {
expect(fastify).toEqual(expressResult);
});
});

// The cookies carry every token, so none may reach the browser in a body (#202).
describe("no adapter returns an ephemeral token in a cookie-transport body", () => {
const originalFetch = global.fetch;
afterEach(() => {
global.fetch = originalFetch;
});

it.each([
"/otp/generate-email-otp",
"/otp/generate-phone-otp",
"/otp/generate-login-email-otp",
"/otp/generate-login-phone-otp",
])("%s", async (path) => {
const scenario = { method: "post", path, cookie: preAuthCookie(), payload: {} };
const sent = upstream(200, { message: "success", token: "re-minted" });

global.fetch = jest.fn(async () => sent);
const fastify = await viaFastify(scenario);
global.fetch = jest.fn(async () => sent);
const expressResult = await viaExpress(scenario);

expect(fastify.status).toBe(200);
expect(fastify.body).toEqual({ message: "success" });
expect(expressResult.body).toEqual(fastify.body);
});

it("/registration/register", async () => {
const scenario = {
method: "post",
path: "/registration/register",
payload: { email: "user@example.com" },
};
const registered = upstream(200, {
message: "Success",
sub: "user-123",
token: "registration-token",
ttl: 300,
});

global.fetch = jest.fn(async () => registered);
const fastify = await viaFastify(scenario);
global.fetch = jest.fn(async () => registered);
const expressResult = await viaExpress(scenario);

expect(fastify.status).toBe(200);
expect(fastify.body).toEqual({ message: "Success", sub: "user-123", ttl: 300 });
expect(expressResult.body).toEqual(fastify.body);
expect(fastify.cookies).toEqual(expressResult.cookies);
expect(fastify.cookies).toEqual([expect.stringMatching(/^seamless-ephemeral=<signed>/)]);
});

it("still returns the token to a bearer client", async () => {
const scenario = {
method: "post",
path: "/otp/generate-login-email-otp",
headers: {
"x-seamless-auth-transport": "bearer",
authorization: "Bearer client-pre-auth",
},
payload: {},
};
const sent = upstream(200, { message: "success", token: "re-minted" });

global.fetch = jest.fn(async () => sent);
const fastify = await viaFastify(scenario);
global.fetch = jest.fn(async () => sent);
const expressResult = await viaExpress(scenario);

expect(fastify.body).toEqual({ message: "success", token: "re-minted" });
expect(expressResult.body).toEqual(fastify.body);
});
});
1 change: 1 addition & 0 deletions packages/nextjs/src/routes/authRoutes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ const otpRequestRoutes: Route[] = (
{ kind, flow, authorization: buildServiceAuthorization(ctx) },
{
...common(ctx, opts),
transport: ctx.transport,
externalDelivery: Boolean(opts.messaging),
serviceAuthorization: deliveryServiceAuthorization(opts),
},
Expand Down
Loading
Loading