Skip to content

Commit 69328bd

Browse files
committed
fix(plugin-auth): the IP allow-list judges the address the server established
The ADR-0069 D5 allow-list and the D7 login audit now establish a request's client address by the platform's one client-address trust rule, the rule the runtime's inbound rate limiter already used: forwarded headers only when the stack declares server.trustProxy, the transport's peer address otherwise. With ranges configured, an address that cannot be established is refused (403 IP_NOT_ALLOWED) and explained once in the operator log. The rule moves unchanged from runtime into plugin-auth's dependency-free ./client-address entry (runtime depends on plugin-auth), and the runtime imports it back. objectstack serve hands server.trustProxy to AuthPlugin as well as to the dispatcher. Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 795ff70 commit 69328bd

14 files changed

Lines changed: 402 additions & 67 deletions

File tree

‎content/docs/permissions/authentication.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ often before the account-level lock engages.
590590

591591
| Setting | Effect |
592592
|---|---|
593-
| `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. |
593+
| `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. |
594594

595595
> **How enforcement works.** Password expiry (an administrator-forced password
596596
> change included) and enforced MFA share a single *session-validation gate*.

‎packages/cli/src/commands/serve.ts‎

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

‎packages/plugins/plugin-auth/package.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
"types": "./dist/rate-limit-storage.d.ts",
1717
"import": "./dist/rate-limit-storage.mjs",
1818
"require": "./dist/rate-limit-storage.js"
19+
},
20+
"./client-address": {
21+
"types": "./dist/client-address.d.ts",
22+
"import": "./dist/client-address.mjs",
23+
"require": "./dist/client-address.js"
1924
}
2025
},
2126
"scripts": {

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

Lines changed: 115 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ import {
127127
} from './remove-member-permission-guard.js';
128128
import { isPlaceholderEmail } from './placeholder-email.js';
129129
import { readAccountInvitation, settleImportResetDelivery } from './account-invitation-marker.js';
130+
import { resolveClientAddress } from './client-address.js';
131+
import { clientAddressStateEntries, readRequestClientAddress } from './request-client-address.js';
130132
import {
131133
REVOKE_SESSION_NOT_FOUND_CODE,
132134
REVOKE_SESSION_NOT_FOUND_MESSAGE,
@@ -1028,13 +1030,35 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
10281030

10291031
/**
10301032
* ADR-0069 D5 — network gating. When non-empty, auth requests (sign-in,
1031-
* session) from a client IP outside these CIDR / exact ranges are rejected
1032-
* with `IP_NOT_ALLOWED` at the auth-route middleware. Requires a trusted proxy
1033-
* to set `x-forwarded-for` / `cf-connecting-ip`; fails OPEN when the client IP
1034-
* can't be determined (so a missing proxy header is a no-op, not a lockout).
1033+
* session) are judged at the auth-route middleware against these CIDR / exact
1034+
* ranges, and refused with `403 IP_NOT_ALLOWED` unless the client address is
1035+
* inside one.
1036+
*
1037+
* The address judged is the one the server ESTABLISHED, by the platform's
1038+
* client-address trust rule ({@link resolveClientAddress}, shared with the
1039+
* runtime's inbound rate limiter): forwarded headers only when
1040+
* {@link trustProxy} is declared, the transport's peer address otherwise.
1041+
* Behind a reverse proxy, declare `server.trustProxy`, or the allow-list
1042+
* judges the proxy's own address.
1043+
*
1044+
* A request whose address cannot be established (no peer address, no trusted
1045+
* forwarded header) is REFUSED with the same `403 IP_NOT_ALLOWED`, and the
1046+
* first such refusal logs a warning naming this setting and `server.trustProxy`.
1047+
* Unset or empty: no restriction, and nothing is judged.
10351048
*/
10361049
allowedIpRanges?: string[];
10371050

1051+
/**
1052+
* The deployment's `server.trustProxy`: believe `X-Forwarded-For` /
1053+
* `X-Real-IP` when establishing a request's client address for the ADR-0069
1054+
* D5 allow-list ({@link allowedIpRanges}) and the D7 login audit
1055+
* (`last_login_ip`). The same authored value the runtime's inbound rate
1056+
* limiter reads; `objectstack serve` passes it to both. A host that composes
1057+
* `AuthPlugin` itself passes the value it gives the dispatcher.
1058+
* @default false
1059+
*/
1060+
trustProxy?: boolean;
1061+
10381062
/**
10391063
* ADR-0069 D2 — better-auth-native per-IP rate limiting, passed through to
10401064
* better-auth's core `rateLimit`. The settings bind tightens `customRules`
@@ -1117,22 +1141,41 @@ function ipv4ToInt(ip: string): number | null {
11171141
return (((p[0] << 24) >>> 0) + (p[1] << 16) + (p[2] << 8) + p[3]) >>> 0;
11181142
}
11191143

1120-
/** ADR-0069 D5 — does `ip` match `range` (IPv4 CIDR `a.b.c.d/n`, or exact IP)? */
1144+
/**
1145+
* `::ffff:203.0.113.7` → `203.0.113.7`; anything else is returned trimmed.
1146+
*
1147+
* A dual-stack listener reports an IPv4 peer in this IPv4-mapped IPv6 form, and
1148+
* the ADR-0069 D5 allow-list judges the transport's peer address when no proxy
1149+
* is trusted. So both sides of a comparison are read as the IPv4 address they
1150+
* denote, or an IPv4 range could never admit an IPv4 client on such a socket.
1151+
*/
1152+
function unmapIpv4(address: string): string {
1153+
const trimmed = address.trim();
1154+
const mapped = /^::ffff:(\d{1,3}(?:\.\d{1,3}){3})$/i.exec(trimmed);
1155+
return mapped ? mapped[1] : trimmed;
1156+
}
1157+
1158+
/**
1159+
* ADR-0069 D5 — does `ip` match `range` (IPv4 CIDR `a.b.c.d/n`, or exact IP)?
1160+
* An IPv4-mapped IPv6 address on either side compares as its IPv4 address.
1161+
*/
11211162
export function ipMatchesRange(ip: string, range: string): boolean {
1122-
const r = (range || '').trim();
1163+
const r = unmapIpv4(range || '');
11231164
if (!r) return false;
1165+
const address = unmapIpv4(ip);
11241166
if (r.includes('/')) {
1125-
const [base, bitsStr] = r.split('/');
1167+
const [rawBase, bitsStr] = r.split('/');
1168+
const base = unmapIpv4(rawBase);
11261169
const bits = Number(bitsStr);
1127-
const ipInt = ipv4ToInt(ip);
1170+
const ipInt = ipv4ToInt(address);
11281171
const baseInt = ipv4ToInt(base);
11291172
if (ipInt === null || baseInt === null || !(bits >= 0 && bits <= 32)) {
1130-
return ip.trim() === base.trim(); // non-IPv4-CIDR → exact-match fallback
1173+
return address === base; // non-IPv4-CIDR → exact-match fallback
11311174
}
11321175
const mask = bits === 0 ? 0 : (~0 << (32 - bits)) >>> 0;
11331176
return ((ipInt & mask) >>> 0) === ((baseInt & mask) >>> 0);
11341177
}
1135-
return ip.trim() === r;
1178+
return address === r;
11361179
}
11371180

11381181
/**
@@ -2839,18 +2882,12 @@ export class AuthManager {
28392882
if (typeof uid === 'string') await this.enforceConcurrentCap(uid);
28402883
// ADR-0069 D7 — login audit: stamp last_login_at/ip on success.
28412884
// Independent of lockout config (recordSignInOutcome no-ops when
2842-
// lockout is off). IP from the trusted forwarded headers, same
2843-
// precedence as the D5 allow-list middleware.
2885+
// lockout is off). The IP is the address `handleRequest`
2886+
// established for this request by the client-address trust rule,
2887+
// the same address the D5 allow-list judged; none when it was
2888+
// undetermined. ⛔ Never re-read from a header here.
28442889
if (typeof uid === 'string') {
2845-
const hdr = (k: string): string =>
2846-
((ctx?.headers?.get?.(k) ?? ctx?.request?.headers?.get?.(k)) as string) || '';
2847-
const fwd = hdr('x-forwarded-for');
2848-
const ip =
2849-
(fwd && fwd.split(',')[0].trim()) ||
2850-
hdr('cf-connecting-ip') ||
2851-
hdr('x-real-ip') ||
2852-
undefined;
2853-
await this.stampLastLogin(uid, ip);
2890+
await this.stampLastLogin(uid, await readRequestClientAddress());
28542891
}
28552892
}
28562893
}
@@ -6437,9 +6474,27 @@ export class AuthManager {
64376474
* response status and log server errors so they are not silently swallowed.
64386475
*
64396476
* @param request - Web standard Request object
6477+
* @param transport - What the transport knows about the request that a Fetch
6478+
* `Request` does not carry: `remoteAddress` is its peer address
6479+
* (`IHttpRequest.remoteAddress`). Omitted, the request's client address is
6480+
* established from trusted forwarded headers alone, and is undetermined
6481+
* when `trustProxy` is not declared.
64406482
* @returns Web standard Response object
64416483
*/
6442-
async handleRequest(request: Request): Promise<Response> {
6484+
async handleRequest(request: Request, transport?: { remoteAddress?: string }): Promise<Response> {
6485+
// ADR-0069 D7 — the client address this request's sign-in audit stamps,
6486+
// established ONCE, here, by the same trust rule the D5 allow-list judges
6487+
// with (`resolveClientAddress`). Read before any rewrite below, and carried
6488+
// to the after-hook in better-auth's request state (`request-client-address.ts`).
6489+
const clientHeaders: Record<string, string> = {};
6490+
request.headers.forEach((value, name) => {
6491+
clientHeaders[name] = value;
6492+
});
6493+
const clientAddress = this.resolveClientAddress({
6494+
headers: clientHeaders,
6495+
...(transport?.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}),
6496+
});
6497+
64436498
// Dev DX: better-auth's CSRF protection rejects state-changing requests
64446499
// (e.g. `/sign-in/email`) with a 403 when neither `Origin` nor `Referer`
64456500
// is present. Browsers always send one; non-browser API clients (curl,
@@ -6515,7 +6570,10 @@ export class AuthManager {
65156570
// The previous single call site flattened it with the `await` below.
65166571
const runHandler = async (): Promise<Response> =>
65176572
await runWithAuthActorScope(() =>
6518-
runWithRequestState(new WeakMap(), () => auth.handler(request)),
6573+
runWithRequestState(
6574+
new WeakMap<object, unknown>(clientAddressStateEntries(clientAddress)),
6575+
() => auth.handler(request),
6576+
),
65196577
);
65206578

65216579
// [#7724] A subject-erasure request is ONE unit of work, and better-auth
@@ -9598,17 +9656,47 @@ export class AuthManager {
95989656

95999657
/**
96009658
* ADR-0069 D5 — is `ip` within the configured allow-list? True (allow) when no
9601-
* ranges are configured, OR when the IP can't be determined (fail-open so a
9602-
* misconfigured proxy never locks everyone out — an admin enabling this must
9603-
* ensure forwarded headers are trusted). Supports IPv4 CIDR + exact IPv4/IPv6.
9659+
* ranges are configured: nothing is judged then. With ranges configured, true
9660+
* only for an address inside one of them, and FALSE for an undetermined
9661+
* address (`undefined` / empty): an allow-list is a gate, and a gate that
9662+
* opens for "unknown" is open to anyone who can make themselves unknown.
9663+
* Supports IPv4 CIDR + exact IPv4/IPv6 (IPv4-mapped IPv6 compares as IPv4).
9664+
*
9665+
* `ip` must be the address the server established
9666+
* ({@link resolveClientAddress}), never a header read by the caller.
96049667
*/
96059668
public isClientIpAllowed(ip: string | undefined): boolean {
96069669
const ranges = this.config.allowedIpRanges;
96079670
if (!ranges || ranges.length === 0) return true;
9608-
if (!ip) return true; // undetermined → fail-open
9671+
if (!ip) return false; // undetermined → refused
96099672
return ranges.some((r) => ipMatchesRange(ip, r));
96109673
}
96119674

9675+
/**
9676+
* The client address this deployment establishes for a request: the
9677+
* platform's client-address trust rule (`./client-address.ts`, shared with the
9678+
* runtime's inbound rate limiter) applied with this deployment's
9679+
* {@link AuthManagerOptions.trustProxy}. Forwarded headers count only when
9680+
* `trustProxy` is declared; otherwise the transport's `remoteAddress`;
9681+
* `undefined` when neither is available. The ADR-0069 D5 allow-list judges
9682+
* this address and the D7 login audit stamps it.
9683+
*/
9684+
public resolveClientAddress(transport: {
9685+
headers?: Record<string, string | string[] | undefined>;
9686+
remoteAddress?: string;
9687+
}): string | undefined {
9688+
return resolveClientAddress({
9689+
...(transport.headers ? { headers: transport.headers } : {}),
9690+
...(transport.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}),
9691+
trustProxy: this.config.trustProxy === true,
9692+
});
9693+
}
9694+
9695+
/** Whether this deployment declared `server.trustProxy` ({@link AuthManagerOptions.trustProxy}). */
9696+
public trustsForwardedHeaders(): boolean {
9697+
return this.config.trustProxy === true;
9698+
}
9699+
96129700
/**
96139701
* Returns the data engine wired into this auth manager. Used by route
96149702
* handlers (e.g. bootstrap-status) that need to query identity tables

0 commit comments

Comments
 (0)