From f4b99155fcf9b90c66a135dc19e95c867e42ebee Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Wed, 29 Jul 2026 20:46:54 -0400 Subject: [PATCH] refactor(core,express): move the passthrough proxy into core The 33 organizations, step-up, TOTP, users, and admin passthrough routes existed only inside the Express adapter, so a new adapter would have had to rebuild both the upstream call and the session gate that guards it. Core now exports proxyRequest for the call, checkProxyIdentity for the gate, and a single buildQueryString replacing three builders that had drifted apart. The Express proxy handler is now a gate check, a call, and a response, and createServer.ts drops from 705 to 667 lines. fix: a repeated query parameter reached the auth API joined into one comma-separated value on the admin and internal-metrics routes. GET /admin/auth-events?type=login&type=logout was forwarded as type=login,logout, and the API's AuthEventQuerySchema accepts type as an array, so the joined value matched no event type and the filter silently returned the wrong set. Arrays are now forwarded as repeated parameters everywhere, and nested objects are dropped rather than reaching the API as [object Object]. Also records, in the still-unreleased changeset for the response contract, that empty success responses no longer carry a JSON content type. That followed from routing every handler through applyResult and was not called out at the time. Refs #72 --- .changeset/core-applies-results.md | 4 +- .changeset/core-proxy-request.md | 16 ++ packages/core/src/handlers/admin.ts | 17 +- packages/core/src/handlers/internalMetrics.ts | 16 +- packages/core/src/index.ts | 1 + packages/core/src/proxyRequest.ts | 147 ++++++++++++++ packages/core/tests/proxyRequest.test.js | 184 ++++++++++++++++++ packages/express/src/createServer.ts | 141 ++++++-------- .../tests/proxyQueryForwarding.test.js | 99 ++++++++++ 9 files changed, 512 insertions(+), 113 deletions(-) create mode 100644 .changeset/core-proxy-request.md create mode 100644 packages/core/src/proxyRequest.ts create mode 100644 packages/core/tests/proxyRequest.test.js create mode 100644 packages/express/tests/proxyQueryForwarding.test.js diff --git a/.changeset/core-applies-results.md b/.changeset/core-applies-results.md index 7b3f2f5..6f2fc9c 100644 --- a/.changeset/core-applies-results.md +++ b/.changeset/core-applies-results.md @@ -18,6 +18,8 @@ Cookie signing moves to core with them, because the cookie format is core's: an The Express adapter drops 291 lines of source and 5.5KB of bundle, and `@seamless-auth/express` no longer carries its own cookie module. Nothing is removed from its public surface. `CookieSameSite` is now re-exported from core rather than declared locally, so `SeamlessAuthServerOptions` is unchanged for adopters. -Responses are unchanged, verified rather than assumed. Status, body, and every `Set-Cookie` header were captured on both revisions across eleven scenarios covering session set and clear, secure and insecure policy, a custom cookie domain, coded and passthrough failures, an empty failure body, and success bodies. All are byte-identical, including `HttpOnly`, `Secure`, `SameSite`, `Domain`, `Path`, and `Max-Age`. +Responses are unchanged with one exception, noted below. Status, body, and every `Set-Cookie` header were captured on both revisions across eleven scenarios covering session set and clear, secure and insecure policy, a custom cookie domain, coded and passthrough failures, an empty failure body, and success bodies. All are byte-identical, including `HttpOnly`, `Secure`, `SameSite`, `Domain`, `Path`, and `Max-Age`. + +Empty responses are now consistent about their content type. A route whose upstream returned success with no body previously sent `Content-Type: application/json` with a zero-length body, because the handler called the framework's JSON method with `undefined`. It now sends no content type, matching the routes that already ended the response instead. `Content-Length: 0` is unchanged either way, and a client reading the body sees nothing in both cases, since parsing an empty body fails regardless of the content type. Anything asserting on the content type of an empty response needs updating. Part of #72. diff --git a/.changeset/core-proxy-request.md b/.changeset/core-proxy-request.md new file mode 100644 index 0000000..c29c718 --- /dev/null +++ b/.changeset/core-proxy-request.md @@ -0,0 +1,16 @@ +--- +"@seamless-auth/core": minor +"@seamless-auth/express": patch +--- + +Move the passthrough proxy into `@seamless-auth/core`, and fix repeated query parameters being joined. + +The 33 organizations, step-up, TOTP, users, and admin passthrough routes existed only inside the Express adapter, with no core equivalent, so a new adapter would have had to rebuild both the upstream call and the session gate that guards it. New exports: + +- `proxyRequest({ authServerUrl, path, method, authorization, serviceAuthorization, forwardedClientIp, query, body })` forwards a request and returns the upstream status and body unchanged. +- `checkProxyIdentity({ subject, cookies, identity, ...cookieNames })` is the pure session gate, returning the rejection to send or `undefined` to proceed. +- `buildQueryString` and `buildUpstreamUrl` replace three separate querystring builders that had drifted apart. + +**Fix:** a repeated query parameter reached the auth API joined into a single comma-separated value on the admin and internal-metrics routes. `GET /admin/auth-events?type=login&type=logout` was forwarded as `type=login,logout`, and the API's `AuthEventQuerySchema` accepts `type` as an array, so the joined value matched no event type and the filter silently returned the wrong set. Array parameters are now forwarded as repeated parameters on every route. Nested objects are dropped rather than stringified, so a query like `?filter[from]=x` can no longer reach the API as `filter=[object Object]`. + +The Express adapter drops 38 lines, `createServer.ts` drops from 705 to 667 lines, and the proxy handler is now a gate check, a call, and a response. diff --git a/packages/core/src/handlers/admin.ts b/packages/core/src/handlers/admin.ts index 012dab2..c5adb2e 100644 --- a/packages/core/src/handlers/admin.ts +++ b/packages/core/src/handlers/admin.ts @@ -1,4 +1,5 @@ import { authFetch } from "../authFetch.js"; +import { buildUpstreamUrl, type QueryInput } from "../proxyRequest.js"; import type { ResultFailure } from "../result.js"; import { readUpstreamFailure } from "../upstreamError.js"; @@ -10,7 +11,7 @@ type BaseOpts = { }; type WithQuery = BaseOpts & { - query?: Record; + query?: QueryInput; }; type WithBody = BaseOpts & { @@ -22,25 +23,13 @@ type Result = ResultFailure & { body?: any; }; -function buildUrl(base: string, query?: Record) { - if (!query) return base; - - const qs = new URLSearchParams( - Object.entries(query) - .filter(([, v]) => v !== undefined && v !== null) - .map(([k, v]) => [k, String(v)]), - ).toString(); - - return qs ? `${base}?${qs}` : base; -} - async function request( method: "GET" | "POST" | "PATCH" | "DELETE", path: string, opts: WithQuery & WithBody, ): Promise { const up = await authFetch( - buildUrl(`${opts.authServerUrl}${path}`, opts.query), + buildUpstreamUrl(opts.authServerUrl, path, opts.query), { method, authorization: opts.authorization, diff --git a/packages/core/src/handlers/internalMetrics.ts b/packages/core/src/handlers/internalMetrics.ts index bb08b03..8fb081e 100644 --- a/packages/core/src/handlers/internalMetrics.ts +++ b/packages/core/src/handlers/internalMetrics.ts @@ -1,4 +1,5 @@ import { authFetch } from "../authFetch.js"; +import { buildUpstreamUrl, type QueryInput } from "../proxyRequest.js"; import type { ResultFailure } from "../result.js"; import { readUpstreamFailure } from "../upstreamError.js"; @@ -10,7 +11,7 @@ type BaseOpts = { }; type WithQuery = BaseOpts & { - query?: Record; + query?: QueryInput; }; type Result = ResultFailure & { @@ -18,20 +19,9 @@ type Result = ResultFailure & { body?: any; }; -function buildUrl(base: string, query?: WithQuery["query"]) { - if (!query) return base; - const qs = new URLSearchParams( - Object.entries(query) - .filter(([, v]) => v !== undefined && v !== null) - .map(([k, v]) => [k, String(v)]), - ).toString(); - - return qs ? `${base}?${qs}` : base; -} - async function get(path: string, opts: WithQuery): Promise { const up = await authFetch( - buildUrl(`${opts.authServerUrl}${path}`, opts.query), + buildUpstreamUrl(opts.authServerUrl, path, opts.query), { method: "GET", authorization: opts.authorization, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 3792b6b..87181c4 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -16,6 +16,7 @@ export { roleGrantsAccess, } from "@seamless-auth/types/role/matching"; export * from "./applyResult.js"; +export * from "./proxyRequest.js"; export * from "./result.js"; export * from "./redaction.js"; diff --git a/packages/core/src/proxyRequest.ts b/packages/core/src/proxyRequest.ts new file mode 100644 index 0000000..c54893d --- /dev/null +++ b/packages/core/src/proxyRequest.ts @@ -0,0 +1,147 @@ +import type { AppliableResult } from "./applyResult.js"; +import { authFetch, type AuthFetchOptions } from "./authFetch.js"; + +/** + * Query parameters to forward upstream. + * + * Values are deliberately `unknown`: adapters hand over whatever their + * framework parsed, and Express's `ParsedQs` can nest objects. Scalars and + * arrays of scalars are forwarded, everything else is dropped, so a nested + * object cannot reach the auth API as `[object Object]`. + */ +export type QueryInput = Record; + +function isScalar(value: unknown): value is string | number | boolean { + return ( + typeof value === "string" || + typeof value === "number" || + typeof value === "boolean" + ); +} + +/** + * Builds a querystring for an upstream call. + * + * An array becomes repeated parameters (`?type=login&type=logout`), which is + * what the auth API's schemas accept. Joining them into one comma-separated + * value produces a parameter that matches nothing upstream. + */ +export function buildQueryString(query?: QueryInput): string { + if (!query) return ""; + + const params = new URLSearchParams(); + + for (const [key, value] of Object.entries(query)) { + if (Array.isArray(value)) { + for (const item of value) { + if (isScalar(item)) params.append(key, String(item)); + } + continue; + } + + if (isScalar(value)) params.append(key, String(value)); + } + + return params.toString(); +} + +export function buildUpstreamUrl( + authServerUrl: string, + path: string, + query?: QueryInput, +): string { + const base = `${authServerUrl}${path.startsWith("/") ? "" : "/"}${path}`; + const qs = buildQueryString(query); + + return qs ? `${base}?${qs}` : base; +} + +export type ProxyIdentity = "preAuth" | "access" | "register"; + +export interface ProxyIdentityInput { + /** `sub` from the verified cookie payload, if the request carried one. */ + subject?: string; + cookies: Record; + identity: ProxyIdentity; + accessCookieName: string; + preAuthCookieName: string; + registrationCookieName: string; +} + +export interface ProxyIdentityRejection { + status: number; + errorCode: string; + /** Set when the caller should log the rejection rather than only return it. */ + warn?: string; +} + +/** + * Checks that a request carries the session a proxied route requires. + * + * Returns the rejection to send, or `undefined` when the request may proceed. + * The cookie payload alone is not enough: it survives a refresh, so the route + * also has to see the specific cookie for the identity it needs. + */ +export function checkProxyIdentity( + input: ProxyIdentityInput, +): ProxyIdentityRejection | undefined { + if (!input.subject) { + return { + status: 401, + errorCode: "Unauthenticated request", + warn: "Missing expected cookie payload/sub.", + }; + } + + const required: Record = { + access: { name: input.accessCookieName, error: "access session required" }, + preAuth: { + name: input.preAuthCookieName, + error: "pre-auth session required", + }, + register: { + name: input.registrationCookieName, + error: "registration session required", + }, + }; + + const { name, error } = required[input.identity]; + + return input.cookies[name] ? undefined : { status: 401, errorCode: error }; +} + +export interface ProxyRequestOptions { + authServerUrl: string; + path: string; + method?: AuthFetchOptions["method"]; + authorization?: string; + serviceAuthorization?: string; + forwardedClientIp?: string; + query?: QueryInput; + body?: unknown; +} + +/** + * Forwards a request to the auth API and returns its status and body unchanged. + * + * Transparent on purpose: a proxied route has no view into what the response + * means, so a failure body is returned as-is rather than reshaped into a code. + */ +export async function proxyRequest( + opts: ProxyRequestOptions, +): Promise { + const method = opts.method ?? "POST"; + + const upstream = await authFetch( + buildUpstreamUrl(opts.authServerUrl, opts.path, opts.query), + { + method, + authorization: opts.authorization, + serviceAuthorization: opts.serviceAuthorization, + forwardedClientIp: opts.forwardedClientIp, + ...(method === "GET" ? {} : { body: opts.body }), + }, + ); + + return { status: upstream.status, body: await upstream.json() }; +} diff --git a/packages/core/tests/proxyRequest.test.js b/packages/core/tests/proxyRequest.test.js new file mode 100644 index 0000000..160a6e7 --- /dev/null +++ b/packages/core/tests/proxyRequest.test.js @@ -0,0 +1,184 @@ +import { jest } from "@jest/globals"; + +const authFetchMock = jest.fn(); + +jest.unstable_mockModule("../dist/authFetch.js", () => ({ + authFetch: authFetchMock, +})); + +const { buildQueryString, buildUpstreamUrl, checkProxyIdentity, proxyRequest } = + await import("../dist/proxyRequest.js"); + +function upstream(status, body) { + return { ok: status >= 200 && status < 300, status, json: async () => body }; +} + +describe("buildQueryString", () => { + it("forwards scalars, coercing numbers and booleans", () => { + expect(buildQueryString({ limit: 10, active: true, q: "x" })).toBe( + "limit=10&active=true&q=x", + ); + }); + + // The auth API's AuthEventQuerySchema accepts `type` as an array. Joining the + // values into one comma-separated parameter matches no event type upstream. + it("repeats an array rather than joining it", () => { + expect(buildQueryString({ type: ["login", "logout"] })).toBe( + "type=login&type=logout", + ); + }); + + it("drops null and undefined", () => { + expect(buildQueryString({ a: null, b: undefined, c: "1" })).toBe("c=1"); + }); + + it("drops a nested object instead of sending [object Object]", () => { + expect(buildQueryString({ filter: { from: "x" }, ok: "1" })).toBe("ok=1"); + }); + + it("drops non-scalar array members", () => { + expect(buildQueryString({ t: ["a", null, { b: 1 }, "c"] })).toBe("t=a&t=c"); + }); + + it("returns empty for no query", () => { + expect(buildQueryString()).toBe(""); + expect(buildQueryString({})).toBe(""); + }); +}); + +describe("buildUpstreamUrl", () => { + it("joins the path whether or not it is rooted", () => { + expect(buildUpstreamUrl("https://auth.test", "/organizations")).toBe( + "https://auth.test/organizations", + ); + expect(buildUpstreamUrl("https://auth.test", "organizations")).toBe( + "https://auth.test/organizations", + ); + }); + + it("omits the separator when the query is empty", () => { + expect(buildUpstreamUrl("https://auth.test", "/x", {})).toBe( + "https://auth.test/x", + ); + }); +}); + +describe("checkProxyIdentity", () => { + const base = { + cookies: {}, + accessCookieName: "sa-access", + preAuthCookieName: "sa-preauth", + registrationCookieName: "sa-register", + }; + + it("rejects a request with no verified subject, and asks the caller to log it", () => { + const rejection = checkProxyIdentity({ ...base, identity: "access" }); + + expect(rejection).toMatchObject({ + status: 401, + errorCode: "Unauthenticated request", + }); + expect(rejection.warn).toBeTruthy(); + }); + + // The payload survives a refresh, so it alone does not prove the request + // carries the session this route needs. + it.each([ + ["access", "access session required"], + ["preAuth", "pre-auth session required"], + ["register", "registration session required"], + ])("rejects %s without its cookie", (identity, errorCode) => { + expect(checkProxyIdentity({ ...base, subject: "u1", identity })).toEqual({ + status: 401, + errorCode, + }); + }); + + it.each([ + ["access", "sa-access"], + ["preAuth", "sa-preauth"], + ["register", "sa-register"], + ])("admits %s when its cookie is present", (identity, cookieName) => { + expect( + checkProxyIdentity({ + ...base, + subject: "u1", + identity, + cookies: { [cookieName]: "value" }, + }), + ).toBeUndefined(); + }); + + it("does not accept a different identity's cookie", () => { + expect( + checkProxyIdentity({ + ...base, + subject: "u1", + identity: "access", + cookies: { "sa-preauth": "value" }, + }), + ).toEqual({ status: 401, errorCode: "access session required" }); + }); +}); + +describe("proxyRequest", () => { + beforeEach(() => authFetchMock.mockReset()); + + it("forwards method, headers, body, and query", async () => { + authFetchMock.mockResolvedValue(upstream(200, { ok: true })); + + const result = await proxyRequest({ + authServerUrl: "https://auth.test", + path: "organizations", + method: "POST", + authorization: "Bearer a", + serviceAuthorization: "svc", + forwardedClientIp: "203.0.113.9", + query: { limit: 5 }, + body: { name: "Acme" }, + }); + + expect(authFetchMock).toHaveBeenCalledWith( + "https://auth.test/organizations?limit=5", + { + method: "POST", + authorization: "Bearer a", + serviceAuthorization: "svc", + forwardedClientIp: "203.0.113.9", + body: { name: "Acme" }, + }, + ); + expect(result).toEqual({ status: 200, body: { ok: true } }); + }); + + it("omits the body on a GET", async () => { + authFetchMock.mockResolvedValue(upstream(200, {})); + + await proxyRequest({ + authServerUrl: "https://auth.test", + path: "/totp/status", + method: "GET", + body: { ignored: true }, + }); + + expect(authFetchMock.mock.calls[0][1]).not.toHaveProperty("body"); + }); + + it("defaults to POST", async () => { + authFetchMock.mockResolvedValue(upstream(200, {})); + + await proxyRequest({ authServerUrl: "https://auth.test", path: "/x" }); + + expect(authFetchMock.mock.calls[0][1].method).toBe("POST"); + }); + + // A proxied route cannot interpret the response, so a failure body is + // returned as-is rather than reshaped into a code. + it("returns an upstream failure body untouched", async () => { + authFetchMock.mockResolvedValue(upstream(403, { error: "forbidden" })); + + expect( + await proxyRequest({ authServerUrl: "https://auth.test", path: "/x" }), + ).toEqual({ status: 403, body: { error: "forbidden" } }); + }); +}); diff --git a/packages/express/src/createServer.ts b/packages/express/src/createServer.ts index c37afce..7e2c81b 100644 --- a/packages/express/src/createServer.ts +++ b/packages/express/src/createServer.ts @@ -3,6 +3,7 @@ import cookieParser from "cookie-parser"; import { createEnsureCookiesMiddleware } from "./middleware/ensureCookies"; import { createOriginGuardMiddleware } from "./middleware/originGuard"; +import { respond } from "./internal/respond"; import type { CookieSameSite } from "@seamless-auth/core"; import type { SeamlessAuthMessagingOptions } from "@seamless-auth/core"; @@ -29,6 +30,8 @@ import * as admin from "./handlers/admin"; import { authFetch, AuthFetchOptions, + checkProxyIdentity, + proxyRequest, redactSensitiveText, } from "@seamless-auth/core"; import { @@ -109,27 +112,6 @@ export interface SeamlessAuthUser { token?: string; } -function buildProxyQueryString(queryInput: Request["query"]): string { - const query = new URLSearchParams(); - - for (const [key, value] of Object.entries(queryInput)) { - if (typeof value === "string") { - query.append(key, value); - continue; - } - - if (Array.isArray(value)) { - for (const item of value) { - if (typeof item === "string") { - query.append(key, item); - } - } - } - } - - return query.toString(); -} - function routeParam(req: Request, name: string): string { const value = req.params[name]; const resolved = Array.isArray(value) ? value[0] : value; @@ -243,61 +225,41 @@ export function createSeamlessAuthServer( method: AuthFetchOptions["method"] = "POST", ) => async (req: Request & { cookiePayload?: any }, res: Response) => { - if (!req.cookiePayload?.sub) { - console.warn( - "[SEAMLESS-AUTH-EXPRESS] - (proxyWithIdentity) - Missing expected cookie payload/sub.", - ); - res.status(401).json({ error: "Unauthenticated request" }); - return; - } - - if ( - identity === "access" && - !req.cookies[resolvedOpts.accessCookieName] - ) { - res.status(401).json({ error: "access session required" }); - return; - } - - if ( - identity === "preAuth" && - !req.cookies[resolvedOpts.preAuthCookieName] - ) { - res.status(401).json({ error: "pre-auth session required" }); - return; - } + const rejection = checkProxyIdentity({ + subject: req.cookiePayload?.sub, + cookies: req.cookies ?? {}, + identity, + accessCookieName: resolvedOpts.accessCookieName, + preAuthCookieName: resolvedOpts.preAuthCookieName, + registrationCookieName: resolvedOpts.registrationCookieName, + }); + + if (rejection) { + if (rejection.warn) { + console.warn( + `[SEAMLESS-AUTH-EXPRESS] - (proxyWithIdentity) - ${rejection.warn}`, + ); + } - if ( - identity === "register" && - !req.cookies[resolvedOpts.registrationCookieName] - ) { - res.status(401).json({ error: "registration session required" }); + res.status(rejection.status).json({ error: rejection.errorCode }); return; } - const authorization = buildServiceAuthorization(req, resolvedOpts); - const forwardedClientIp = buildForwardedClientIp(req, resolvedOpts.resolveClientIp); - const serviceAuthorization = buildProxyServiceAuthorization(resolvedOpts); - const options = - method == "GET" - ? { method, authorization, serviceAuthorization, forwardedClientIp } - : { - method, - authorization, - serviceAuthorization, - forwardedClientIp, - body: req.body, - }; - - const queryString = buildProxyQueryString(req.query); - const resolvedPath = typeof path === "function" ? path(req) : path; - const upstream = await authFetch( - `${resolvedOpts.authServerUrl}/${resolvedPath}${queryString ? `?${queryString}` : ""}`, - options, - ); - - const data = await upstream.json(); - res.status(upstream.status).json(data); + const result = await proxyRequest({ + authServerUrl: resolvedOpts.authServerUrl, + path: typeof path === "function" ? path(req) : path, + method, + authorization: buildServiceAuthorization(req, resolvedOpts), + serviceAuthorization: buildProxyServiceAuthorization(resolvedOpts), + forwardedClientIp: buildForwardedClientIp( + req, + resolvedOpts.resolveClientIp, + ), + query: req.query, + body: req.body, + }); + + respond(res, result, resolvedOpts); }; // Runs before ensureCookies and the routes so a blocked cross-site request @@ -401,7 +363,8 @@ export function createSeamlessAuthServer( r.get( "/organizations/:organizationId", proxyWithIdentity( - req => `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + (req) => + `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, "access", "GET", ), @@ -409,7 +372,8 @@ export function createSeamlessAuthServer( r.patch( "/organizations/:organizationId", proxyWithIdentity( - req => `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + (req) => + `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, "access", "PATCH", ), @@ -420,7 +384,8 @@ export function createSeamlessAuthServer( r.get( "/organizations/:organizationId/members", proxyWithIdentity( - req => `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, + (req) => + `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, "access", "GET", ), @@ -428,14 +393,15 @@ export function createSeamlessAuthServer( r.post( "/organizations/:organizationId/members", proxyWithIdentity( - req => `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, + (req) => + `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, "access", ), ); r.patch( "/organizations/:organizationId/members/:userId", proxyWithIdentity( - req => + (req) => `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members/${encodeURIComponent(routeParam(req, "userId"))}`, "access", "PATCH", @@ -444,7 +410,7 @@ export function createSeamlessAuthServer( r.delete( "/organizations/:organizationId/members/:userId", proxyWithIdentity( - req => + (req) => `organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members/${encodeURIComponent(routeParam(req, "userId"))}`, "access", "DELETE", @@ -491,7 +457,10 @@ export function createSeamlessAuthServer( { method: "GET", serviceAuthorization: buildProxyServiceAuthorization(resolvedOpts), - forwardedClientIp: buildForwardedClientIp(req, resolvedOpts.resolveClientIp), + forwardedClientIp: buildForwardedClientIp( + req, + resolvedOpts.resolveClientIp, + ), }, ); @@ -600,7 +569,8 @@ export function createSeamlessAuthServer( r.get( "/admin/organizations/:organizationId", proxyWithIdentity( - req => `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + (req) => + `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, "access", "GET", ), @@ -608,7 +578,8 @@ export function createSeamlessAuthServer( r.patch( "/admin/organizations/:organizationId", proxyWithIdentity( - req => `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, + (req) => + `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}`, "access", "PATCH", ), @@ -616,7 +587,7 @@ export function createSeamlessAuthServer( r.get( "/admin/organizations/:organizationId/members", proxyWithIdentity( - req => + (req) => `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, "access", "GET", @@ -625,7 +596,7 @@ export function createSeamlessAuthServer( r.post( "/admin/organizations/:organizationId/members", proxyWithIdentity( - req => + (req) => `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members`, "access", ), @@ -633,7 +604,7 @@ export function createSeamlessAuthServer( r.patch( "/admin/organizations/:organizationId/members/:userId", proxyWithIdentity( - req => + (req) => `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members/${encodeURIComponent(routeParam(req, "userId"))}`, "access", "PATCH", @@ -642,7 +613,7 @@ export function createSeamlessAuthServer( r.delete( "/admin/organizations/:organizationId/members/:userId", proxyWithIdentity( - req => + (req) => `admin/organizations/${encodeURIComponent(routeParam(req, "organizationId"))}/members/${encodeURIComponent(routeParam(req, "userId"))}`, "access", "DELETE", diff --git a/packages/express/tests/proxyQueryForwarding.test.js b/packages/express/tests/proxyQueryForwarding.test.js new file mode 100644 index 0000000..d6332fc --- /dev/null +++ b/packages/express/tests/proxyQueryForwarding.test.js @@ -0,0 +1,99 @@ +// Locks how query parameters reach the auth API. The admin routes hand +// `req.query` straight through, so a repeated parameter has to stay repeated: +// the API's AuthEventQuerySchema accepts `type` as an array, and joining the +// values into `type=login,logout` matches no event type upstream. +import { jest } from "@jest/globals"; +import express from "express"; +import jwt from "jsonwebtoken"; +import request from "supertest"; + +const { default: createSeamlessAuthServer } = await import("../dist/index.js"); + +const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; + +function createJsonResponse(status, body) { + return { ok: status >= 200 && status < 300, status, json: async () => body }; +} + +function createAccessCookie() { + const token = jwt.sign( + { + sub: "admin-123", + roles: ["admin"], + sessionId: "session-123", + token: "access-token", + }, + COOKIE_SECRET, + { algorithm: "HS256", expiresIn: "300s" }, + ); + + return `seamless-access=${token}`; +} + +function createApp() { + const app = express(); + + app.use( + "/auth", + createSeamlessAuthServer({ + authServerUrl: "https://auth.example.com", + cookieSecret: COOKIE_SECRET, + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + jwksKid: "test-main", + }), + ); + + return app; +} + +describe("proxy query forwarding", () => { + const originalFetch = global.fetch; + let requestedUrl; + + beforeEach(() => { + requestedUrl = undefined; + global.fetch = jest.fn(async (url) => { + requestedUrl = String(url); + return createJsonResponse(200, { ok: true }); + }); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + async function get(path) { + await request(createApp()).get(path).set("Cookie", createAccessCookie()); + return requestedUrl; + } + + it("repeats an array parameter on an admin route", async () => { + const url = await get( + "/auth/admin/auth-events?type=login&type=logout&limit=5", + ); + + expect(url).toBe( + "https://auth.example.com/admin/auth-events?type=login&type=logout&limit=5", + ); + expect(url).not.toContain("login%2Clogout"); + }); + + it("repeats an array parameter on a passthrough route", async () => { + const url = await get("/auth/organizations?type=a&type=b"); + + expect(url).toBe("https://auth.example.com/organizations?type=a&type=b"); + }); + + it("forwards a scalar parameter unchanged", async () => { + expect(await get("/auth/admin/sessions?limit=5")).toBe( + "https://auth.example.com/admin/sessions?limit=5", + ); + }); + + it("sends no query separator when there is no query", async () => { + expect(await get("/auth/totp/status")).toBe( + "https://auth.example.com/totp/status", + ); + }); +});