fix(plugin-auth): the auth IP allow-list judges the client address the server established

.changeset/24739-auth-ip-allowlist-trust-rule.md

@@ -0,0 +1,29 @@
+---
+'@objectstack/plugin-auth': patch
+'@objectstack/runtime': patch
+'@objectstack/cli': patch
+'@objectstack/service-settings': patch
+---
+
+fix(plugin-auth): the auth IP allow-list judges the client address the server established (#24739)
+
+Clause-②: no
+
+The `allowed_ip_ranges` auth setting (ADR-0069 D5) and the `last_login_ip` login audit (D7) now establish a request's client address by the same rule as the inbound rate limiter: `X-Forwarded-For` (its first entry), then `X-Real-IP`, count only when the stack declares `server.trustProxy`. Otherwise the address is the connection's peer address.
+
+**For operators who set `allowed_ip_ranges`:**
+
+- Behind a reverse proxy or load balancer, declare `server: { trustProxy: true }` in the stack before you upgrade. Without it, the allow-list judges the proxy's own address for every request.
+- A request whose client address cannot be established (the transport reports no peer address, and no forwarded header is trusted) is now refused with `403 IP_NOT_ALLOWED`. Before, it was admitted. The first such refusal logs a warning that names `allowed_ip_ranges` and `server.trustProxy`.
+- `CF-Connecting-IP` is no longer read. Behind Cloudflare, declare `server.trustProxy`, and the `X-Forwarded-For` it sets is used.
+- A dual-stack listener reports an IPv4 client in IPv4-mapped form (`::ffff:203.0.113.7`). That address is now matched as the IPv4 address it denotes.
+- With `allowed_ip_ranges` unset or empty, nothing changes.
+
+**`last_login_ip`** records the address the allow-list judged, or no address when none could be established. It is no longer read from a forwarded header the deployment does not trust.
+
+**By package:**
+
+- `@objectstack/plugin-auth`: the rule is published as the dependency-free `@objectstack/plugin-auth/client-address` entry (`resolveClientAddress`). `AuthPlugin` and `AuthManager` take a `trustProxy` option. `AuthManager.handleRequest` takes an optional second argument that carries the transport's `remoteAddress`.
+- `@objectstack/runtime`: the inbound rate limiter imports the rule from that entry instead of keeping its own copy. Its keys are unchanged.
+- `@objectstack/cli`: `objectstack serve` passes the stack's `server.trustProxy` to `AuthPlugin` as well as to the dispatcher. A host that composes `AuthPlugin` itself passes `trustProxy` the same way.
+- `@objectstack/service-settings`: the `allowed_ip_ranges` help text says which address is judged.

content/docs/permissions/authentication.mdx

@@ -590,7 +590,7 @@ often before the account-level lock engages.
 
 | Setting | Effect |
 |---|---|
-| `allowed_ip_ranges` | Restrict authenticated access to one or more IPv4 CIDR ranges. IP is extracted with trust-proxy awareness (`x-forwarded-for` / `cf-connecting-ip` / `x-real-ip`); requests whose IP can't be determined fail open. |
+| `allowed_ip_ranges` | Restrict authenticated access to one or more IPv4 CIDR ranges (or exact IPs). The address judged is the one the server establishes, by the same rule as the inbound rate limiter: `X-Forwarded-For` / `X-Real-IP` only when the stack declares `server.trustProxy`, the connection's peer address otherwise. Behind a reverse proxy, declare `server.trustProxy`, or the allow-list judges the proxy's own address. A request whose address cannot be established is refused with `403 IP_NOT_ALLOWED`, like one outside the ranges. `last_login_ip` records the same address. |
 
 > **How enforcement works.** Password expiry (an administrator-forced password
 > change included) and enforced MFA share a single *session-validation gate*.

packages/cli/src/commands/serve.ts

@@ -4255,6 +4255,12 @@ export default class Serve extends Command {
               appName: resolveDeploymentAppName((config as any).email ?? {}, process.env, (config as any).appName),
               socialProviders: Object.keys(socialProviders).length > 0 ? socialProviders : undefined,
               trustedOrigins: trustedOrigins.length ? trustedOrigins : undefined,
+              // The stack's `server.trustProxy`, the same authored value the
+              // dispatcher's inbound rate limiter receives below: whether the
+              // ADR-0069 D5 IP allow-list and the D7 login audit may believe
+              // X-Forwarded-For / X-Real-IP when establishing a client address.
+              // Undeclared, both judge the transport's peer address.
+              trustProxy: (config as any).server?.trustProxy === true,
               // Enable the admin plugin by default so the Setup app's
               // ban/unban/set-password/impersonate/set-role row actions
               // resolve to real endpoints. The plugin self-gates by role

packages/plugins/plugin-auth/package.json

@@ -16,6 +16,11 @@
       "types": "./dist/rate-limit-storage.d.ts",
       "import": "./dist/rate-limit-storage.mjs",
       "require": "./dist/rate-limit-storage.js"
+    },
+    "./client-address": {
+      "types": "./dist/client-address.d.ts",
+      "import": "./dist/client-address.mjs",
+      "require": "./dist/client-address.js"
     }
   },
   "scripts": {

packages/plugins/plugin-auth/src/auth-manager.test.ts

@@ -4822,10 +4822,26 @@ describe('AuthManager', () => {
       expect(m.isClientIpAllowed('8.8.8.8')).toBe(false);
     });
 
-    it('fails OPEN when the client IP cannot be determined (no proxy header)', () => {
+    it('refuses an address the server could not establish once ranges are configured', () => {
       const m = mgr({ allowedIpRanges: ['203.0.113.0/24'] });
-      expect(m.isClientIpAllowed(undefined)).toBe(true);
-      expect(m.isClientIpAllowed('')).toBe(true);
+      expect(m.isClientIpAllowed(undefined)).toBe(false);
+      expect(m.isClientIpAllowed('')).toBe(false);
+    });
+
+    it('ipMatchesRange reads an IPv4-mapped IPv6 address as the IPv4 address it denotes', () => {
+      expect(ipMatchesRange('::ffff:203.0.113.5', '203.0.113.0/24')).toBe(true);
+      expect(ipMatchesRange('::FFFF:203.0.114.5', '203.0.113.0/24')).toBe(false);
+      expect(ipMatchesRange('::ffff:10.0.0.1', '10.0.0.1')).toBe(true);
+      expect(ipMatchesRange('10.0.0.1', '::ffff:10.0.0.1')).toBe(true);
+      expect(ipMatchesRange('2001:db8::1', '2001:db8::1')).toBe(true);
+    });
+
+    it('resolveClientAddress believes forwarded headers only under trustProxy', () => {
+      const headers = { 'x-forwarded-for': '203.0.113.9, 10.0.0.2' };
+      expect(mgr({}).resolveClientAddress({ headers, remoteAddress: '10.0.0.2' })).toBe('10.0.0.2');
+      expect(mgr({ trustProxy: true }).resolveClientAddress({ headers, remoteAddress: '10.0.0.2' }))
+        .toBe('203.0.113.9');
+      expect(mgr({}).resolveClientAddress({ headers })).toBeUndefined();
     });
   });
 

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

@@ -127,6 +127,8 @@ import {
 } from './remove-member-permission-guard.js';
 import { isPlaceholderEmail } from './placeholder-email.js';
 import { readAccountInvitation, settleImportResetDelivery } from './account-invitation-marker.js';
+import { resolveClientAddress } from './client-address.js';
+import { clientAddressStateEntries, readRequestClientAddress } from './request-client-address.js';
 import {
   REVOKE_SESSION_NOT_FOUND_CODE,
   REVOKE_SESSION_NOT_FOUND_MESSAGE,
@@ -1028,13 +1030,35 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
 
   /**
    * ADR-0069 D5 — network gating. When non-empty, auth requests (sign-in,
-   * session) from a client IP outside these CIDR / exact ranges are rejected
-   * with `IP_NOT_ALLOWED` at the auth-route middleware. Requires a trusted proxy
-   * to set `x-forwarded-for` / `cf-connecting-ip`; fails OPEN when the client IP
-   * can't be determined (so a missing proxy header is a no-op, not a lockout).
+   * session) are judged at the auth-route middleware against these CIDR / exact
+   * ranges, and refused with `403 IP_NOT_ALLOWED` unless the client address is
+   * inside one.
+   *
+   * The address judged is the one the server ESTABLISHED, by the platform's
+   * client-address trust rule ({@link resolveClientAddress}, shared with the
+   * runtime's inbound rate limiter): forwarded headers only when
+   * {@link trustProxy} is declared, the transport's peer address otherwise.
+   * Behind a reverse proxy, declare `server.trustProxy`, or the allow-list
+   * judges the proxy's own address.
+   *
+   * A request whose address cannot be established (no peer address, no trusted
+   * forwarded header) is REFUSED with the same `403 IP_NOT_ALLOWED`, and the
+   * first such refusal logs a warning naming this setting and `server.trustProxy`.
+   * Unset or empty: no restriction, and nothing is judged.
    */
   allowedIpRanges?: string[];
 
+  /**
+   * The deployment's `server.trustProxy`: believe `X-Forwarded-For` /
+   * `X-Real-IP` when establishing a request's client address for the ADR-0069
+   * D5 allow-list ({@link allowedIpRanges}) and the D7 login audit
+   * (`last_login_ip`). The same authored value the runtime's inbound rate
+   * limiter reads; `objectstack serve` passes it to both. A host that composes
+   * `AuthPlugin` itself passes the value it gives the dispatcher.
+   * @default false
+   */
+  trustProxy?: boolean;
+
   /**
    * ADR-0069 D2 — better-auth-native per-IP rate limiting, passed through to
    * better-auth's core `rateLimit`. The settings bind tightens `customRules`
@@ -1117,22 +1141,41 @@ function ipv4ToInt(ip: string): number | null {
   return (((p[0] << 24) >>> 0) + (p[1] << 16) + (p[2] << 8) + p[3]) >>> 0;
 }
 
-/** ADR-0069 D5 — does `ip` match `range` (IPv4 CIDR `a.b.c.d/n`, or exact IP)? */
+/**
+ * `::ffff:203.0.113.7` → `203.0.113.7`; anything else is returned trimmed.
+ *
+ * A dual-stack listener reports an IPv4 peer in this IPv4-mapped IPv6 form, and
+ * the ADR-0069 D5 allow-list judges the transport's peer address when no proxy
+ * is trusted. So both sides of a comparison are read as the IPv4 address they
+ * denote, or an IPv4 range could never admit an IPv4 client on such a socket.
+ */
+function unmapIpv4(address: string): string {
+  const trimmed = address.trim();
+  const mapped = /^::ffff:(\d{1,3}(?:\.\d{1,3}){3})$/i.exec(trimmed);
+  return mapped ? mapped[1] : trimmed;
+}
+
+/**
+ * ADR-0069 D5 — does `ip` match `range` (IPv4 CIDR `a.b.c.d/n`, or exact IP)?
+ * An IPv4-mapped IPv6 address on either side compares as its IPv4 address.
+ */
 export function ipMatchesRange(ip: string, range: string): boolean {
-  const r = (range || '').trim();
+  const r = unmapIpv4(range || '');
   if (!r) return false;
+  const address = unmapIpv4(ip);
   if (r.includes('/')) {
-    const [base, bitsStr] = r.split('/');
+    const [rawBase, bitsStr] = r.split('/');
+    const base = unmapIpv4(rawBase);
     const bits = Number(bitsStr);
-    const ipInt = ipv4ToInt(ip);
+    const ipInt = ipv4ToInt(address);
     const baseInt = ipv4ToInt(base);
     if (ipInt === null || baseInt === null || !(bits >= 0 && bits <= 32)) {
-      return ip.trim() === base.trim(); // non-IPv4-CIDR → exact-match fallback
+      return address === base; // non-IPv4-CIDR → exact-match fallback
     }
     const mask = bits === 0 ? 0 : (~0 << (32 - bits)) >>> 0;
     return ((ipInt & mask) >>> 0) === ((baseInt & mask) >>> 0);
   }
-  return ip.trim() === r;
+  return address === r;
 }
 
 /**
@@ -2839,18 +2882,12 @@ export class AuthManager {
                 if (typeof uid === 'string') await this.enforceConcurrentCap(uid);
                 // ADR-0069 D7 — login audit: stamp last_login_at/ip on success.
                 // Independent of lockout config (recordSignInOutcome no-ops when
-                // lockout is off). IP from the trusted forwarded headers, same
-                // precedence as the D5 allow-list middleware.
+                // lockout is off). The IP is the address `handleRequest`
+                // established for this request by the client-address trust rule,
+                // the same address the D5 allow-list judged; none when it was
+                // undetermined. ⛔ Never re-read from a header here.
                 if (typeof uid === 'string') {
-                  const hdr = (k: string): string =>
-                    ((ctx?.headers?.get?.(k) ?? ctx?.request?.headers?.get?.(k)) as string) || '';
-                  const fwd = hdr('x-forwarded-for');
-                  const ip =
-                    (fwd && fwd.split(',')[0].trim()) ||
-                    hdr('cf-connecting-ip') ||
-                    hdr('x-real-ip') ||
-                    undefined;
-                  await this.stampLastLogin(uid, ip);
+                  await this.stampLastLogin(uid, await readRequestClientAddress());
                 }
               }
             }
@@ -6437,9 +6474,27 @@ export class AuthManager {
    * response status and log server errors so they are not silently swallowed.
    *
    * @param request - Web standard Request object
+   * @param transport - What the transport knows about the request that a Fetch
+   *   `Request` does not carry: `remoteAddress` is its peer address
+   *   (`IHttpRequest.remoteAddress`). Omitted, the request's client address is
+   *   established from trusted forwarded headers alone, and is undetermined
+   *   when `trustProxy` is not declared.
    * @returns Web standard Response object
    */
-  async handleRequest(request: Request): Promise<Response> {
+  async handleRequest(request: Request, transport?: { remoteAddress?: string }): Promise<Response> {
+    // ADR-0069 D7 — the client address this request's sign-in audit stamps,
+    // established ONCE, here, by the same trust rule the D5 allow-list judges
+    // with (`resolveClientAddress`). Read before any rewrite below, and carried
+    // to the after-hook in better-auth's request state (`request-client-address.ts`).
+    const clientHeaders: Record<string, string> = {};
+    request.headers.forEach((value, name) => {
+      clientHeaders[name] = value;
+    });
+    const clientAddress = this.resolveClientAddress({
+      headers: clientHeaders,
+      ...(transport?.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}),
+    });
+
     // Dev DX: better-auth's CSRF protection rejects state-changing requests
     // (e.g. `/sign-in/email`) with a 403 when neither `Origin` nor `Referer`
     // is present. Browsers always send one; non-browser API clients (curl,
@@ -6515,7 +6570,10 @@ export class AuthManager {
     // The previous single call site flattened it with the `await` below.
     const runHandler = async (): Promise<Response> =>
       await runWithAuthActorScope(() =>
-        runWithRequestState(new WeakMap(), () => auth.handler(request)),
+        runWithRequestState(
+          new WeakMap<object, unknown>(clientAddressStateEntries(clientAddress)),
+          () => auth.handler(request),
+        ),
       );
 
     // [#7724] A subject-erasure request is ONE unit of work, and better-auth
@@ -9598,17 +9656,47 @@ export class AuthManager {
 
   /**
    * ADR-0069 D5 — is `ip` within the configured allow-list? True (allow) when no
-   * ranges are configured, OR when the IP can't be determined (fail-open so a
-   * misconfigured proxy never locks everyone out — an admin enabling this must
-   * ensure forwarded headers are trusted). Supports IPv4 CIDR + exact IPv4/IPv6.
+   * ranges are configured: nothing is judged then. With ranges configured, true
+   * only for an address inside one of them, and FALSE for an undetermined
+   * address (`undefined` / empty): an allow-list is a gate, and a gate that
+   * opens for "unknown" is open to anyone who can make themselves unknown.
+   * Supports IPv4 CIDR + exact IPv4/IPv6 (IPv4-mapped IPv6 compares as IPv4).
+   *
+   * `ip` must be the address the server established
+   * ({@link resolveClientAddress}), never a header read by the caller.
    */
   public isClientIpAllowed(ip: string | undefined): boolean {
     const ranges = this.config.allowedIpRanges;
     if (!ranges || ranges.length === 0) return true;
-    if (!ip) return true; // undetermined → fail-open
+    if (!ip) return false; // undetermined → refused
     return ranges.some((r) => ipMatchesRange(ip, r));
   }
 
+  /**
+   * The client address this deployment establishes for a request: the
+   * platform's client-address trust rule (`./client-address.ts`, shared with the
+   * runtime's inbound rate limiter) applied with this deployment's
+   * {@link AuthManagerOptions.trustProxy}. Forwarded headers count only when
+   * `trustProxy` is declared; otherwise the transport's `remoteAddress`;
+   * `undefined` when neither is available. The ADR-0069 D5 allow-list judges
+   * this address and the D7 login audit stamps it.
+   */
+  public resolveClientAddress(transport: {
+    headers?: Record<string, string | string[] | undefined>;
+    remoteAddress?: string;
+  }): string | undefined {
+    return resolveClientAddress({
+      ...(transport.headers ? { headers: transport.headers } : {}),
+      ...(transport.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}),
+      trustProxy: this.config.trustProxy === true,
+    });
+  }
+
+  /** Whether this deployment declared `server.trustProxy` ({@link AuthManagerOptions.trustProxy}). */
+  public trustsForwardedHeaders(): boolean {
+    return this.config.trustProxy === true;
+  }
+
   /**
    * Returns the data engine wired into this auth manager. Used by route
    * handlers (e.g. bootstrap-status) that need to query identity tables