diff --git a/.changeset/24877-bridge-transport.md b/.changeset/24877-bridge-transport.md new file mode 100644 index 00000000000..dfd0f4ba66d --- /dev/null +++ b/.changeset/24877-bridge-transport.md @@ -0,0 +1,12 @@ +--- +'@objectstack/plugin-auth': patch +--- + +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`. `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. +- **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/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..bc0859e97e2 --- /dev/null +++ b/packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts @@ -0,0 +1,339 @@ +// 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`), +// on the same facts as the request it serves: +// +// - 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 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 +// 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`. 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 +// `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). 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'; +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 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(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) => + 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 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; + 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, 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) => Promise<{ 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: 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: async () => ({ body: { permissions: { user: ['list'] } } }), + }, + // Reshape-and-re-dispatch helpers (`register-sso-provider.ts`). + { + route: 'POST /admin/sso/register', + inner: '/sso/register', + request: async (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: async (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: async (s) => ({ bearer: s.adminBearer, body: { providerId: UNKNOWN_PROVIDER_ID } }), + }, + { + route: 'POST /admin/sso/verify-domain', + inner: '/sso/verify-domain', + 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: 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: async (s) => { + const target = await s.revokeTarget(); + return { bearer: target.bearer, body: { [SYS_SESSION_REVOKE_ID_PARAM]: target.sessionId } }; + }, + }, +]; + +let untrusted: Stage; +let trusted: Stage; + +beforeAll(async () => { + untrusted = await stage({}); + trusted = await stage({ trustProxy: true }); +}); + +beforeEach(() => { + vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); +afterEach(() => { + vi.restoreAllMocks(); +}); + +/** + * 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( + `${ORIGIN}${BASE}${path}`, + { + method, + headers: { + 'content-type': 'application/json', + origin: ORIGIN, + ...forwarding, + [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]; +} + +/** What a caller sends with no trusted proxy declared: forwarding headers naming another address. */ +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: Record[] = [ + { '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(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(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}`, + ); + expect(handed.map((h) => h.address)).toEqual(handed.map(() => PEER)); + }); + } +}); + +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-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 f7318b9d814..95e87723dfe 100644 --- a/packages/plugins/plugin-auth/src/client-address.ts +++ b/packages/plugins/plugin-auth/src/client-address.ts @@ -38,9 +38,16 @@ * `./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). */ +// [#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 { /** Request headers, lower-cased as every adapter delivers them. */ @@ -71,9 +78,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..92e68a76f1d 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 { carryForwardedClientAddress } from './client-address-forwarding.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,11 @@ function rewriteSsoAdminUrl(request: Request, fromSuffix: RegExp, toPath: string } } +/** + * 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 +434,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..06170ee70ff 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 { carryForwardedClientAddress } from './client-address-forwarding.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; import { readJsonRequestBody } from './request-json-body.js'; @@ -62,6 +63,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 +133,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..8011b99a40e 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 { carryForwardedClientAddress } from './client-address-forwarding.js'; import { inProcessRedispatchUrl } from './in-process-redispatch.js'; import { PLATFORM_ADMIN_REFUSAL_MESSAGES, @@ -210,9 +211,9 @@ export function sysSessionRevokeDepsFor(manager: SysSessionRevokeManager, basePa /** * 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 +227,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(