From e5c4445e3216df9f5c1ba614b67297ad8f4ec831 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 14:24:52 +0000 Subject: [PATCH 1/5] fix(plugin-auth): every in-process bridge hands handleRequest the request's transport The client-address trust rule (ADR-0069 D5/D7) establishes a re-dispatched request's address from the transport's peer when no trusted proxy is declared. Eight raw mounts re-dispatched through AuthManager.handleRequest with the request alone; each now hands over transportOf(c), as the catch-all does. Pinned per bridge. Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude --- .../plugins/plugin-auth/src/auth-plugin.ts | 48 ++- .../bridge-redispatch-transport.pin.test.ts | 283 ++++++++++++++++++ 2 files changed, 322 insertions(+), 9 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 03e026deaed..e1313b25475 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -69,7 +69,7 @@ import { registerMemberRoleCanonicalization } from './member-role-canonical.js'; import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js'; import { MANAGED_EXTENSION_EDITABLE_FIELDS } from './managed-extension-fields.js'; import { runSetInitialPassword } from './set-initial-password.js'; -import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain } from './register-sso-provider.js'; +import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain, type AuthRequestHandler } from './register-sso-provider.js'; import { runResendVerificationEmail } from './send-verification-email.js'; import { runSysSessionRevoke, sysSessionRevokeDepsFor } from './sys-session-revoke.js'; import { readJsonRequestBody } from './request-json-body.js'; @@ -2960,6 +2960,21 @@ export class AuthPlugin implements Plugin { return verdict.actor; }; + // ── [#24877] ADR-0069 D5/D7 — better-auth's universal handler for the + // request a raw mount below serves, WITH that request's transport. Every + // in-process bridge re-dispatches through `AuthManager.handleRequest` with + // the transport of the request it serves — a bridge that hands a helper a + // handler takes it from here; one that delegates the request itself passes + // `transportOf(c)` inline, as the catch-all at the end of this method + // does — so `handleRequest` establishes the re-dispatched request's client + // address by the platform's trust rule (`resolveClientAddress`): forwarded + // headers only when the deployment declares `server.trustProxy`, the + // connection peer otherwise. `handleRequest(req)` alone leaves the rule no + // peer to judge on. Pinned per bridge by + // `bridge-redispatch-transport.pin.test.ts`. + const handleWithTransportOf = (c: any): AuthRequestHandler => (request) => + this.authManager!.handleRequest(request, transportOf(c)); + // ──────────────────────────────────────────────────────────────────── // SSO admin: register an external OIDC IdP from the flat metadata form // (ADR-0135 D6). `@better-auth/sso`'s POST /sso/register expects the protocol @@ -2993,7 +3008,7 @@ export class AuthPlugin implements Plugin { const gated = await gateAdmin(c); if (gated instanceof Response) return gated; const { status, body } = await runRegisterSsoProviderFromForm( - (req) => this.authManager!.handleRequest(req), + handleWithTransportOf(c), c.req.raw, ); return c.json(body, status as any); @@ -3293,7 +3308,7 @@ export class AuthPlugin implements Plugin { try { const gated = await gateAdmin(c); if (gated instanceof Response) return gated; - return await this.authManager!.handleRequest(c.req.raw); + return await this.authManager!.handleRequest(c.req.raw, transportOf(c)); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); ctx.logger.error('[AuthPlugin] admin/remove-user failed', err); @@ -3370,7 +3385,7 @@ export class AuthPlugin implements Plugin { return c.json(answer, 200); } } - return await this.authManager!.handleRequest(c.req.raw); + return await this.authManager!.handleRequest(c.req.raw, transportOf(c)); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); ctx.logger.error('[AuthPlugin] admin/has-permission failed', err); @@ -3493,7 +3508,7 @@ export class AuthPlugin implements Plugin { const gated = await gateAdmin(c); if (gated instanceof Response) return gated; const { status, body } = await runRegisterSamlProviderFromForm( - (req) => this.authManager!.handleRequest(req), + handleWithTransportOf(c), c.req.raw, ); return c.json(body, status as any); @@ -3521,7 +3536,7 @@ export class AuthPlugin implements Plugin { const gated = await gateAdmin(c); if (gated instanceof Response) return gated; const { status, body } = await runRequestDomainVerification( - (req) => this.authManager!.handleRequest(req), + handleWithTransportOf(c), c.req.raw, { domainVerificationEnabled: this.authManager!.isSsoDomainVerificationEnabled() }, ); @@ -3539,7 +3554,7 @@ export class AuthPlugin implements Plugin { const gated = await gateAdmin(c); if (gated instanceof Response) return gated; const { status, body } = await runVerifyDomain( - (req) => this.authManager!.handleRequest(req), + handleWithTransportOf(c), c.req.raw, { domainVerificationEnabled: this.authManager!.isSsoDomainVerificationEnabled() }, ); @@ -3671,10 +3686,25 @@ export class AuthPlugin implements Plugin { // judge). Same family as `sys-oauth-application/register` above: an // object-scoped mount outside `/admin/`, because it admits the owner. // See `sys-session-revoke.ts` for the lanes and refusals. + // + // [#24877] The owner lane's re-dispatch goes through the manager slice's + // `handleRequest`, so the slice handed over is the manager's own seams + // with `handleRequest` bound to this request's transport + // (`handleWithTransportOf` above) — the same handler every other bridge + // re-dispatches through. rawApp.post(`${basePath}/sys-session/revoke`, async (c: any) => { try { + const manager = this.authManager!; const { status, body } = await runSysSessionRevoke( - sysSessionRevokeDepsFor(this.authManager!, basePath), + sysSessionRevokeDepsFor( + { + getApi: () => manager.getApi(), + getAuthContext: () => manager.getAuthContext(), + getDataEngine: () => manager.getDataEngine(), + handleRequest: handleWithTransportOf(c), + }, + basePath, + ), c.req.raw, ); return c.json(body, status as any); @@ -3702,7 +3732,7 @@ export class AuthPlugin implements Plugin { rawApp.post(`${basePath}/send-verification-email`, async (c: any) => { try { const { status, body } = await runResendVerificationEmail( - (req) => this.authManager!.handleRequest(req), + handleWithTransportOf(c), c.req.raw, ); return c.json(body, status); diff --git a/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts b/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts new file mode 100644 index 00000000000..702655bc31b --- /dev/null +++ b/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts @@ -0,0 +1,283 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#24877] Every in-process bridge judges the request it re-dispatches by the +// platform's client-address trust rule (ADR-0069 D5/D7, `./client-address.ts`): +// +// - forwarded headers count only when the deployment declares +// `server.trustProxy` (`AuthManagerOptions.trustProxy`); +// - otherwise the address is the transport's peer (`IHttpRequest.remoteAddress`). +// +// `AuthManager.handleRequest` establishes that address from the request AND the +// transport it is handed, so a bridge has to hand over the transport of the +// request it serves, as the catch-all does (`transportOf(c)` in +// `auth-plugin.ts`). One pin per bridge, each stating the rule for its route. +// +// What is read is the one channel the server's decision reaches the vendored +// library through: the platform-owned header `handleRequest` writes on every +// request it hands the library's handler (`client-address-vendor.ts`). Every +// request a bridge re-dispatches during one call is read, and the bridge's own +// inner route has to be among them, so a bridge that never reached the library +// cannot pass vacuously. +// +// The fixture is the D5/D7 suites' (`client-address-vendor-trust-rule.test.ts`): +// `AuthPlugin.registerAuthRoutes` mounted on a real Hono app in front of a real +// `AuthManager` on the installed better-auth, with the transport peer supplied +// the way `@hono/node-server` supplies it, as the Node request on +// `c.env.incoming`. No trusted proxy is declared. Every request also carries +// forwarded headers and the platform-owned header naming another address, set +// by the caller: none of them may decide the address. Nothing reaches the +// network: every re-dispatch below is answered by the vendor's own refusal +// before any discovery or DNS work. +// +// The library's handler is observed through a plain instance wrapper, not +// `vi.spyOn`: the imported `impersonation-bearer-rotation.test` registers +// `afterEach(restoreAllMocks)` in this file too, which would strip a spy after +// the first case. + +import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from 'vitest'; +import { Hono } from 'hono'; +import { AuthManager } from './auth-manager'; +import { AuthPlugin } from './auth-plugin'; +import { VENDOR_CLIENT_ADDRESS_HEADER } from './client-address-vendor'; +import { SYS_SESSION_REVOKE_ID_PARAM } from './sys-session-revoke'; +import { createMemoryEngine } from './impersonation-bearer-rotation.test'; +import { inviteForAudienceGate } from './audience-gate-test-support'; +import type { PluginContext } from '@objectstack/core'; + +const SECRET = 'test-secret-at-least-32-chars-long!!'; +const PASSWORD = 'S3cure!Passw0rd-24877'; +const ORIGIN = 'http://localhost:3000'; +const BASE = '/api/v1/auth'; +const ADMIN_EMAIL = 'admin.24877@example.com'; +const MEMBER_EMAIL = 'member.24877@example.com'; + +/** Documentation ranges (RFC 5737): the connection peer, and what the caller's headers claim. */ +const PEER = '203.0.113.7'; +const CLAIMED = '198.51.100.9'; + +/** `@better-auth/sso` reserves this id, and refuses it before any discovery or endpoint validation. */ +const RESERVED_PROVIDER_ID = 'credential'; +const UNKNOWN_PROVIDER_ID = 'pin-24877-no-such-provider'; + +const mockCtx = (): PluginContext => + ({ + registerService: vi.fn(), + getService: vi.fn((name: string) => (name === 'manifest' ? { register: vi.fn() } : undefined)), + getServices: vi.fn(() => new Map()), + hook: vi.fn(), + trigger: vi.fn(), + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + getKernel: vi.fn(), + }) as any; + +/** One request the vendored library's handler was handed: its path, and the client address it carried. */ +interface Handed { + path: string; + address: string | null; +} + +interface Stage { + app: Hono; + adminBearer: string; + memberBearer: string; + /** A session of the member's own, minted for the revoke bridge alone. */ + revokeBearer: string; + revokeSessionId: string; + /** Every request the library's handler was handed, in order. */ + handed: Handed[]; +} + +async function stage(): Promise { + const engine = createMemoryEngine(); + const manager = new AuthManager({ + secret: SECRET, + baseUrl: ORIGIN, + dataEngine: engine, + plugins: { admin: true, sso: true, ssoDomainVerification: true }, + } as any); + + const direct = (path: string, body: unknown) => + manager.handleRequest( + new Request(`${ORIGIN}${BASE}${path}`, { + method: 'POST', + headers: { 'content-type': 'application/json', origin: ORIGIN }, + body: JSON.stringify(body), + }), + ); + + for (const [email, name] of [ + [ADMIN_EMAIL, 'Platform Admin'], + [MEMBER_EMAIL, 'Plain Member'], + ]) { + await inviteForAudienceGate(manager, email); + const res = await direct('/sign-up/email', { email, password: PASSWORD, name }); + expect(res.status, `sign-up ${email}: ${await res.clone().text()}`).toBe(200); + } + const users = (engine.tables.get('sys_user') ?? []) as any[]; + // The legacy scalar `isPlatformAdminUser` accepts — `gateAdmin` admits. + users.find((r) => r.email === ADMIN_EMAIL)!.role = 'admin'; + const memberId = String(users.find((r) => r.email === MEMBER_EMAIL)!.id); + + const bearerFor = async (email: string) => { + const res = await direct('/sign-in/email', { email, password: PASSWORD }); + const token = res.headers.get('set-auth-token'); + expect(token, `sign-in ${email}: ${await res.clone().text()}`).toBeTruthy(); + return token!; + }; + const adminBearer = await bearerFor(ADMIN_EMAIL); + const memberBearer = await bearerFor(MEMBER_EMAIL); + const memberSessions = () => + ((engine.tables.get('sys_session') ?? []) as any[]).filter((r) => String(r.user_id) === memberId); + const before = memberSessions().length; + const revokeBearer = await bearerFor(MEMBER_EMAIL); + const minted = memberSessions().slice(before); + expect(minted, 'the revoke bridge’s own session').toHaveLength(1); + const revokeSessionId = String(minted[0].id); + + const auth = await (manager as any).getOrCreateAuth(); + const handler = auth.handler; + const handed: Handed[] = []; + auth.handler = async (request: Request) => { + handed.push({ path: new URL(request.url).pathname, address: request.headers.get(VENDOR_CLIENT_ADDRESS_HEADER) }); + return handler(request); + }; + + // The REAL route registration — raw mounts ahead of the catch-all. + const app = new Hono(); + const ctx = mockCtx(); + const plugin = new AuthPlugin({ secret: SECRET }); + await plugin.init(ctx); + (plugin as any).authManager = manager; + (plugin as any).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx); + + return { app, adminBearer, memberBearer, revokeBearer, revokeSessionId, handed }; +} + +/** A bridge: its mount, the vendor route it re-dispatches to, and a request that gets that far. */ +interface Bridge { + route: string; + inner: string; + request: (s: Stage) => { bearer?: string; body: unknown }; +} + +const BRIDGES: Bridge[] = [ + // Gate-then-delegate: the request itself is handed on. + { + route: 'POST /admin/remove-user', + inner: '/admin/remove-user', + request: (s) => ({ bearer: s.adminBearer, body: { userId: 'usr_pin_24877_no_such_user' } }), + }, + { + route: 'POST /admin/has-permission', + inner: '/admin/has-permission', + // Anonymous: the delegate lane. + request: () => ({ body: { permissions: { user: ['list'] } } }), + }, + // Reshape-and-re-dispatch helpers (`register-sso-provider.ts`). + { + route: 'POST /admin/sso/register', + inner: '/sso/register', + request: (s) => ({ + bearer: s.adminBearer, + body: { + providerId: RESERVED_PROVIDER_ID, + issuer: 'https://idp.pin-24877.example.com', + domain: 'pin-24877.example.com', + clientId: 'pin-24877-client', + clientSecret: 'pin-24877-secret', + }, + }), + }, + { + route: 'POST /admin/sso/register-saml', + inner: '/sso/register', + request: (s) => ({ + bearer: s.adminBearer, + body: { + providerId: RESERVED_PROVIDER_ID, + issuer: 'https://idp.pin-24877.example.com', + domain: 'pin-24877.example.com', + entryPoint: 'https://idp.pin-24877.example.com/sso', + cert: 'pin-24877-cert', + }, + }), + }, + { + route: 'POST /admin/sso/request-domain-verification', + inner: '/sso/request-domain-verification', + request: (s) => ({ bearer: s.adminBearer, body: { providerId: UNKNOWN_PROVIDER_ID } }), + }, + { + route: 'POST /admin/sso/verify-domain', + inner: '/sso/verify-domain', + request: (s) => ({ bearer: s.adminBearer, body: { providerId: UNKNOWN_PROVIDER_ID } }), + }, + // The one-click resend (`send-verification-email.ts`): a session read, then the send. + { + route: 'POST /send-verification-email', + inner: '/send-verification-email', + request: (s) => ({ bearer: s.memberBearer, body: {} }), + }, + // The owner lane of the session revoke (`sys-session-revoke.ts`). + { + route: 'POST /sys-session/revoke', + inner: '/revoke-session', + request: (s) => ({ bearer: s.revokeBearer, body: { [SYS_SESSION_REVOKE_ID_PARAM]: s.revokeSessionId } }), + }, +]; + +let s: Stage; + +beforeAll(async () => { + s = await stage(); +}); + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** Serve `route` (`METHOD /path`) from the connection peer, with the caller's headers naming another address. */ +async function through(route: string, { bearer, body }: { bearer?: string; body?: unknown }): Promise { + const [method, path] = route.split(' ') as [string, string]; + s.handed.length = 0; + const res = await s.app.request( + `${ORIGIN}${BASE}${path}`, + { + method, + headers: { + 'content-type': 'application/json', + origin: ORIGIN, + 'x-forwarded-for': CLAIMED, + 'x-real-ip': CLAIMED, + [VENDOR_CLIENT_ADDRESS_HEADER]: CLAIMED, + ...(bearer ? { authorization: `Bearer ${bearer}` } : {}), + }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }, + { incoming: { socket: { remoteAddress: PEER } } }, + ); + await res.arrayBuffer(); + return [...s.handed]; +} + +describe('[#24877] every in-process bridge judges what it re-dispatches on the connection peer when no trusted proxy is declared', () => { + it('control — a request through the catch-all is judged on its connection peer', async () => { + const handed = await through('GET /get-session', { bearer: s.memberBearer }); + expect(handed.map((h) => h.path)).toEqual([`${BASE}/get-session`]); + expect(handed.map((h) => h.address)).toEqual([PEER]); + }); + + for (const bridge of BRIDGES) { + it(`a request through ${bridge.route} is judged on its connection peer when no trusted proxy is declared`, async () => { + const handed = await through(bridge.route, bridge.request(s)); + expect(handed.map((h) => h.path), 'the bridge re-dispatched to its vendor route').toContain( + `${BASE}${bridge.inner}`, + ); + expect(handed.map((h) => h.address)).toEqual(handed.map(() => PEER)); + }); + } +}); From 23b1608408c3a1a434a080df4b2436262e1d3311 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 14:35:27 +0000 Subject: [PATCH 2/5] chore(changeset): plugin-auth patch for the bridge transport fix Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude --- .changeset/24877-bridge-transport.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .changeset/24877-bridge-transport.md diff --git a/.changeset/24877-bridge-transport.md b/.changeset/24877-bridge-transport.md new file mode 100644 index 00000000000..6daf1b300ad --- /dev/null +++ b/.changeset/24877-bridge-transport.md @@ -0,0 +1,12 @@ +--- +'@objectstack/plugin-auth': patch +--- + +fix(plugin-auth): every in-process auth bridge hands better-auth the connection it was called on (#24877) + +Clause-②: no + +Eight auth routes answer by re-dispatching a request to better-auth in-process: `POST /api/v1/auth/admin/remove-user`, `/admin/has-permission`, `/admin/sso/register`, `/admin/sso/register-saml`, `/admin/sso/request-domain-verification`, `/admin/sso/verify-domain`, `/send-verification-email` and `/sys-session/revoke`. Each handed `AuthManager.handleRequest` the request alone. The client-address trust rule (`server.trustProxy`) establishes an address from the connection's peer when no trusted proxy is declared, and without the connection it had no peer to use for those requests. + +- **Now.** Each route hands `handleRequest` the connection it was called on, as the main auth mount does. Without a declared trusted proxy, the request it re-dispatches is judged on the connection's peer address, whatever the request's `X-Forwarded-For` or `X-Real-IP` headers say. +- **Unchanged.** What each route accepts or refuses, and every answer it gives. The main auth mount, and the `allowed_ip_ranges` allow-list in front of every auth route, already judged on the connection. From 3761b500f0534a3f8c0310222b4fab64030d8b6c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 16:12:07 +0000 Subject: [PATCH 3/5] fix(plugin-auth): re-dispatched requests carry the forwarding headers the trust rule reads The six bridges whose helper rebuilds the request before re-dispatching it now carry the incoming request's forwarding headers onto it, read from the one list the client-address trust rule reads (FORWARDED_CLIENT_ADDRESS_HEADERS, exported from client-address.ts, which resolveClientAddress now iterates). Under a declared server.trustProxy the re-dispatched request is judged on the forwarded client address; without one the rule ignores these headers. Pins extended with the declared-trust branch per bridge. Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude --- .changeset/24877-bridge-transport.md | 9 +- .../bridge-redispatch-transport.pin.test.ts | 136 ++++++++++++------ .../plugins/plugin-auth/src/client-address.ts | 21 ++- .../plugin-auth/src/register-sso-provider.ts | 38 +++-- .../src/send-verification-email.ts | 18 +++ .../plugin-auth/src/sys-session-revoke.ts | 23 ++- 6 files changed, 183 insertions(+), 62 deletions(-) diff --git a/.changeset/24877-bridge-transport.md b/.changeset/24877-bridge-transport.md index 6daf1b300ad..21c45487195 100644 --- a/.changeset/24877-bridge-transport.md +++ b/.changeset/24877-bridge-transport.md @@ -2,11 +2,12 @@ '@objectstack/plugin-auth': patch --- -fix(plugin-auth): every in-process auth bridge hands better-auth the connection it was called on (#24877) +fix(plugin-auth): every in-process auth bridge judges the request it re-dispatches on the same client-address facts as the request it serves (#24877) Clause-②: no -Eight auth routes answer by re-dispatching a request to better-auth in-process: `POST /api/v1/auth/admin/remove-user`, `/admin/has-permission`, `/admin/sso/register`, `/admin/sso/register-saml`, `/admin/sso/request-domain-verification`, `/admin/sso/verify-domain`, `/send-verification-email` and `/sys-session/revoke`. Each handed `AuthManager.handleRequest` the request alone. The client-address trust rule (`server.trustProxy`) establishes an address from the connection's peer when no trusted proxy is declared, and without the connection it had no peer to use for those requests. +Eight auth routes answer by re-dispatching a request to better-auth in-process: `POST /api/v1/auth/admin/remove-user`, `/admin/has-permission`, `/admin/sso/register`, `/admin/sso/register-saml`, `/admin/sso/request-domain-verification`, `/admin/sso/verify-domain`, `/send-verification-email` and `/sys-session/revoke`. `AuthManager.handleRequest` establishes the re-dispatched request's client address by the client-address trust rule: `X-Forwarded-For` (its first entry), then `X-Real-IP`, count only when the stack declares `server.trustProxy`, and otherwise the address is the connection's peer address. Each route handed `handleRequest` the request alone, without the connection it arrived on. Six of them also rebuild the request before re-dispatching it, and the rebuilt request did not carry the forwarding headers. -- **Now.** Each route hands `handleRequest` the connection it was called on, as the main auth mount does. Without a declared trusted proxy, the request it re-dispatches is judged on the connection's peer address, whatever the request's `X-Forwarded-For` or `X-Real-IP` headers say. -- **Unchanged.** What each route accepts or refuses, and every answer it gives. The main auth mount, and the `allowed_ip_ranges` allow-list in front of every auth route, already judged on the connection. +- **Now.** Each route hands `handleRequest` the connection it was called on, as the main auth mount does. Each rebuilt request carries the incoming request's `X-Forwarded-For` and `X-Real-IP`. Without a declared trusted proxy, the re-dispatched request is judged on the connection's peer address, whatever those headers say. With `server.trustProxy` declared, it is judged on the forwarded client address, the same address as the request the route serves. +- **For hosts.** `@objectstack/plugin-auth/client-address` exports `FORWARDED_CLIENT_ADDRESS_HEADERS`, the forwarding headers `resolveClientAddress` reads, in the order it reads them. A seam that builds a request of its own carries these headers by reading this list. +- **Unchanged.** What each route accepts or refuses, and every answer it gives. `resolveClientAddress` decides exactly as before. The main auth mount, and the `allowed_ip_ranges` allow-list in front of every auth route, already judged on the connection. diff --git a/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts b/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts index 702655bc31b..e9533683fe5 100644 --- a/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts +++ b/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts @@ -1,16 +1,19 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. // // [#24877] Every in-process bridge judges the request it re-dispatches by the -// platform's client-address trust rule (ADR-0069 D5/D7, `./client-address.ts`): +// platform's client-address trust rule (ADR-0069 D5/D7, `./client-address.ts`), +// on the same facts as the request it serves: // -// - forwarded headers count only when the deployment declares -// `server.trustProxy` (`AuthManagerOptions.trustProxy`); +// - forwarded headers (`FORWARDED_CLIENT_ADDRESS_HEADERS`) count only when the +// deployment declares `server.trustProxy` (`AuthManagerOptions.trustProxy`); // - otherwise the address is the transport's peer (`IHttpRequest.remoteAddress`). // // `AuthManager.handleRequest` establishes that address from the request AND the -// transport it is handed, so a bridge has to hand over the transport of the -// request it serves, as the catch-all does (`transportOf(c)` in -// `auth-plugin.ts`). One pin per bridge, each stating the rule for its route. +// transport it is handed. So a bridge hands over the transport of the request it +// serves, as the catch-all does (`transportOf(c)` in `auth-plugin.ts`), and a +// bridge whose helper builds the re-dispatched request carries the incoming +// request's forwarding headers onto it. One pin per bridge on each branch of the +// rule, each stating the rule for its route. // // What is read is the one channel the server's decision reaches the vendored // library through: the platform-owned header `handleRequest` writes on every @@ -23,11 +26,11 @@ // `AuthPlugin.registerAuthRoutes` mounted on a real Hono app in front of a real // `AuthManager` on the installed better-auth, with the transport peer supplied // the way `@hono/node-server` supplies it, as the Node request on -// `c.env.incoming`. No trusted proxy is declared. Every request also carries -// forwarded headers and the platform-owned header naming another address, set -// by the caller: none of them may decide the address. Nothing reaches the -// network: every re-dispatch below is answered by the vendor's own refusal -// before any discovery or DNS work. +// `c.env.incoming`. Two deployments: one declaring no trusted proxy, one +// declaring `trustProxy`. Every request also carries the platform-owned header +// naming another address, set by the caller, and it may decide nothing on either +// branch. Nothing reaches the network: every re-dispatch below is answered by +// the vendor's own refusal before any discovery or DNS work. // // The library's handler is observed through a plain instance wrapper, not // `vi.spyOn`: the imported `impersonation-bearer-rotation.test` registers @@ -51,9 +54,12 @@ const BASE = '/api/v1/auth'; const ADMIN_EMAIL = 'admin.24877@example.com'; const MEMBER_EMAIL = 'member.24877@example.com'; -/** Documentation ranges (RFC 5737): the connection peer, and what the caller's headers claim. */ +/** Documentation ranges (RFC 5737). No trusted proxy: the connection peer, and what the caller's headers claim. */ const PEER = '203.0.113.7'; const CLAIMED = '198.51.100.9'; +/** A declared trusted proxy: the forwarded client address, and the proxy hop that is the connection peer. */ +const FORWARDED_CLIENT = '192.0.2.44'; +const PROXY_HOP = '10.0.0.2'; /** `@better-auth/sso` reserves this id, and refuses it before any discovery or endpoint validation. */ const RESERVED_PROVIDER_ID = 'credential'; @@ -80,20 +86,20 @@ interface Stage { app: Hono; adminBearer: string; memberBearer: string; - /** A session of the member's own, minted for the revoke bridge alone. */ - revokeBearer: string; - revokeSessionId: string; + /** A fresh session of the member's own: the revoke bridge's owner lane ends the one it names. */ + revokeTarget(): Promise<{ bearer: string; sessionId: string }>; /** Every request the library's handler was handed, in order. */ handed: Handed[]; } -async function stage(): Promise { +async function stage(deployment: { trustProxy?: boolean }): Promise { const engine = createMemoryEngine(); const manager = new AuthManager({ secret: SECRET, baseUrl: ORIGIN, dataEngine: engine, plugins: { admin: true, sso: true, ssoDomainVerification: true }, + ...deployment, } as any); const direct = (path: string, body: unknown) => @@ -128,11 +134,13 @@ async function stage(): Promise { const memberBearer = await bearerFor(MEMBER_EMAIL); const memberSessions = () => ((engine.tables.get('sys_session') ?? []) as any[]).filter((r) => String(r.user_id) === memberId); - const before = memberSessions().length; - const revokeBearer = await bearerFor(MEMBER_EMAIL); - const minted = memberSessions().slice(before); - expect(minted, 'the revoke bridge’s own session').toHaveLength(1); - const revokeSessionId = String(minted[0].id); + const revokeTarget = async () => { + const before = memberSessions().length; + const bearer = await bearerFor(MEMBER_EMAIL); + const minted = memberSessions().slice(before); + expect(minted, 'the revoke bridge’s own session').toHaveLength(1); + return { bearer, sessionId: String(minted[0].id) }; + }; const auth = await (manager as any).getOrCreateAuth(); const handler = auth.handler; @@ -150,14 +158,14 @@ async function stage(): Promise { (plugin as any).authManager = manager; (plugin as any).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx); - return { app, adminBearer, memberBearer, revokeBearer, revokeSessionId, handed }; + return { app, adminBearer, memberBearer, revokeTarget, handed }; } /** A bridge: its mount, the vendor route it re-dispatches to, and a request that gets that far. */ interface Bridge { route: string; inner: string; - request: (s: Stage) => { bearer?: string; body: unknown }; + request: (s: Stage) => Promise<{ bearer?: string; body: unknown }>; } const BRIDGES: Bridge[] = [ @@ -165,19 +173,19 @@ const BRIDGES: Bridge[] = [ { route: 'POST /admin/remove-user', inner: '/admin/remove-user', - request: (s) => ({ bearer: s.adminBearer, body: { userId: 'usr_pin_24877_no_such_user' } }), + request: async (s) => ({ bearer: s.adminBearer, body: { userId: 'usr_pin_24877_no_such_user' } }), }, { route: 'POST /admin/has-permission', inner: '/admin/has-permission', // Anonymous: the delegate lane. - request: () => ({ body: { permissions: { user: ['list'] } } }), + request: async () => ({ body: { permissions: { user: ['list'] } } }), }, // Reshape-and-re-dispatch helpers (`register-sso-provider.ts`). { route: 'POST /admin/sso/register', inner: '/sso/register', - request: (s) => ({ + request: async (s) => ({ bearer: s.adminBearer, body: { providerId: RESERVED_PROVIDER_ID, @@ -191,7 +199,7 @@ const BRIDGES: Bridge[] = [ { route: 'POST /admin/sso/register-saml', inner: '/sso/register', - request: (s) => ({ + request: async (s) => ({ bearer: s.adminBearer, body: { providerId: RESERVED_PROVIDER_ID, @@ -205,31 +213,36 @@ const BRIDGES: Bridge[] = [ { route: 'POST /admin/sso/request-domain-verification', inner: '/sso/request-domain-verification', - request: (s) => ({ bearer: s.adminBearer, body: { providerId: UNKNOWN_PROVIDER_ID } }), + request: async (s) => ({ bearer: s.adminBearer, body: { providerId: UNKNOWN_PROVIDER_ID } }), }, { route: 'POST /admin/sso/verify-domain', inner: '/sso/verify-domain', - request: (s) => ({ bearer: s.adminBearer, body: { providerId: UNKNOWN_PROVIDER_ID } }), + request: async (s) => ({ bearer: s.adminBearer, body: { providerId: UNKNOWN_PROVIDER_ID } }), }, // The one-click resend (`send-verification-email.ts`): a session read, then the send. { route: 'POST /send-verification-email', inner: '/send-verification-email', - request: (s) => ({ bearer: s.memberBearer, body: {} }), + request: async (s) => ({ bearer: s.memberBearer, body: {} }), }, // The owner lane of the session revoke (`sys-session-revoke.ts`). { route: 'POST /sys-session/revoke', inner: '/revoke-session', - request: (s) => ({ bearer: s.revokeBearer, body: { [SYS_SESSION_REVOKE_ID_PARAM]: s.revokeSessionId } }), + request: async (s) => { + const target = await s.revokeTarget(); + return { bearer: target.bearer, body: { [SYS_SESSION_REVOKE_ID_PARAM]: target.sessionId } }; + }, }, ]; -let s: Stage; +let untrusted: Stage; +let trusted: Stage; beforeAll(async () => { - s = await stage(); + untrusted = await stage({}); + trusted = await stage({ trustProxy: true }); }); beforeEach(() => { @@ -240,8 +253,18 @@ afterEach(() => { vi.restoreAllMocks(); }); -/** Serve `route` (`METHOD /path`) from the connection peer, with the caller's headers naming another address. */ -async function through(route: string, { bearer, body }: { bearer?: string; body?: unknown }): Promise { +/** + * Serve `route` (`METHOD /path`) on stage `s` from connection peer `peer`, with + * the caller's `forwarding` headers and the platform-owned header naming + * another address; answer with every request the library's handler was handed. + */ +async function through( + s: Stage, + route: string, + peer: string, + forwarding: Record, + { bearer, body }: { bearer?: string; body?: unknown }, +): Promise { const [method, path] = route.split(' ') as [string, string]; s.handed.length = 0; const res = await s.app.request( @@ -251,29 +274,38 @@ async function through(route: string, { bearer, body }: { bearer?: string; body? headers: { 'content-type': 'application/json', origin: ORIGIN, - 'x-forwarded-for': CLAIMED, - 'x-real-ip': CLAIMED, + ...forwarding, [VENDOR_CLIENT_ADDRESS_HEADER]: CLAIMED, ...(bearer ? { authorization: `Bearer ${bearer}` } : {}), }, ...(body === undefined ? {} : { body: JSON.stringify(body) }), }, - { incoming: { socket: { remoteAddress: PEER } } }, + { incoming: { socket: { remoteAddress: peer } } }, ); await res.arrayBuffer(); return [...s.handed]; } +/** What a caller sends with no trusted proxy declared: forwarding headers naming another address. */ +const CLAIMED_FORWARDING = { 'x-forwarded-for': CLAIMED, 'x-real-ip': CLAIMED }; +/** What a declared trusted proxy forwards, in each of the two spellings the rule reads. */ +const PROXY_FORWARDING = [ + { 'x-forwarded-for': `${FORWARDED_CLIENT}, ${PROXY_HOP}` }, + { 'x-real-ip': FORWARDED_CLIENT }, +]; + describe('[#24877] every in-process bridge judges what it re-dispatches on the connection peer when no trusted proxy is declared', () => { it('control — a request through the catch-all is judged on its connection peer', async () => { - const handed = await through('GET /get-session', { bearer: s.memberBearer }); + const handed = await through(untrusted, 'GET /get-session', PEER, CLAIMED_FORWARDING, { + bearer: untrusted.memberBearer, + }); expect(handed.map((h) => h.path)).toEqual([`${BASE}/get-session`]); expect(handed.map((h) => h.address)).toEqual([PEER]); }); for (const bridge of BRIDGES) { it(`a request through ${bridge.route} is judged on its connection peer when no trusted proxy is declared`, async () => { - const handed = await through(bridge.route, bridge.request(s)); + const handed = await through(untrusted, bridge.route, PEER, CLAIMED_FORWARDING, await bridge.request(untrusted)); expect(handed.map((h) => h.path), 'the bridge re-dispatched to its vendor route').toContain( `${BASE}${bridge.inner}`, ); @@ -281,3 +313,27 @@ describe('[#24877] every in-process bridge judges what it re-dispatches on the c }); } }); + +describe('[#24877] every in-process bridge judges what it re-dispatches on the forwarded client address when a trusted proxy is declared', () => { + it('control — a request through the catch-all is judged on the forwarded client address', async () => { + for (const forwarding of PROXY_FORWARDING) { + const handed = await through(trusted, 'GET /get-session', PROXY_HOP, forwarding, { + bearer: trusted.memberBearer, + }); + expect(handed.map((h) => h.path), JSON.stringify(forwarding)).toEqual([`${BASE}/get-session`]); + expect(handed.map((h) => h.address), JSON.stringify(forwarding)).toEqual([FORWARDED_CLIENT]); + } + }); + + for (const bridge of BRIDGES) { + it(`a request through ${bridge.route} is judged on the forwarded client address when a trusted proxy is declared`, async () => { + for (const forwarding of PROXY_FORWARDING) { + const handed = await through(trusted, bridge.route, PROXY_HOP, forwarding, await bridge.request(trusted)); + expect(handed.map((h) => h.path), `the bridge re-dispatched to its vendor route (${JSON.stringify(forwarding)})`).toContain( + `${BASE}${bridge.inner}`, + ); + expect(handed.map((h) => h.address), JSON.stringify(forwarding)).toEqual(handed.map(() => FORWARDED_CLIENT)); + } + }); + } +}); diff --git a/packages/plugins/plugin-auth/src/client-address.ts b/packages/plugins/plugin-auth/src/client-address.ts index f7318b9d814..25f8bcd3de4 100644 --- a/packages/plugins/plugin-auth/src/client-address.ts +++ b/packages/plugins/plugin-auth/src/client-address.ts @@ -41,6 +41,17 @@ * dependency of every consumer of the entry. */ +/** + * The forwarded headers the rule reads, in the order it believes them, and + * only when the deployment declares `server.trustProxy`: `X-Forwarded-For` + * (its leftmost entry), then `X-Real-IP`. Lower-cased, as every adapter + * delivers them. {@link resolveClientAddress} reads exactly this list, so a + * seam that builds a request of its own from one it was handed — an + * in-process re-dispatch — carries the facts the rule reads by iterating it, + * never a hand-copied set of names (#24877). + */ +export const FORWARDED_CLIENT_ADDRESS_HEADERS: readonly string[] = Object.freeze(['x-forwarded-for', 'x-real-ip']); + /** The three facts the rule decides from. */ export interface ClientAddressInput { /** Request headers, lower-cased as every adapter delivers them. */ @@ -71,9 +82,13 @@ function firstHeader( * when it has established none. See the module header for the rule. */ export function resolveClientAddress(input: ClientAddressInput): string | undefined { - const forwarded = input.trustProxy - ? firstHeader(input.headers, 'x-forwarded-for') ?? firstHeader(input.headers, 'x-real-ip') - : undefined; + let forwarded: string | undefined; + if (input.trustProxy) { + for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS) { + forwarded = firstHeader(input.headers, name); + if (forwarded !== undefined) break; + } + } const address = forwarded ?? input.remoteAddress; return address ? address : undefined; } diff --git a/packages/plugins/plugin-auth/src/register-sso-provider.ts b/packages/plugins/plugin-auth/src/register-sso-provider.ts index 0868cf27d36..79c42511e4f 100644 --- a/packages/plugins/plugin-auth/src/register-sso-provider.ts +++ b/packages/plugins/plugin-auth/src/register-sso-provider.ts @@ -22,6 +22,7 @@ * (per-environment runtime) — mirroring `runSetInitialPassword`. */ +import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; import { readJsonRequestBody } from './request-json-body.js'; @@ -75,6 +76,7 @@ async function resolveActiveOrganizationId( if (cookie) h.set('cookie', cookie); const authz = headers.get('authorization'); if (authz) h.set('authorization', authz); + carryForwardedClientAddress(headers, h); // [#22398] A cookie request reads without renewal: this response's cookie // is never sent (`in-process-redispatch.ts`). const resp = await handle(new Request(inProcessRedispatchUrl(sessionUrl, h), { method: 'GET', headers: h })); @@ -201,12 +203,7 @@ export async function runRegisterSsoProviderFromForm( } catch { return { status: 400, body: { success: false, error: { code: 'INVALID_REQUEST', message: 'Bad request URL' } } }; } - const headers = new Headers({ 'content-type': 'application/json' }); - const cookie = request.headers.get('cookie'); - if (cookie) headers.set('cookie', cookie); - const authz = request.headers.get('authorization'); - if (authz) headers.set('authorization', authz); - headers.set('origin', request.headers.get('origin') || origin); + const headers = forwardAuthHeaders(request, origin); // Org-scope the provider to the caller's active organization (best-effort). // `@better-auth/sso`'s management endpoints (delete / update / domain @@ -307,12 +304,7 @@ export async function runRegisterSamlProviderFromForm( }; if (identifierFormat) samlConfig.identifierFormat = identifierFormat; - const headers = new Headers({ 'content-type': 'application/json' }); - const cookie = request.headers.get('cookie'); - if (cookie) headers.set('cookie', cookie); - const authz = request.headers.get('authorization'); - if (authz) headers.set('authorization', authz); - headers.set('origin', request.headers.get('origin') || origin); + const headers = forwardAuthHeaders(request, origin); // Org-scope to the caller's active org (best-effort) so any org owner/admin // can manage the provider — see the OIDC helper above. @@ -430,6 +422,27 @@ function rewriteSsoAdminUrl(request: Request, fromSuffix: RegExp, toPath: string } } +/** + * [#24877] Carry the forwarding facts the client-address trust rule reads + * (`FORWARDED_CLIENT_ADDRESS_HEADERS`) from the request a bridge serves onto + * the request it re-dispatches, as the incoming request had them. + * `AuthManager.handleRequest` judges the re-dispatched request by that rule, + * so behind a declared `server.trustProxy` it reads the forwarded client + * address the incoming request carried; without one the rule ignores these + * headers and judges on the connection peer. + */ +function carryForwardedClientAddress(from: Headers, to: Headers): void { + for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS) { + const value = from.get(name); + if (value !== null) to.set(name, value); + } +} + +/** + * The headers every re-dispatch below carries: the caller's credential + * (cookie / bearer), its `Origin` (or the request's own), and its forwarding + * facts. + */ function forwardAuthHeaders(request: Request, origin: string): Headers { const headers = new Headers({ 'content-type': 'application/json' }); const cookie = request.headers.get('cookie'); @@ -437,6 +450,7 @@ function forwardAuthHeaders(request: Request, origin: string): Headers { const authz = request.headers.get('authorization'); if (authz) headers.set('authorization', authz); headers.set('origin', request.headers.get('origin') || origin); + carryForwardedClientAddress(request.headers, headers); return headers; } diff --git a/packages/plugins/plugin-auth/src/send-verification-email.ts b/packages/plugins/plugin-auth/src/send-verification-email.ts index 14d37eb37cb..4e59b55cfcf 100644 --- a/packages/plugins/plugin-auth/src/send-verification-email.ts +++ b/packages/plugins/plugin-auth/src/send-verification-email.ts @@ -30,6 +30,7 @@ */ import type { AuthRequestHandler } from './register-sso-provider.js'; +import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; import { readJsonRequestBody } from './request-json-body.js'; @@ -42,6 +43,21 @@ export interface ResendVerificationEmailResult { const trimStr = (v: unknown): string => (typeof v === 'string' ? v.trim() : ''); +/** + * [#24877] Carry the forwarding facts the client-address trust rule reads + * (`FORWARDED_CLIENT_ADDRESS_HEADERS`) from the request this route serves onto + * a request it re-dispatches, as the incoming request had them, so + * `AuthManager.handleRequest` judges both on the same facts: the forwarded + * client address behind a declared `server.trustProxy`, the connection peer + * otherwise. + */ +function carryForwardedClientAddress(from: Headers, to: Headers): void { + for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS) { + const value = from.get(name); + if (value !== null) to.set(name, value); + } +} + /** * Resolve the caller's own email by re-dispatching a `/get-session` through the * same better-auth handler (best-effort). Returns `undefined` when there is no @@ -62,6 +78,7 @@ async function resolveSessionEmail( if (cookie) h.set('cookie', cookie); const authz = headers.get('authorization'); if (authz) h.set('authorization', authz); + carryForwardedClientAddress(headers, h); // [#22398] A cookie request reads without renewal: this response's cookie // is never sent (`in-process-redispatch.ts`). const resp = await handle(new Request(inProcessRedispatchUrl(sessionUrl, h), { method: 'GET', headers: h })); @@ -131,6 +148,7 @@ export async function runResendVerificationEmail( const authz = request.headers.get('authorization'); if (authz) headers.set('authorization', authz); headers.set('origin', request.headers.get('origin') || origin); + carryForwardedClientAddress(request.headers, headers); // Re-dispatch to the real better-auth route (the universal handler bypasses // this wrapper, so there is no recursion) with the resolved email. diff --git a/packages/plugins/plugin-auth/src/sys-session-revoke.ts b/packages/plugins/plugin-auth/src/sys-session-revoke.ts index c6f8029af07..4c9d2418030 100644 --- a/packages/plugins/plugin-auth/src/sys-session-revoke.ts +++ b/packages/plugins/plugin-auth/src/sys-session-revoke.ts @@ -70,6 +70,7 @@ import { SystemObjectName } from '@objectstack/spec/system'; import type { AuthSessionApi } from '@objectstack/spec/contracts'; import { inProcessSessionReadInput } from '@objectstack/types'; +import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; import { PLATFORM_ADMIN_REFUSAL_MESSAGES, @@ -208,11 +209,26 @@ export function sysSessionRevokeDepsFor(manager: SysSessionRevokeManager, basePa }; } +/** + * [#24877] Carry the forwarding facts the client-address trust rule reads + * (`FORWARDED_CLIENT_ADDRESS_HEADERS`) from the request this route serves onto + * the request the owner lane re-dispatches, as the incoming request had them, + * so `AuthManager.handleRequest` judges both on the same facts: the forwarded + * client address behind a declared `server.trustProxy`, the connection peer + * otherwise. + */ +function carryForwardedClientAddress(from: Headers, to: Headers): void { + for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS) { + const value = from.get(name); + if (value !== null) to.set(name, value); + } +} + /** * Owner lane: better-auth's `/revoke-session`, with the resolved token and the - * caller's own credentials. Only status and body come back from here, so the - * re-dispatch carries `disableRefresh` for a cookie request - * (`inProcessRedispatchUrl`). + * caller's own credentials and forwarding facts. Only status and body come + * back from here, so the re-dispatch carries `disableRefresh` for a cookie + * request (`inProcessRedispatchUrl`). */ async function revokeAsOwner( deps: SysSessionRevokeDeps, @@ -226,6 +242,7 @@ async function revokeAsOwner( const authorization = request.headers.get('authorization'); if (authorization) headers.set('authorization', authorization); headers.set('origin', request.headers.get('origin') || incoming.origin); + carryForwardedClientAddress(request.headers, headers); const url = new URL(`${deps.basePath}/revoke-session`, incoming.origin).href; const response = await deps.handle( From 486e46d1b30647b55405cf38f4a4d1680b400d74 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 16:24:39 +0000 Subject: [PATCH 4/5] test(plugin-auth): type the forwarding-header fixtures as header records The declared-trust pins passed an inferred union of two object literals (each with the other key optional-undefined) where through() takes a Record of strings: TS2345 twice under tsconfig.test.json. Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude --- .../plugin-auth/src/bridge-redispatch-transport.pin.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts b/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts index e9533683fe5..bc0859e97e2 100644 --- a/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts +++ b/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts @@ -287,9 +287,9 @@ async function through( } /** What a caller sends with no trusted proxy declared: forwarding headers naming another address. */ -const CLAIMED_FORWARDING = { 'x-forwarded-for': CLAIMED, 'x-real-ip': CLAIMED }; +const CLAIMED_FORWARDING: Record = { 'x-forwarded-for': CLAIMED, 'x-real-ip': CLAIMED }; /** What a declared trusted proxy forwards, in each of the two spellings the rule reads. */ -const PROXY_FORWARDING = [ +const PROXY_FORWARDING: Record[] = [ { 'x-forwarded-for': `${FORWARDED_CLIENT}, ${PROXY_HOP}` }, { 'x-real-ip': FORWARDED_CLIENT }, ]; From fbd92ad310df75fe938242f6d4af858f05b90f6b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 17:30:04 +0000 Subject: [PATCH 5/5] refactor(plugin-auth): the forwarding-header list and its one copy live in an internal module FORWARDED_CLIENT_ADDRESS_HEADERS moves off the published ./client-address entry into client-address-forwarding.ts, which is on no exports map and re-exported from neither the package root nor ./client-address. resolveClientAddress imports the list from it and decides as before. The three module-local copies of carryForwardedClientAddress are replaced by the one shared function beside the list. The changeset no longer names a subpath export. Claude-Session: https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y Co-authored-by: Claude --- .changeset/24877-bridge-transport.md | 1 - .../src/client-address-forwarding.ts | 42 +++++++++++++++++++ .../plugins/plugin-auth/src/client-address.ts | 18 ++++---- .../plugin-auth/src/register-sso-provider.ts | 18 +------- .../src/send-verification-email.ts | 17 +------- .../plugin-auth/src/sys-session-revoke.ts | 17 +------- 6 files changed, 52 insertions(+), 61 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/client-address-forwarding.ts diff --git a/.changeset/24877-bridge-transport.md b/.changeset/24877-bridge-transport.md index 21c45487195..dfd0f4ba66d 100644 --- a/.changeset/24877-bridge-transport.md +++ b/.changeset/24877-bridge-transport.md @@ -9,5 +9,4 @@ Clause-②: no Eight auth routes answer by re-dispatching a request to better-auth in-process: `POST /api/v1/auth/admin/remove-user`, `/admin/has-permission`, `/admin/sso/register`, `/admin/sso/register-saml`, `/admin/sso/request-domain-verification`, `/admin/sso/verify-domain`, `/send-verification-email` and `/sys-session/revoke`. `AuthManager.handleRequest` establishes the re-dispatched request's client address by the client-address trust rule: `X-Forwarded-For` (its first entry), then `X-Real-IP`, count only when the stack declares `server.trustProxy`, and otherwise the address is the connection's peer address. Each route handed `handleRequest` the request alone, without the connection it arrived on. Six of them also rebuild the request before re-dispatching it, and the rebuilt request did not carry the forwarding headers. - **Now.** Each route hands `handleRequest` the connection it was called on, as the main auth mount does. Each rebuilt request carries the incoming request's `X-Forwarded-For` and `X-Real-IP`. Without a declared trusted proxy, the re-dispatched request is judged on the connection's peer address, whatever those headers say. With `server.trustProxy` declared, it is judged on the forwarded client address, the same address as the request the route serves. -- **For hosts.** `@objectstack/plugin-auth/client-address` exports `FORWARDED_CLIENT_ADDRESS_HEADERS`, the forwarding headers `resolveClientAddress` reads, in the order it reads them. A seam that builds a request of its own carries these headers by reading this list. - **Unchanged.** What each route accepts or refuses, and every answer it gives. `resolveClientAddress` decides exactly as before. The main auth mount, and the `allowed_ip_ranges` allow-list in front of every auth route, already judged on the connection. diff --git a/packages/plugins/plugin-auth/src/client-address-forwarding.ts b/packages/plugins/plugin-auth/src/client-address-forwarding.ts new file mode 100644 index 00000000000..4e7a5f55640 --- /dev/null +++ b/packages/plugins/plugin-auth/src/client-address-forwarding.ts @@ -0,0 +1,42 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The forwarding facts the client-address trust rule reads + * (`./client-address.ts`, ADR-0069 D5/D7), and the one way a request this + * plugin re-dispatches in-process carries them (#24877). + * + * INTERNAL. This module is not on the package's `exports` map, and neither the + * package root (`index.ts`) nor the published `./client-address` entry + * re-exports it: nothing outside this package reads the list. + * + * ⛔ Keep this module free of imports. The published `./client-address` entry + * — the lean entry the runtime loads without the better-auth family — imports + * the list from here, so a value import here would become a load-time + * dependency of every consumer of that entry. + */ + +/** + * The forwarded headers the rule reads, in the order it believes them, and + * only when the deployment declares `server.trustProxy`: `X-Forwarded-For` + * (its leftmost entry), then `X-Real-IP`. Lower-cased, as every adapter + * delivers them. `resolveClientAddress` reads exactly this list. + */ +export const FORWARDED_CLIENT_ADDRESS_HEADERS: readonly string[] = Object.freeze(['x-forwarded-for', 'x-real-ip']); + +/** + * Carry the forwarding facts the rule reads ({@link FORWARDED_CLIENT_ADDRESS_HEADERS}) + * from the request a bridge serves onto the request it re-dispatches, as the + * incoming request had them. `AuthManager.handleRequest` judges the + * re-dispatched request by the rule, so it is judged on the same facts as the + * request the bridge serves: the forwarded client address behind a declared + * `server.trustProxy`, the connection peer otherwise, when the rule ignores + * these headers. The bridges' header builders (`register-sso-provider.ts`, + * `send-verification-email.ts`, `sys-session-revoke.ts`) call this; none of + * them names a header itself. + */ +export function carryForwardedClientAddress(from: Headers, to: Headers): void { + for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS) { + const value = from.get(name); + if (value !== null) to.set(name, value); + } +} diff --git a/packages/plugins/plugin-auth/src/client-address.ts b/packages/plugins/plugin-auth/src/client-address.ts index 25f8bcd3de4..95e87723dfe 100644 --- a/packages/plugins/plugin-auth/src/client-address.ts +++ b/packages/plugins/plugin-auth/src/client-address.ts @@ -38,19 +38,15 @@ * `./client-address` subpath entry, like `./rate-limit-storage`, so the runtime * reaches it without loading the better-auth family (see `tsup.config.ts`). * ⛔ Keep this module free of imports: a value import here becomes a load-time - * dependency of every consumer of the entry. + * dependency of every consumer of the entry. The one exception is the list of + * forwarded headers the rule reads, from `./client-address-forwarding.ts`, + * which is internal and itself import-free (#24877). */ -/** - * The forwarded headers the rule reads, in the order it believes them, and - * only when the deployment declares `server.trustProxy`: `X-Forwarded-For` - * (its leftmost entry), then `X-Real-IP`. Lower-cased, as every adapter - * delivers them. {@link resolveClientAddress} reads exactly this list, so a - * seam that builds a request of its own from one it was handed — an - * in-process re-dispatch — carries the facts the rule reads by iterating it, - * never a hand-copied set of names (#24877). - */ -export const FORWARDED_CLIENT_ADDRESS_HEADERS: readonly string[] = Object.freeze(['x-forwarded-for', 'x-real-ip']); +// [#24877] The forwarded headers this rule reads, in the order it believes +// them. The list lives in an internal module so the in-process re-dispatches +// carry exactly these facts without the list being published on this entry. +import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address-forwarding.js'; /** The three facts the rule decides from. */ export interface ClientAddressInput { diff --git a/packages/plugins/plugin-auth/src/register-sso-provider.ts b/packages/plugins/plugin-auth/src/register-sso-provider.ts index 79c42511e4f..92e68a76f1d 100644 --- a/packages/plugins/plugin-auth/src/register-sso-provider.ts +++ b/packages/plugins/plugin-auth/src/register-sso-provider.ts @@ -22,7 +22,7 @@ * (per-environment runtime) — mirroring `runSetInitialPassword`. */ -import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address.js'; +import { carryForwardedClientAddress } from './client-address-forwarding.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; import { readJsonRequestBody } from './request-json-body.js'; @@ -422,22 +422,6 @@ function rewriteSsoAdminUrl(request: Request, fromSuffix: RegExp, toPath: string } } -/** - * [#24877] Carry the forwarding facts the client-address trust rule reads - * (`FORWARDED_CLIENT_ADDRESS_HEADERS`) from the request a bridge serves onto - * the request it re-dispatches, as the incoming request had them. - * `AuthManager.handleRequest` judges the re-dispatched request by that rule, - * so behind a declared `server.trustProxy` it reads the forwarded client - * address the incoming request carried; without one the rule ignores these - * headers and judges on the connection peer. - */ -function carryForwardedClientAddress(from: Headers, to: Headers): void { - for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS) { - const value = from.get(name); - if (value !== null) to.set(name, value); - } -} - /** * The headers every re-dispatch below carries: the caller's credential * (cookie / bearer), its `Origin` (or the request's own), and its forwarding diff --git a/packages/plugins/plugin-auth/src/send-verification-email.ts b/packages/plugins/plugin-auth/src/send-verification-email.ts index 4e59b55cfcf..06170ee70ff 100644 --- a/packages/plugins/plugin-auth/src/send-verification-email.ts +++ b/packages/plugins/plugin-auth/src/send-verification-email.ts @@ -30,7 +30,7 @@ */ import type { AuthRequestHandler } from './register-sso-provider.js'; -import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address.js'; +import { carryForwardedClientAddress } from './client-address-forwarding.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; import { readJsonRequestBody } from './request-json-body.js'; @@ -43,21 +43,6 @@ export interface ResendVerificationEmailResult { const trimStr = (v: unknown): string => (typeof v === 'string' ? v.trim() : ''); -/** - * [#24877] Carry the forwarding facts the client-address trust rule reads - * (`FORWARDED_CLIENT_ADDRESS_HEADERS`) from the request this route serves onto - * a request it re-dispatches, as the incoming request had them, so - * `AuthManager.handleRequest` judges both on the same facts: the forwarded - * client address behind a declared `server.trustProxy`, the connection peer - * otherwise. - */ -function carryForwardedClientAddress(from: Headers, to: Headers): void { - for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS) { - const value = from.get(name); - if (value !== null) to.set(name, value); - } -} - /** * Resolve the caller's own email by re-dispatching a `/get-session` through the * same better-auth handler (best-effort). Returns `undefined` when there is no diff --git a/packages/plugins/plugin-auth/src/sys-session-revoke.ts b/packages/plugins/plugin-auth/src/sys-session-revoke.ts index 4c9d2418030..8011b99a40e 100644 --- a/packages/plugins/plugin-auth/src/sys-session-revoke.ts +++ b/packages/plugins/plugin-auth/src/sys-session-revoke.ts @@ -70,7 +70,7 @@ import { SystemObjectName } from '@objectstack/spec/system'; import type { AuthSessionApi } from '@objectstack/spec/contracts'; import { inProcessSessionReadInput } from '@objectstack/types'; -import { FORWARDED_CLIENT_ADDRESS_HEADERS } from './client-address.js'; +import { carryForwardedClientAddress } from './client-address-forwarding.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; import { PLATFORM_ADMIN_REFUSAL_MESSAGES, @@ -209,21 +209,6 @@ export function sysSessionRevokeDepsFor(manager: SysSessionRevokeManager, basePa }; } -/** - * [#24877] Carry the forwarding facts the client-address trust rule reads - * (`FORWARDED_CLIENT_ADDRESS_HEADERS`) from the request this route serves onto - * the request the owner lane re-dispatches, as the incoming request had them, - * so `AuthManager.handleRequest` judges both on the same facts: the forwarded - * client address behind a declared `server.trustProxy`, the connection peer - * otherwise. - */ -function carryForwardedClientAddress(from: Headers, to: Headers): void { - for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS) { - const value = from.get(name); - if (value !== null) to.set(name, value); - } -} - /** * Owner lane: better-auth's `/revoke-session`, with the resolved token and the * caller's own credentials and forwarding facts. Only status and body come