Skip to content

Commit 2941a19

Browse files
fix(plugin-auth): every in-process auth bridge hands handleRequest the request's transport (#24931)
Fixes #24877 Clause-②: no ## What changes `AuthManager.handleRequest(request, transport?)` establishes a request's client address by the platform's client-address trust rule (ADR-0069 D5/D7, `resolveClientAddress`, `server.trustProxy`): forwarded headers only when the deployment declares a trusted proxy, the transport's peer address otherwise. Every in-process bridge in `auth-plugin.ts` that re-dispatches through `handleRequest` now judges the re-dispatched request on the same facts as the request it serves, on both branches of that rule: - **The transport.** Each bridge hands `handleRequest` the transport of the request it serves, as the catch-all auth mount does (`transportOf(c)`). - **The forwarding headers.** The six bridges whose helper rebuilds the request before re-dispatching it now carry the incoming request's forwarding headers onto it. The list is the rule's own, `FORWARDED_CLIENT_ADDRESS_HEADERS`. `resolveClientAddress` now iterates that same list, and the helpers copy it through one shared function, so no helper names a header itself. Without a declared trusted proxy, the rule ignores those headers and the re-dispatched request is judged on the connection peer. With one, it is judged on the forwarded client address. | Bridge | How it reaches `handleRequest` | Transport | Forwarding headers | |---|---|---|---| | `POST /admin/remove-user` | delegates the request itself | `handleRequest(c.req.raw, transportOf(c))` | already on the request | | `POST /admin/has-permission` (delegate lane) | delegates the request itself | `handleRequest(c.req.raw, transportOf(c))` | already on the request | | `POST /admin/sso/register` | `runRegisterSsoProviderFromForm(handle, …)` | `handle` is `handleWithTransportOf(c)` | `forwardAuthHeaders` (now also used here), and the session read | | `POST /admin/sso/register-saml` | `runRegisterSamlProviderFromForm(handle, …)` | `handle` is `handleWithTransportOf(c)` | `forwardAuthHeaders` (now also used here), and the session read | | `POST /admin/sso/request-domain-verification` | `runRequestDomainVerification(handle, …)` | `handle` is `handleWithTransportOf(c)` | `forwardAuthHeaders` | | `POST /admin/sso/verify-domain` | `runVerifyDomain(handle, …)` | `handle` is `handleWithTransportOf(c)` | `forwardAuthHeaders` | | `POST /send-verification-email` | `runResendVerificationEmail(handle, …)` | `handle` is `handleWithTransportOf(c)` | the session read and the send | | `POST /sys-session/revoke` | `sysSessionRevokeDepsFor(manager, …)`, whose `handle` calls `manager.handleRequest` | the manager slice's `handleRequest` is `handleWithTransportOf(c)` | the owner lane's re-dispatch | `handleWithTransportOf(c)` is one helper beside `gateAdmin`: `(request) => this.authManager!.handleRequest(request, transportOf(c))`. The list and the one copy function, `carryForwardedClientAddress(from, to)`, live in a new internal module, `client-address-forwarding.ts`. That module is import-free and is not on the package's `exports` map. Neither the package root nor the published `./client-address` entry re-exports it, so this PR adds no published export: the built `./client-address` entry still exports `resolveClientAddress` alone. `client-address.ts` imports the list for `resolveClientAddress`. The three helper modules call the shared function: twice in `register-sso-provider.ts` (`forwardAuthHeaders` and the session read), twice in `send-verification-email.ts` (the session read and the send), and once in `sys-session-revoke.ts` (the owner lane). The OIDC and SAML register builders were byte-identical to `forwardAuthHeaders` and now call it. `transportOf` and `handleRequest` are unchanged. `resolveClientAddress` decides exactly as before: it reads `X-Forwarded-For`, then `X-Real-IP`, now from the list rather than from two literals. ### The eighth bridge The card lists seven sites, from `git grep -n "handleRequest(" -- packages/plugins/plugin-auth/src/auth-plugin.ts`. `POST /sys-session/revoke` is an eighth. Its owner lane re-dispatches through `handleRequest` one hop away, inside `sysSessionRevokeDepsFor` (`sys-session-revoke.ts`: `handle: (request) => manager.handleRequest(request)`), so that grep does not list it. It is the same rule and the same mechanical fix, so it is fixed here. The mount hands the factory a manager slice whose `handleRequest` carries the transport. The seat accepted it and amended the claim's surface. ## Pins `packages/plugins/plugin-auth/src/bridge-redispatch-transport.pin.test.ts` uses the D5/D7 real-mount fixture from `client-address-vendor-trust-rule.test.ts`: `AuthPlugin.registerAuthRoutes` on a real Hono app in front of a real `AuthManager` on the installed better-auth, with the peer on `c.env.incoming`. It runs two deployments: one declaring no trusted proxy, one declaring `trustProxy`. Each pin reads the client address carried by every request the bridge hands the vendored library's handler. That address is the platform-owned header `handleRequest` writes from the address it established. The bridge's own inner route must be among those requests, so a bridge that never reached the library cannot pass vacuously. Every request also carries the platform-owned header naming another address, and on neither branch may it decide anything. No trusted proxy declared (the caller's forwarding headers name another address): - control: a request through the catch-all is judged on its connection peer - a request through ROUTE is judged on its connection peer when no trusted proxy is declared, once for each of the eight bridges A trusted proxy declared (the connection peer is the proxy hop; each pin sends `X-Forwarded-For` and, separately, `X-Real-IP`): - control: a request through the catch-all is judged on the forwarded client address - a request through ROUTE is judged on the forwarded client address when a trusted proxy is declared, once for each of the eight bridges ## Ablation Each leg ran through `scripts/ablation-replace.mjs` with the anchor hit count declared and the mutation confirmed on disk. Legs A to C ran at `486e46d1b`; `auth-plugin.ts` and the pin file are byte-identical at this PR's head. Legs D to G ran at this PR's head, `fbd92ad31`. Each restore was proven: blob equals HEAD's, `git diff HEAD` is empty, and the tree is clean. The test imports its subjects from `src/`, so no `dist/` sits on the path. | Leg | Forbidden behaviour put back | Red (`Tests N failed \| M passed (28)`) | |---|---|---| | A | `handleWithTransportOf` drops the transport | 6: the no-trust pins of sso/register, sso/register-saml, request-domain-verification, verify-domain, send-verification-email and sys-session/revoke | | B | the two delegate sites drop `transportOf(c)` (2 hits) | 2: the no-trust pins of remove-user and has-permission | | C | the revoke mount goes back to `sysSessionRevokeDepsFor(this.authManager!, basePath)` | 1: the no-trust pin of sys-session/revoke | | D | `register-sso-provider.ts` stops calling the shared copy (2 calls) | 4: the trusted-proxy pins of sso/register, sso/register-saml, request-domain-verification and verify-domain | | E | `send-verification-email.ts` stops calling it (2 calls) | 1: the trusted-proxy pin of send-verification-email | | F | `sys-session-revoke.ts` stops calling it (1 call) | 1: the trusted-proxy pin of sys-session/revoke | | G | the shared `carryForwardedClientAddress` copies nothing | 6: the trusted-proxy pins of the six helper bridges | Every other pin, and both controls, stayed green in every leg. ## Verification All readings at `fbd92ad31`: this branch with `origin/main` `74fcec1ed` merged. No later `main` commit touches `plugin-auth` or the lockfile. - `pnpm --filter '@objectstack/plugin-auth...' build`, then `pnpm --filter @objectstack/plugin-auth run typecheck`: exit 0. That covers `tsc --noEmit`, the examples config and `check:test-typecheck: OK`, whose debt ledger is unchanged at 10 files / 88 errors / 23 signatures. The pin file is inside that program and has no entry in the ledger. - The built entries were read back. `dist/client-address.js` and `dist/client-address.mjs` export `resolveClientAddress` alone and require nothing. The package root exports neither the list nor the copy. Spot checks of `resolveClientAddress` gave the same answers as before: the chain's first entry, then `X-Real-IP`, the peer when trust is undeclared, and `undefined` when neither is present. - `pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2`: `Test Files 172 passed (172)`, `Tests 3309 passed | 10 skipped (3319)`. - `node scripts/pm/dispatch-gates.mjs --commands` with no paths printed 72 commands. Each ran with its exit code captured before any pipe. 71 exited 0 on the first pass. `check:dual-build-cjs-loads` exited 3 (`PREREQUISITE NOT MET`: no workspace `dist/` yet in this fresh worktree). After a later gate in the same pass had built the workspace, it re-ran and exited 0. `--ran` reconciled them: `72 derived famil(ies) accounted for — 72 run, 0 NOT-MEASURED`. - `check:published-readme-exports` exited 0, with 0 entries in its baseline. The roster gates under touched directories also exited 0: `check-changeset-fixed`, `check:authz-resolver`, `check:error-code-casing` and `check:filter-alias-parity`. eslint (`--no-inline-config`) on the seven changed source files exited 0. - CI on `fbd92ad31`: 41 check-runs, 34 success and 7 skipped, among them `Lint & Repo Gates`, `TypeScript Type Check`, `Test Core`, `Build Core` and `Governed Surface Queue Guard`. ## Acceptance notes - `sysSessionRevokeDepsFor` could take the transport itself. That edit lands in the factory's signature, outside this card's surface. The slice keeps the transport change in `auth-plugin.ts`. - `IAuthService.handleRequest(request)` in the spec contract has no transport parameter, and two callers outside `plugin-auth` hand the request alone. Their reach is unmeasured, and both sit in landings this card excludes. Carrier: none. - No new gate. The pins sit beside the code, and a bridge added later is covered only when its PR pins it. --- _Generated by [Claude Code](https://claude.ai/code/session_01UmqzD97e14iws6hmVhgZ5y)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 19e5338 commit 2941a19

8 files changed

Lines changed: 465 additions & 28 deletions
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/plugin-auth': patch
3+
---
4+
5+
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)
6+
7+
Clause-②: no
8+
9+
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.
10+
11+
- **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.
12+
- **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.

‎packages/plugins/plugin-auth/src/auth-plugin.ts‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ import { registerMemberRoleCanonicalization } from './member-role-canonical.js';
6969
import { SYS_USER_PROFILE_EDIT_FIELDS } from './sys-user-writable-fields.js';
7070
import { MANAGED_EXTENSION_EDITABLE_FIELDS } from './managed-extension-fields.js';
7171
import { runSetInitialPassword } from './set-initial-password.js';
72-
import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain } from './register-sso-provider.js';
72+
import { runRegisterSsoProviderFromForm, runRegisterSamlProviderFromForm, runRequestDomainVerification, runVerifyDomain, type AuthRequestHandler } from './register-sso-provider.js';
7373
import { runResendVerificationEmail } from './send-verification-email.js';
7474
import { runSysSessionRevoke, sysSessionRevokeDepsFor } from './sys-session-revoke.js';
7575
import { readJsonRequestBody } from './request-json-body.js';
@@ -2960,6 +2960,21 @@ export class AuthPlugin implements Plugin {
29602960
return verdict.actor;
29612961
};
29622962

2963+
// ── [#24877] ADR-0069 D5/D7 — better-auth's universal handler for the
2964+
// request a raw mount below serves, WITH that request's transport. Every
2965+
// in-process bridge re-dispatches through `AuthManager.handleRequest` with
2966+
// the transport of the request it serves — a bridge that hands a helper a
2967+
// handler takes it from here; one that delegates the request itself passes
2968+
// `transportOf(c)` inline, as the catch-all at the end of this method
2969+
// does — so `handleRequest` establishes the re-dispatched request's client
2970+
// address by the platform's trust rule (`resolveClientAddress`): forwarded
2971+
// headers only when the deployment declares `server.trustProxy`, the
2972+
// connection peer otherwise. `handleRequest(req)` alone leaves the rule no
2973+
// peer to judge on. Pinned per bridge by
2974+
// `bridge-redispatch-transport.pin.test.ts`.
2975+
const handleWithTransportOf = (c: any): AuthRequestHandler => (request) =>
2976+
this.authManager!.handleRequest(request, transportOf(c));
2977+
29632978
// ────────────────────────────────────────────────────────────────────
29642979
// SSO admin: register an external OIDC IdP from the flat metadata form
29652980
// (ADR-0135 D6). `@better-auth/sso`'s POST /sso/register expects the protocol
@@ -2993,7 +3008,7 @@ export class AuthPlugin implements Plugin {
29933008
const gated = await gateAdmin(c);
29943009
if (gated instanceof Response) return gated;
29953010
const { status, body } = await runRegisterSsoProviderFromForm(
2996-
(req) => this.authManager!.handleRequest(req),
3011+
handleWithTransportOf(c),
29973012
c.req.raw,
29983013
);
29993014
return c.json(body, status as any);
@@ -3293,7 +3308,7 @@ export class AuthPlugin implements Plugin {
32933308
try {
32943309
const gated = await gateAdmin(c);
32953310
if (gated instanceof Response) return gated;
3296-
return await this.authManager!.handleRequest(c.req.raw);
3311+
return await this.authManager!.handleRequest(c.req.raw, transportOf(c));
32973312
} catch (error) {
32983313
const err = error instanceof Error ? error : new Error(String(error));
32993314
ctx.logger.error('[AuthPlugin] admin/remove-user failed', err);
@@ -3370,7 +3385,7 @@ export class AuthPlugin implements Plugin {
33703385
return c.json(answer, 200);
33713386
}
33723387
}
3373-
return await this.authManager!.handleRequest(c.req.raw);
3388+
return await this.authManager!.handleRequest(c.req.raw, transportOf(c));
33743389
} catch (error) {
33753390
const err = error instanceof Error ? error : new Error(String(error));
33763391
ctx.logger.error('[AuthPlugin] admin/has-permission failed', err);
@@ -3493,7 +3508,7 @@ export class AuthPlugin implements Plugin {
34933508
const gated = await gateAdmin(c);
34943509
if (gated instanceof Response) return gated;
34953510
const { status, body } = await runRegisterSamlProviderFromForm(
3496-
(req) => this.authManager!.handleRequest(req),
3511+
handleWithTransportOf(c),
34973512
c.req.raw,
34983513
);
34993514
return c.json(body, status as any);
@@ -3521,7 +3536,7 @@ export class AuthPlugin implements Plugin {
35213536
const gated = await gateAdmin(c);
35223537
if (gated instanceof Response) return gated;
35233538
const { status, body } = await runRequestDomainVerification(
3524-
(req) => this.authManager!.handleRequest(req),
3539+
handleWithTransportOf(c),
35253540
c.req.raw,
35263541
{ domainVerificationEnabled: this.authManager!.isSsoDomainVerificationEnabled() },
35273542
);
@@ -3539,7 +3554,7 @@ export class AuthPlugin implements Plugin {
35393554
const gated = await gateAdmin(c);
35403555
if (gated instanceof Response) return gated;
35413556
const { status, body } = await runVerifyDomain(
3542-
(req) => this.authManager!.handleRequest(req),
3557+
handleWithTransportOf(c),
35433558
c.req.raw,
35443559
{ domainVerificationEnabled: this.authManager!.isSsoDomainVerificationEnabled() },
35453560
);
@@ -3671,10 +3686,25 @@ export class AuthPlugin implements Plugin {
36713686
// judge). Same family as `sys-oauth-application/register` above: an
36723687
// object-scoped mount outside `/admin/`, because it admits the owner.
36733688
// See `sys-session-revoke.ts` for the lanes and refusals.
3689+
//
3690+
// [#24877] The owner lane's re-dispatch goes through the manager slice's
3691+
// `handleRequest`, so the slice handed over is the manager's own seams
3692+
// with `handleRequest` bound to this request's transport
3693+
// (`handleWithTransportOf` above) — the same handler every other bridge
3694+
// re-dispatches through.
36743695
rawApp.post(`${basePath}/sys-session/revoke`, async (c: any) => {
36753696
try {
3697+
const manager = this.authManager!;
36763698
const { status, body } = await runSysSessionRevoke(
3677-
sysSessionRevokeDepsFor(this.authManager!, basePath),
3699+
sysSessionRevokeDepsFor(
3700+
{
3701+
getApi: () => manager.getApi(),
3702+
getAuthContext: () => manager.getAuthContext(),
3703+
getDataEngine: () => manager.getDataEngine(),
3704+
handleRequest: handleWithTransportOf(c),
3705+
},
3706+
basePath,
3707+
),
36783708
c.req.raw,
36793709
);
36803710
return c.json(body, status as any);
@@ -3702,7 +3732,7 @@ export class AuthPlugin implements Plugin {
37023732
rawApp.post(`${basePath}/send-verification-email`, async (c: any) => {
37033733
try {
37043734
const { status, body } = await runResendVerificationEmail(
3705-
(req) => this.authManager!.handleRequest(req),
3735+
handleWithTransportOf(c),
37063736
c.req.raw,
37073737
);
37083738
return c.json(body, status);

0 commit comments

Comments
 (0)