From 30dcedfa59c25755def8feececd5d3164b700b79 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Wed, 29 Jul 2026 23:40:38 -0400 Subject: [PATCH 1/2] refactor(core): extract the copy-pasted session helpers Seven handlers repeated the same block: verify the signed access token, check it describes the same subject as the response body, read the sid claim, then build the access and refresh cookie payloads. Seven copies of a security-relevant check is seven places to get an early return wrong, and the upstream fields were read untyped, so a rename on the API side surfaced as an undefined cookie field at runtime rather than a type error. issueSessionCookies does the whole thing in one call, verifyUpstreamSession covers the one flow that verifies without issuing a session, and UpstreamSessionResponse states what the API returns. The handlers drop 220 lines. The access cookie from webAuthn/register/finish now carries organizationId: null where it previously omitted the key. Every other flow already included it, and the refresh path writes it on every reissue, so that session disagreed with itself after one refresh. Closes #136 --- .changeset/extract-session-helpers.md | 15 ++ packages/core/src/handlers/finishLogin.ts | 53 +----- packages/core/src/handlers/finishRegister.ts | 49 +----- packages/core/src/handlers/login.ts | 18 +- packages/core/src/handlers/oauthHandlers.ts | 53 +----- .../pollMagicLinkConfirmationHandler.ts | 53 +----- .../src/handlers/switchOrganizationHandler.ts | 43 +---- .../src/handlers/verifyLoginOtpHandler.ts | 53 +----- packages/core/src/index.ts | 2 + packages/core/src/upstreamSession.ts | 117 +++++++++++++ packages/core/tests/upstreamSession.test.js | 163 ++++++++++++++++++ 11 files changed, 348 insertions(+), 271 deletions(-) create mode 100644 .changeset/extract-session-helpers.md create mode 100644 packages/core/src/upstreamSession.ts create mode 100644 packages/core/tests/upstreamSession.test.js diff --git a/.changeset/extract-session-helpers.md b/.changeset/extract-session-helpers.md new file mode 100644 index 0000000..c01060b --- /dev/null +++ b/.changeset/extract-session-helpers.md @@ -0,0 +1,15 @@ +--- +"@seamless-auth/core": patch +--- + +Extract the copy-pasted session helpers, with a typed `UpstreamSessionResponse`. + +Seven handlers repeated the same block: verify the signed access token, check it describes the same subject as the response body, read the `sid` claim, then build the access and refresh cookie payloads. Seven copies of a security-relevant check is seven places to get an early return wrong, and the upstream fields were read untyped, so a rename on the API side surfaced as an undefined cookie field at runtime rather than a type error. + +`issueSessionCookies` now does the whole thing in one call, `verifyUpstreamSession` is available for the one flow that verifies without issuing a session (login, which issues only the pre-auth cookie), and `UpstreamSessionResponse` states what the auth API returns when it issues a session. The handlers drop 220 lines. + +One behavior change: the access cookie issued by `POST /webAuthn/register/finish` now carries `organizationId: null` where it previously omitted the key. Every other flow already included it, and the refresh path writes it on every reissue, so that session disagreed with itself after a single refresh. It is now consistent from the start. + +New exports: `issueSessionCookies`, `verifyUpstreamSession`, `UpstreamSessionResponse`, `VerifiedUpstreamSession`, `IssueSessionCookiesOptions`. + +Closes #136. diff --git a/packages/core/src/handlers/finishLogin.ts b/packages/core/src/handlers/finishLogin.ts index a8e8451..e5f4e09 100644 --- a/packages/core/src/handlers/finishLogin.ts +++ b/packages/core/src/handlers/finishLogin.ts @@ -1,8 +1,8 @@ import { authFetch } from "../authFetch.js"; +import { issueSessionCookies } from "../upstreamSession.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; import type { CookiePayload } from "../ensureCookies.js"; -import { verifySignedAuthResponse } from "../verifySignedAuthResponse.js"; export interface FinishLoginInput { body: unknown; @@ -51,52 +51,15 @@ export async function finishLoginHandler( }; } - const verifiedAccessToken = await verifySignedAuthResponse( - data.token, - opts.authServerUrl, - opts.audience, - ); - - if (!verifiedAccessToken) { - throw new Error("Invalid signed response from Auth Server"); - } - - if (verifiedAccessToken.sub !== data.sub) { - throw new Error("Signature mismatch with data payload"); - } - - const sessionId = - typeof verifiedAccessToken.sid === "string" - ? verifiedAccessToken.sid - : undefined; - return { status: 200, body: data, - setCookies: [ - { - name: opts.accessCookieName, - value: { - sub: data.sub, - ...(sessionId === undefined ? {} : { sessionId }), - token: data.token, - roles: data.roles, - email: data.email, - phone: data.phone, - organizationId: data.organizationId ?? null, - }, - ttl: data.ttl, - domain: opts.cookieDomain, - }, - { - name: opts.refreshCookieName, - value: { - sub: data.sub, - refreshToken: data.refreshToken, - }, - ttl: data.refreshTtl, - domain: opts.cookieDomain, - }, - ], + setCookies: await issueSessionCookies(data, { + authServerUrl: opts.authServerUrl, + audience: opts.audience, + accessCookieName: opts.accessCookieName, + refreshCookieName: opts.refreshCookieName, + cookieDomain: opts.cookieDomain, + }), }; } diff --git a/packages/core/src/handlers/finishRegister.ts b/packages/core/src/handlers/finishRegister.ts index 9b6ce1b..83c653b 100644 --- a/packages/core/src/handlers/finishRegister.ts +++ b/packages/core/src/handlers/finishRegister.ts @@ -1,8 +1,8 @@ import { authFetch } from "../authFetch.js"; +import { issueSessionCookies } from "../upstreamSession.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; import type { CookiePayload } from "../ensureCookies.js"; -import { verifySignedAuthResponse } from "../verifySignedAuthResponse.js"; export interface FinishRegisterInput { authorization?: string; @@ -52,47 +52,14 @@ export async function finishRegisterHandler( }; } - const verified = await verifySignedAuthResponse( - data.token, - opts.authServerUrl, - opts.audience, - ); - - if (!verified) { - throw new Error("Invalid signed response from Auth Server"); - } - - if (verified.sub !== data.sub) { - throw new Error("Signature mismatch with data payload"); - } - - const sessionId = typeof verified.sid === "string" ? verified.sid : undefined; - return { status: 204, - setCookies: [ - { - name: opts.accessCookieName, - value: { - sub: data.sub, - ...(sessionId === undefined ? {} : { sessionId }), - token: data.token, - roles: data.roles, - email: data.email, - phone: data.phone, - }, - ttl: data.ttl, - domain: opts.cookieDomain, - }, - { - name: opts.refreshCookieName, - value: { - sub: data.sub, - refreshToken: data.refreshToken, - }, - ttl: data.refreshTtl, - domain: opts.cookieDomain, - }, - ], + setCookies: await issueSessionCookies(data, { + authServerUrl: opts.authServerUrl, + audience: opts.audience, + accessCookieName: opts.accessCookieName, + refreshCookieName: opts.refreshCookieName, + cookieDomain: opts.cookieDomain, + }), }; } diff --git a/packages/core/src/handlers/login.ts b/packages/core/src/handlers/login.ts index 5cb54f3..a1ee054 100644 --- a/packages/core/src/handlers/login.ts +++ b/packages/core/src/handlers/login.ts @@ -2,7 +2,7 @@ import { authFetch } from "../authFetch.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; import type { CookiePayload } from "../ensureCookies.js"; -import { verifySignedAuthResponse } from "../verifySignedAuthResponse.js"; +import { verifyUpstreamSession } from "../upstreamSession.js"; export interface LoginInput { body: unknown; @@ -52,19 +52,9 @@ export async function loginHandler( }; } - const verified = await verifySignedAuthResponse( - data.token, - opts.authServerUrl, - opts.audience, - ); - - if (!verified) { - throw new Error("Invalid signed response from Auth Server"); - } - - if (verified.sub !== data.sub) { - throw new Error("Signature mismatch with data payload"); - } + // Login issues only the pre-auth cookie, so it verifies the response without + // building session cookies from it. + await verifyUpstreamSession(data, opts.authServerUrl, opts.audience); const body = { ...(typeof data.message === "string" ? { message: data.message } : {}), diff --git a/packages/core/src/handlers/oauthHandlers.ts b/packages/core/src/handlers/oauthHandlers.ts index 3c7e924..77522a8 100644 --- a/packages/core/src/handlers/oauthHandlers.ts +++ b/packages/core/src/handlers/oauthHandlers.ts @@ -1,8 +1,8 @@ import { authFetch } from "../authFetch.js"; +import { issueSessionCookies } from "../upstreamSession.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; import type { CookiePayload } from "../ensureCookies.js"; -import { verifySignedAuthResponse } from "../verifySignedAuthResponse.js"; export interface OAuthHandlerOptions { authServerUrl: string; @@ -90,52 +90,15 @@ export async function finishOAuthLoginHandler( }; } - const verifiedAccessToken = await verifySignedAuthResponse( - data.token, - opts.authServerUrl, - opts.audience, - ); - - if (!verifiedAccessToken) { - throw new Error("Invalid signed response from Auth Server"); - } - - if (verifiedAccessToken.sub !== data.sub) { - throw new Error("Signature mismatch with data payload"); - } - - const sessionId = - typeof verifiedAccessToken.sid === "string" - ? verifiedAccessToken.sid - : undefined; - return { status: up.status, body: data, - setCookies: [ - { - name: opts.accessCookieName, - value: { - sub: data.sub, - ...(sessionId === undefined ? {} : { sessionId }), - token: data.token, - roles: data.roles, - email: data.email, - phone: data.phone, - organizationId: data.organizationId ?? null, - }, - ttl: data.ttl, - domain: opts.cookieDomain, - }, - { - name: opts.refreshCookieName, - value: { - sub: data.sub, - refreshToken: data.refreshToken, - }, - ttl: data.refreshTtl, - domain: opts.cookieDomain, - }, - ], + setCookies: await issueSessionCookies(data, { + authServerUrl: opts.authServerUrl, + audience: opts.audience, + accessCookieName: opts.accessCookieName, + refreshCookieName: opts.refreshCookieName, + cookieDomain: opts.cookieDomain, + }), }; } diff --git a/packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts b/packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts index 80d974c..1b3c749 100644 --- a/packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts +++ b/packages/core/src/handlers/pollMagicLinkConfirmationHandler.ts @@ -1,8 +1,8 @@ import { authFetch } from "../authFetch.js"; +import { issueSessionCookies } from "../upstreamSession.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; import type { CookiePayload } from "../ensureCookies.js"; -import { verifySignedAuthResponse } from "../verifySignedAuthResponse.js"; export interface PollMagicLinkConfirmationInput { authorization?: string; @@ -62,52 +62,15 @@ export async function pollMagicLinkConfirmationHandler( }; } - const verifiedAccessToken = await verifySignedAuthResponse( - data.token, - opts.authServerUrl, - opts.audience, - ); - - if (!verifiedAccessToken) { - throw new Error("Invalid signed response from Auth Server"); - } - - if (verifiedAccessToken.sub !== data.sub) { - throw new Error("Signature mismatch with data payload"); - } - - const sessionId = - typeof verifiedAccessToken.sid === "string" - ? verifiedAccessToken.sid - : undefined; - return { status: 200, body: data, - setCookies: [ - { - name: opts.accessCookieName, - value: { - sub: data.sub, - ...(sessionId === undefined ? {} : { sessionId }), - token: data.token, - roles: data.roles, - email: data.email, - phone: data.phone, - organizationId: data.organizationId ?? null, - }, - ttl: data.ttl, - domain: opts.cookieDomain, - }, - { - name: opts.refreshCookieName, - value: { - sub: data.sub, - refreshToken: data.refreshToken, - }, - ttl: data.refreshTtl, - domain: opts.cookieDomain, - }, - ], + setCookies: await issueSessionCookies(data, { + authServerUrl: opts.authServerUrl, + audience: opts.audience, + accessCookieName: opts.accessCookieName, + refreshCookieName: opts.refreshCookieName, + cookieDomain: opts.cookieDomain, + }), }; } diff --git a/packages/core/src/handlers/switchOrganizationHandler.ts b/packages/core/src/handlers/switchOrganizationHandler.ts index ecaa57a..b32ffc2 100644 --- a/packages/core/src/handlers/switchOrganizationHandler.ts +++ b/packages/core/src/handlers/switchOrganizationHandler.ts @@ -1,8 +1,8 @@ import { authFetch } from "../authFetch.js"; +import { issueSessionCookies } from "../upstreamSession.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; import type { CookiePayload } from "../ensureCookies.js"; -import { verifySignedAuthResponse } from "../verifySignedAuthResponse.js"; export interface SwitchOrganizationInput { organizationId: string; @@ -59,43 +59,14 @@ export async function switchOrganizationHandler( }; } - const verifiedAccessToken = await verifySignedAuthResponse( - data.token, - opts.authServerUrl, - opts.audience, - ); - - if (!verifiedAccessToken) { - throw new Error("Invalid signed response from Auth Server"); - } - - if (verifiedAccessToken.sub !== data.sub) { - throw new Error("Signature mismatch with data payload"); - } - - const sessionId = - typeof verifiedAccessToken.sid === "string" - ? verifiedAccessToken.sid - : undefined; - return { status: up.status, body: data, - setCookies: [ - { - name: opts.accessCookieName, - value: { - sub: data.sub, - ...(sessionId === undefined ? {} : { sessionId }), - token: data.token, - roles: data.roles, - email: data.email, - phone: data.phone, - organizationId: data.organizationId ?? null, - }, - ttl: data.ttl, - domain: opts.cookieDomain, - }, - ], + setCookies: await issueSessionCookies(data, { + authServerUrl: opts.authServerUrl, + audience: opts.audience, + accessCookieName: opts.accessCookieName, + cookieDomain: opts.cookieDomain, + }), }; } diff --git a/packages/core/src/handlers/verifyLoginOtpHandler.ts b/packages/core/src/handlers/verifyLoginOtpHandler.ts index 10da93a..46cf0a7 100644 --- a/packages/core/src/handlers/verifyLoginOtpHandler.ts +++ b/packages/core/src/handlers/verifyLoginOtpHandler.ts @@ -1,8 +1,8 @@ import { authFetch } from "../authFetch.js"; +import { issueSessionCookies } from "../upstreamSession.js"; import { readPassthroughFailure } from "../upstreamError.js"; import type { ResultFailure } from "../result.js"; import type { CookiePayload } from "../ensureCookies.js"; -import { verifySignedAuthResponse } from "../verifySignedAuthResponse.js"; export interface VerifyLoginOtpInput { body: unknown; @@ -65,53 +65,16 @@ async function verifyOtp( }; } - const verifiedAccessToken = await verifySignedAuthResponse( - data.token, - opts.authServerUrl, - opts.audience, - ); - - if (!verifiedAccessToken) { - throw new Error("Invalid signed response from Auth Server"); - } - - if (verifiedAccessToken.sub !== data.sub) { - throw new Error("Signature mismatch with data payload"); - } - - const sessionId = - typeof verifiedAccessToken.sid === "string" - ? verifiedAccessToken.sid - : undefined; - return { status: up.status, body: data, - setCookies: [ - { - name: opts.accessCookieName, - value: { - sub: data.sub, - ...(sessionId === undefined ? {} : { sessionId }), - token: data.token, - roles: data.roles, - email: data.email, - phone: data.phone, - organizationId: data.organizationId ?? null, - }, - ttl: data.ttl, - domain: opts.cookieDomain, - }, - { - name: opts.refreshCookieName, - value: { - sub: data.sub, - refreshToken: data.refreshToken, - }, - ttl: data.refreshTtl, - domain: opts.cookieDomain, - }, - ], + setCookies: await issueSessionCookies(data, { + authServerUrl: opts.authServerUrl, + audience: opts.audience, + accessCookieName: opts.accessCookieName, + refreshCookieName: opts.refreshCookieName, + cookieDomain: opts.cookieDomain, + }), }; } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7408d31..e5cb65a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -7,6 +7,7 @@ export * from "./verifyRefreshCookie.js"; export * from "./verifySignedAuthResponse.js"; export * from "./refreshAccessToken.js"; export * from "./getSeamlessUser.js"; +export * from "./logger.js"; export * from "./guards.js"; export * from "./createServiceToken.js"; export * from "./validateSecrets.js"; @@ -22,6 +23,7 @@ export * from "./apiContract.js"; export * from "./applyResult.js"; export * from "./proxyRequest.js"; export * from "./result.js"; +export * from "./upstreamSession.js"; export * from "./redaction.js"; export * from "./handlers/login.js"; diff --git a/packages/core/src/upstreamSession.ts b/packages/core/src/upstreamSession.ts new file mode 100644 index 0000000..fe3fd99 --- /dev/null +++ b/packages/core/src/upstreamSession.ts @@ -0,0 +1,117 @@ +import type { SessionCookie } from "./applyResult.js"; +import { verifySignedAuthResponse } from "./verifySignedAuthResponse.js"; + +/** + * What the auth API returns when it issues a session. + * + * The handlers read these fields off the parsed response, which was untyped + * before: a rename upstream showed up as an undefined cookie field at runtime + * rather than a type error here. + * + * The index signature is deliberate. Several handlers forward the whole body to + * the caller, so it carries more than the fields this package reads, and the + * type should not claim otherwise. + */ +export interface UpstreamSessionResponse { + sub: string; + token: string; + refreshToken?: string; + roles?: string[]; + email?: string; + phone?: string | null; + organizationId?: string | null; + ttl: number; + refreshTtl?: number; + [key: string]: unknown; +} + +export interface VerifiedUpstreamSession { + /** The `sid` claim, when the access token carries one. */ + sessionId?: string; +} + +/** + * Verifies the access token the auth API signed and confirms it describes the + * same subject as the response body. + * + * Throws rather than returning a failure: a response that fails either check is + * not a rejected login, it is a response this package cannot trust, and + * continuing would mint a session from it. + */ +export async function verifyUpstreamSession( + data: UpstreamSessionResponse, + authServerUrl: string, + audience: string, +): Promise { + const verified = await verifySignedAuthResponse( + data.token, + authServerUrl, + audience, + ); + + if (!verified) { + throw new Error("Invalid signed response from Auth Server"); + } + + if (verified.sub !== data.sub) { + throw new Error("Signature mismatch with data payload"); + } + + return { + sessionId: typeof verified.sid === "string" ? verified.sid : undefined, + }; +} + +export interface IssueSessionCookiesOptions { + authServerUrl: string; + audience: string; + accessCookieName: string; + /** Omit for a flow that reissues the access cookie without rotating refresh. */ + refreshCookieName?: string; + cookieDomain?: string; +} + +/** + * Verifies an upstream session response and builds the cookies for it. + * + * One place decides what a session cookie holds, so the flows that issue one + * cannot disagree about it. + */ +export async function issueSessionCookies( + data: UpstreamSessionResponse, + opts: IssueSessionCookiesOptions, +): Promise { + const { sessionId } = await verifyUpstreamSession( + data, + opts.authServerUrl, + opts.audience, + ); + + const cookies: SessionCookie[] = [ + { + name: opts.accessCookieName, + value: { + sub: data.sub, + ...(sessionId === undefined ? {} : { sessionId }), + token: data.token, + roles: data.roles, + email: data.email, + phone: data.phone, + organizationId: data.organizationId ?? null, + }, + ttl: data.ttl, + domain: opts.cookieDomain, + }, + ]; + + if (opts.refreshCookieName) { + cookies.push({ + name: opts.refreshCookieName, + value: { sub: data.sub, refreshToken: data.refreshToken }, + ttl: data.refreshTtl as number, + domain: opts.cookieDomain, + }); + } + + return cookies; +} diff --git a/packages/core/tests/upstreamSession.test.js b/packages/core/tests/upstreamSession.test.js new file mode 100644 index 0000000..36c47d1 --- /dev/null +++ b/packages/core/tests/upstreamSession.test.js @@ -0,0 +1,163 @@ +import { jest } from "@jest/globals"; + +const verifySignedAuthResponseMock = jest.fn(); + +jest.unstable_mockModule("../dist/verifySignedAuthResponse.js", () => ({ + verifySignedAuthResponse: verifySignedAuthResponseMock, +})); + +const { issueSessionCookies, verifyUpstreamSession } = await import( + "../dist/upstreamSession.js" +); + +const SESSION = { + sub: "user-1", + token: "access-token", + refreshToken: "refresh-token", + roles: ["admin"], + email: "user@acme.test", + phone: null, + organizationId: "org-1", + ttl: 300, + refreshTtl: 3600, +}; + +const OPTIONS = { + authServerUrl: "https://auth.test", + audience: "https://auth.test", + accessCookieName: "seamless-access", + refreshCookieName: "seamless-refresh", + cookieDomain: "acme.test", +}; + +beforeEach(() => verifySignedAuthResponseMock.mockReset()); + +describe("verifyUpstreamSession", () => { + it("returns the session id from the sid claim", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ + sub: "user-1", + sid: "s-1", + }); + + expect( + await verifyUpstreamSession(SESSION, "https://auth.test", "aud"), + ).toEqual({ sessionId: "s-1" }); + }); + + it("omits the session id when the claim is absent or not a string", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + expect( + (await verifyUpstreamSession(SESSION, "https://auth.test", "aud")) + .sessionId, + ).toBeUndefined(); + + verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1", sid: 7 }); + expect( + (await verifyUpstreamSession(SESSION, "https://auth.test", "aud")) + .sessionId, + ).toBeUndefined(); + }); + + // Neither is a rejected login. Both mean the response cannot be trusted, and + // continuing would mint a session from it. + it("throws on an unverifiable response", async () => { + verifySignedAuthResponseMock.mockResolvedValue(null); + + await expect( + verifyUpstreamSession(SESSION, "https://auth.test", "aud"), + ).rejects.toThrow("Invalid signed response from Auth Server"); + }); + + it("throws when the token describes a different subject", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ sub: "someone-else" }); + + await expect( + verifyUpstreamSession(SESSION, "https://auth.test", "aud"), + ).rejects.toThrow("Signature mismatch with data payload"); + }); + + it("verifies against the configured server and audience", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + + await verifyUpstreamSession(SESSION, "https://auth.test", "the-audience"); + + expect(verifySignedAuthResponseMock).toHaveBeenCalledWith( + "access-token", + "https://auth.test", + "the-audience", + ); + }); +}); + +describe("issueSessionCookies", () => { + it("builds the access and refresh cookies", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ + sub: "user-1", + sid: "s-1", + }); + + expect(await issueSessionCookies(SESSION, OPTIONS)).toEqual([ + { + name: "seamless-access", + value: { + sub: "user-1", + sessionId: "s-1", + token: "access-token", + roles: ["admin"], + email: "user@acme.test", + phone: null, + organizationId: "org-1", + }, + ttl: 300, + domain: "acme.test", + }, + { + name: "seamless-refresh", + value: { sub: "user-1", refreshToken: "refresh-token" }, + ttl: 3600, + domain: "acme.test", + }, + ]); + }); + + it("omits the session id key entirely when there is no sid", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + + const [access] = await issueSessionCookies(SESSION, OPTIONS); + + expect(access.value).not.toHaveProperty("sessionId"); + }); + + // The refresh path writes `organizationId: null` on every reissue, so a + // session that omitted it disagreed with itself after one refresh. + it("always carries an organization id, null when there is none", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + + const [access] = await issueSessionCookies( + { ...SESSION, organizationId: undefined }, + OPTIONS, + ); + + expect(access.value.organizationId).toBeNull(); + }); + + it("issues only the access cookie when no refresh cookie is named", async () => { + verifySignedAuthResponseMock.mockResolvedValue({ sub: "user-1" }); + + const cookies = await issueSessionCookies(SESSION, { + ...OPTIONS, + refreshCookieName: undefined, + }); + + expect(cookies).toHaveLength(1); + expect(cookies[0].name).toBe("seamless-access"); + }); + + it("does not build cookies from a response it could not verify", async () => { + verifySignedAuthResponseMock.mockResolvedValue(null); + + await expect(issueSessionCookies(SESSION, OPTIONS)).rejects.toThrow( + "Invalid signed response from Auth Server", + ); + }); +}); From 8d93829d196a0b43dcbc39d18418eb6062e9f983 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Wed, 29 Jul 2026 23:40:38 -0400 Subject: [PATCH 2/2] feat(core): let adopters route diagnostics away from console Core wrote to console directly, so an adopter could not capture, level, or silence its output. An adapter with its own logger still leaked core's lines to console, so one request could produce output in two places. setSeamlessLogger accepts anything with warn and error, which a platform logger already satisfies, and defaults back to console when called with nothing. Process-wide rather than per-request: it changes where the diagnostics go, not what context they carry. Core logs a failed signature verification and a misconfigured external-delivery setup, neither of which needs request context, and threading a logger through every handler would be a much larger change. Closes #137 --- .changeset/injectable-logger.md | 15 ++++ packages/core/src/deliverAuthMessage.ts | 19 +++- packages/core/src/ensureCookies.ts | 12 ++- packages/core/src/logger.ts | 38 ++++++++ packages/core/src/redaction.ts | 10 ++- packages/core/src/refreshAccessToken.ts | 5 +- packages/core/src/verifySignedAuthResponse.ts | 5 +- packages/core/tests/logger.test.js | 87 +++++++++++++++++++ 8 files changed, 181 insertions(+), 10 deletions(-) create mode 100644 .changeset/injectable-logger.md create mode 100644 packages/core/src/logger.ts create mode 100644 packages/core/tests/logger.test.js diff --git a/.changeset/injectable-logger.md b/.changeset/injectable-logger.md new file mode 100644 index 0000000..90b1884 --- /dev/null +++ b/.changeset/injectable-logger.md @@ -0,0 +1,15 @@ +--- +"@seamless-auth/core": patch +--- + +Let adopters route this package's diagnostics somewhere other than `console`. + +Core wrote to `console` directly, so an adopter could not capture, level, or silence its output. An adapter with its own logger still leaked core's lines to `console`, which meant one request could produce output in two places. + +`setSeamlessLogger(logger)` accepts anything with `warn` and `error`, which a platform logger already satisfies, and `setSeamlessLogger()` with no argument goes back to `console`. Nothing changes for callers that do not set one. + +The logger is process-wide, not per-request, so it changes where core's diagnostics go rather than attaching request context to them. Core logs two things (a failed signature verification and a misconfigured external-delivery setup), and neither depends on request context. Threading a per-request logger through every handler would be a larger change. + +New exports: `setSeamlessLogger`, `getSeamlessLogger`, `SeamlessLogger`. + +Closes #137. diff --git a/packages/core/src/deliverAuthMessage.ts b/packages/core/src/deliverAuthMessage.ts index c5ff204..f25a104 100644 --- a/packages/core/src/deliverAuthMessage.ts +++ b/packages/core/src/deliverAuthMessage.ts @@ -4,10 +4,15 @@ import type { SeamlessAuthMessagingOptions, SmsMessage, } from "./authMessaging.js"; +import { getSeamlessLogger } from "./logger.js"; function applyEmailOverride( override: - | ((input: TInput, defaults: EmailMessage, context: { appName?: string }) => EmailMessage) + | (( + input: TInput, + defaults: EmailMessage, + context: { appName?: string }, + ) => EmailMessage) | undefined, input: TInput, defaults: EmailMessage, @@ -18,7 +23,11 @@ function applyEmailOverride( function applySmsOverride( override: - | ((input: TInput, defaults: SmsMessage, context: { appName?: string }) => SmsMessage) + | (( + input: TInput, + defaults: SmsMessage, + context: { appName?: string }, + ) => SmsMessage) | undefined, input: TInput, defaults: SmsMessage, @@ -163,7 +172,9 @@ export async function deliverAuthMessage( } } -export function stripDelivery(body: T): Omit { +export function stripDelivery( + body: T, +): Omit { const { delivery: _delivery, ...rest } = body; return rest; } @@ -189,7 +200,7 @@ export async function applyExternalDelivery( } if (messaging) { - console.warn( + getSeamlessLogger().warn( "[SeamlessAuth] External delivery was requested but the auth API returned no delivery payload, so no message was sent. Verify that serviceSecret matches the auth API's API_SERVICE_TOKEN.", ); } diff --git a/packages/core/src/ensureCookies.ts b/packages/core/src/ensureCookies.ts index fd3405f..0dddd85 100644 --- a/packages/core/src/ensureCookies.ts +++ b/packages/core/src/ensureCookies.ts @@ -303,7 +303,11 @@ export async function ensureCookies( const refreshCookie = input.cookies[opts.refreshCookieName]; if (required && !cookieValue) { - const refreshed = await refreshRequiredCookie(cookieName, refreshCookie, opts); + const refreshed = await refreshRequiredCookie( + cookieName, + refreshCookie, + opts, + ); if (!refreshed) { return { @@ -329,7 +333,11 @@ export async function ensureCookies( const token = typeof payload.token === "string" ? payload.token : undefined; if (required && !token && cookieName === opts.accessCookieName) { - const refreshed = await refreshRequiredCookie(cookieName, refreshCookie, opts); + const refreshed = await refreshRequiredCookie( + cookieName, + refreshCookie, + opts, + ); if (refreshed) { return refreshed; diff --git a/packages/core/src/logger.ts b/packages/core/src/logger.ts new file mode 100644 index 0000000..0c795d4 --- /dev/null +++ b/packages/core/src/logger.ts @@ -0,0 +1,38 @@ +/** + * Where this package writes its diagnostics. + * + * Only the two levels it actually uses. Keeping the interface at what is needed + * means an adopter can satisfy it with an object literal, and a platform logger + * (`pino`, `console`, a Fastify `request.log`) already does. + */ +export interface SeamlessLogger { + warn(message: string): void; + error(message: string): void; +} + +const consoleLogger: SeamlessLogger = { + warn: (message) => console.warn(message), + error: (message) => console.error(message), +}; + +let activeLogger: SeamlessLogger = consoleLogger; + +/** + * Routes this package's diagnostics somewhere other than `console`. + * + * Serverless and edge runtimes often ship logs through a platform logger, and a + * structured setup wants these as events rather than strings. Without this an + * adopter cannot capture, level, or silence them: an adapter that logs through + * its own logger still leaks core's lines to `console`, so one request produces + * output in two places. + * + * Pass nothing to go back to `console`. + */ +export function setSeamlessLogger(logger?: SeamlessLogger): void { + activeLogger = logger ?? consoleLogger; +} + +/** The logger in effect. Read per call, so a later swap takes effect. */ +export function getSeamlessLogger(): SeamlessLogger { + return activeLogger; +} diff --git a/packages/core/src/redaction.ts b/packages/core/src/redaction.ts index 99d507a..8c7a2ed 100644 --- a/packages/core/src/redaction.ts +++ b/packages/core/src/redaction.ts @@ -4,8 +4,14 @@ const SENSITIVE_TEXT_PATTERNS: Array<[RegExp, string]> = [ [/\b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}\b/gi, REDACTED], [/\b(Bearer\s+)[A-Za-z0-9._~+/=-]+/gi, `$1${REDACTED}`], [/(\/magic-link\/verify\/)[^/?#\s]+/gi, `$1${REDACTED}`], - [/([?&](?:token|bootstrapToken|state|code|salt)=)[^&#\s]+/gi, `$1${REDACTED}`], - [/\b((?:token|bootstrapToken|verificationToken|identifier|phone|state|code|secret|salt)\s*[:=]\s*)[^,&\s}]+/gi, `$1${REDACTED}`], + [ + /([?&](?:token|bootstrapToken|state|code|salt)=)[^&#\s]+/gi, + `$1${REDACTED}`, + ], + [ + /\b((?:token|bootstrapToken|verificationToken|identifier|phone|state|code|secret|salt)\s*[:=]\s*)[^,&\s}]+/gi, + `$1${REDACTED}`, + ], [/\b(client_secret=)[^&\s]+/gi, `$1${REDACTED}`], ]; diff --git a/packages/core/src/refreshAccessToken.ts b/packages/core/src/refreshAccessToken.ts index fefb388..812c262 100644 --- a/packages/core/src/refreshAccessToken.ts +++ b/packages/core/src/refreshAccessToken.ts @@ -26,7 +26,10 @@ type RefreshAccessTokenResult = { refreshTtl: number; }; -const inFlightRefreshes = new Map>(); +const inFlightRefreshes = new Map< + string, + Promise +>(); const recentRefreshResults = new Map< string, { result: RefreshAccessTokenResult; expiresAt: number } diff --git a/packages/core/src/verifySignedAuthResponse.ts b/packages/core/src/verifySignedAuthResponse.ts index 3a54ecd..1da495f 100644 --- a/packages/core/src/verifySignedAuthResponse.ts +++ b/packages/core/src/verifySignedAuthResponse.ts @@ -1,4 +1,5 @@ import { createRemoteJWKSet, jwtVerify } from "jose"; +import { getSeamlessLogger } from "./logger.js"; // jose caches keys and applies a refetch cooldown per JWKS instance, so the instance // must outlive a single call. Memoize per JWKS URL (one per auth server, so the map @@ -31,7 +32,9 @@ export async function verifySignedAuthResponse( return payload as T; } catch { - console.error("[SeamlessAuth] Failed to verify signed auth response."); + getSeamlessLogger().error( + "[SeamlessAuth] Failed to verify signed auth response.", + ); return null; } } diff --git a/packages/core/tests/logger.test.js b/packages/core/tests/logger.test.js new file mode 100644 index 0000000..acb5505 --- /dev/null +++ b/packages/core/tests/logger.test.js @@ -0,0 +1,87 @@ +import { jest } from "@jest/globals"; + +const { getSeamlessLogger, setSeamlessLogger } = await import( + "../dist/logger.js" +); +const { applyExternalDelivery } = await import("../dist/deliverAuthMessage.js"); + +function recorder() { + const warned = []; + const errored = []; + return { + warned, + errored, + warn: (m) => warned.push(m), + error: (m) => errored.push(m), + }; +} + +afterEach(() => setSeamlessLogger()); + +describe("setSeamlessLogger", () => { + it("defaults to console", () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + + getSeamlessLogger().warn("hello"); + + expect(warn).toHaveBeenCalledWith("hello"); + warn.mockRestore(); + }); + + it("routes diagnostics to an injected logger", () => { + const logger = recorder(); + setSeamlessLogger(logger); + + getSeamlessLogger().warn("a"); + getSeamlessLogger().error("b"); + + expect(logger.warned).toEqual(["a"]); + expect(logger.errored).toEqual(["b"]); + }); + + it("goes back to console when passed nothing", () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + setSeamlessLogger(recorder()); + setSeamlessLogger(); + + getSeamlessLogger().warn("back"); + + expect(warn).toHaveBeenCalledWith("back"); + warn.mockRestore(); + }); + + // Read per call, so injecting a logger after a module captured the reference + // still takes effect. + it("applies to a later swap", () => { + const first = recorder(); + const second = recorder(); + + setSeamlessLogger(first); + getSeamlessLogger().warn("one"); + setSeamlessLogger(second); + getSeamlessLogger().warn("two"); + + expect(first.warned).toEqual(["one"]); + expect(second.warned).toEqual(["two"]); + }); +}); + +describe("core diagnostics go through it", () => { + it("routes the missing delivery payload warning", async () => { + const logger = recorder(); + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + setSeamlessLogger(logger); + + await applyExternalDelivery( + { email: { name: "e", send: async () => ({}) } }, + { + message: "sent", + }, + ); + + expect(logger.warned).toHaveLength(1); + expect(logger.warned[0]).toContain("no delivery payload"); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); +});