Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changeset/24799-vendor-client-address-trust.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
'@objectstack/plugin-auth': patch
---

fix(plugin-auth): the vendored auth library records and keys on the client address the server established (#24799)

Clause-②: no

better-auth reads a request's client address with its own reader, and three things are keyed on what it reads: a session's `ip_address` (and the `login` / `logout` audit rows copied from it), the address on the `login_failed` / `account_locked` audit rows, and the key of better-auth's own per-address rate limiter. That reader did not follow `server.trustProxy`. It now follows the same rule as the `allowed_ip_ranges` allow-list and `last_login_ip`: `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. When neither is available, better-auth is handed no address.

**For operators:**

- Behind a reverse proxy or load balancer, declare `server: { trustProxy: true }` in the stack. Without it, sessions and audit rows record the proxy's own address, and better-auth's per-address rate limit counts every client behind the proxy as one client.
- With `server.trustProxy` declared, a forwarded chain (`client, proxy`) now resolves to its first entry. Before, better-auth read a multi-entry header as no address.
- A request whose client address cannot be established records no address, and better-auth's rate limiter counts it in one shared bucket.

**For hosts:** `AuthManager.handleRequest` now drops any `x-objectstack-client-address` request header a caller sends. The platform writes that header itself, from the address it established, and better-auth reads no other header for the client address. A programmatic `auth.api.*` call is handed whatever headers its caller passes. Route a client's request through `handleRequest` instead.
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,20 @@ function deployment(config: Record<string, unknown> = {}) {
return { engine, manager, events };
}

// The client is the connection's peer address, handed over the way the auth
// route hands the transport's peer: the address the client-address trust rule
// (`client-address.ts`) establishes when no proxy is declared trusted.
const post = (manager: AuthManager, path: string, body: unknown) =>
manager.handleRequest(
new Request(`${BASE}${path}`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'x-forwarded-for': CLIENT_IP,
'user-agent': CLIENT_UA,
},
body: JSON.stringify(body),
}),
{ remoteAddress: CLIENT_IP },
);

async function signUp(manager: AuthManager): Promise<void> {
Expand Down
6 changes: 4 additions & 2 deletions packages/plugins/plugin-auth/src/auth-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { AuthManager, ipMatchesRange } from './auth-manager';
import { vendorClientAddressOptions } from './client-address-vendor';
import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/objectql';

// Mock better-auth so we can control the handler behaviour
Expand Down Expand Up @@ -2116,6 +2117,7 @@ describe('AuthManager', () => {
domain: '.objectos.app',
},
useSecureCookies: true,
ipAddress: vendorClientAddressOptions(),
});
});

Expand All @@ -2142,7 +2144,7 @@ describe('AuthManager', () => {
expect(capturedConfig.advanced.cookiePrefix).toBe('objectos');
});

it('should NOT include advanced when not provided', async () => {
it('carries only the library client-address source when no advanced option is provided', async () => {
let capturedConfig: any;
(betterAuth as any).mockImplementation((config: any) => {
capturedConfig = config;
Expand All @@ -2157,7 +2159,7 @@ describe('AuthManager', () => {
await manager.getAuthInstance();
warnSpy.mockRestore();

expect(capturedConfig).not.toHaveProperty('advanced');
expect(capturedConfig.advanced).toEqual({ ipAddress: vendorClientAddressOptions() });
});
});

Expand Down
52 changes: 35 additions & 17 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ import {
import { isPlaceholderEmail } from './placeholder-email.js';
import { readAccountInvitation, settleImportResetDelivery } from './account-invitation-marker.js';
import { resolveClientAddress } from './client-address.js';
import { vendorClientAddressOptions, withVendorClientAddress } from './client-address-vendor.js';
import { clientAddressStateEntries, readRequestClientAddress } from './request-client-address.js';
import {
REVOKE_SESSION_NOT_FOUND_CODE,
Expand Down Expand Up @@ -1059,9 +1060,13 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
* 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.
* (`last_login_ip`) — and for what the vendored auth library records and
* keys on by client address (session `ip_address`, the sign-in audit
* fingerprint, its own per-address {@link rateLimit}), which is handed that
* same address (`client-address-vendor.ts`). 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;
Expand Down Expand Up @@ -3082,19 +3087,23 @@ export class AuthManager {
return origins.length ? { trustedOrigins: origins } : {};
})(),

// Advanced options (cross-subdomain cookies, secure cookies, CSRF, etc.)
...(this.config.advanced ? {
advanced: {
...(this.config.advanced.crossSubDomainCookies
? { crossSubDomainCookies: this.config.advanced.crossSubDomainCookies } : {}),
...(this.config.advanced.useSecureCookies != null
? { useSecureCookies: this.config.advanced.useSecureCookies } : {}),
...(this.config.advanced.disableCSRFCheck != null
? { disableCSRFCheck: this.config.advanced.disableCSRFCheck } : {}),
...(this.config.advanced.cookiePrefix != null
? { cookiePrefix: this.config.advanced.cookiePrefix } : {}),
},
} : {}),
// Advanced options (cross-subdomain cookies, secure cookies, CSRF, etc.),
// and — always — the library's own client-address source: the one
// platform-owned header `handleRequest` writes from the address the
// client-address trust rule established (`client-address-vendor.ts`).
// Never left at the library's default reader, which knows neither the
// transport's peer address nor `server.trustProxy`.
advanced: {
...(this.config.advanced?.crossSubDomainCookies
? { crossSubDomainCookies: this.config.advanced.crossSubDomainCookies } : {}),
...(this.config.advanced?.useSecureCookies != null
? { useSecureCookies: this.config.advanced.useSecureCookies } : {}),
...(this.config.advanced?.disableCSRFCheck != null
? { disableCSRFCheck: this.config.advanced.disableCSRFCheck } : {}),
...(this.config.advanced?.cookiePrefix != null
? { cookiePrefix: this.config.advanced.cookiePrefix } : {}),
ipAddress: vendorClientAddressOptions(),
},
};

const auth = betterAuth(betterAuthConfig);
Expand Down Expand Up @@ -6497,7 +6506,8 @@ export class AuthManager {
* `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.
* when `trustProxy` is not declared. Either way the vendored library is
* handed that address and no other (`client-address-vendor.ts`).
* @returns Web standard Response object
*/
async handleRequest(request: Request, transport?: { remoteAddress?: string }): Promise<Response> {
Expand All @@ -6514,6 +6524,14 @@ export class AuthManager {
...(transport?.remoteAddress ? { remoteAddress: transport.remoteAddress } : {}),
});

// The vendored library's OWN client-address reader (session `ip_address`,
// the sign-in audit fingerprint, its per-address rate limiter) reads only
// the platform-owned header (`vendorClientAddressOptions`, composed in
// `createAuthInstance`). Written HERE, from the address just established,
// after dropping any copy the caller sent — so the library answers the same
// address the D5 allow-list judged, or none. See `client-address-vendor.ts`.
request = withVendorClientAddress(request, clientAddress);

// 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,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
//
// The vendored auth library's OWN client-address source follows the
// platform's client-address trust rule (`./client-address.ts`), the rule the
// ADR-0069 D5 allow-list judges with and the D7 login audit stamps by:
//
// - forwarded headers count only when the deployment declares
// `server.trustProxy` (`AuthManagerOptions.trustProxy`);
// - otherwise the address is the transport's peer (`IHttpRequest.remoteAddress`);
// - neither available ⇒ the library is handed NO address.
//
// One pin per branch. Each reads, off one composition, the three things the
// library keys on a client address (`client-address-vendor.ts`): the session
// row's `ip_address`, the refused-sign-in audit fingerprint, and the key its
// per-address rate limiter consumes. The fixture is the D5/D7 suite's
// (`client-ip-allow-list-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`.
//
// Every request also carries the platform-owned header itself, set by the
// caller: in no branch may a caller's copy reach the library.
//
// ADR-0112 is `code` AND `status`; the refused sign-in asserts both, so a
// refusal from anywhere but the credential check cannot stand in for it.

import { describe, it, expect, vi, 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 { 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-24799';
const ORIGIN = 'http://localhost:3000';
const BASE = '/api/v1/auth';
const EMAIL = 'member.24799@example.com';

/** Documentation ranges (RFC 5737), and one private proxy hop. */
const CLIENT = '203.0.113.7';
const OTHER = '198.51.100.9';
const PROXY_HOP = '10.0.0.2';

/** The credential check's own refusal (ADR-0112 `code` + `status`). */
const CREDENTIAL_REFUSAL = { status: 401, code: 'INVALID_EMAIL_OR_PASSWORD' };

const mockCtx = () =>
({
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 unknown as PluginContext;

async function verdict(res: Response): Promise<{ status: number; code?: string }> {
const text = await res.text();
let code: string | undefined;
try {
const parsed = JSON.parse(text);
code = parsed?.error?.code ?? parsed?.code;
} catch {
/* non-JSON body → no code */
}
return { status: res.status, code };
}

/** What the library keyed on one client address, per seam. */
interface Observed {
/** `sys_session.ip_address` of the session the successful sign-in minted. */
session: unknown;
/** The `login_failed` audit event's address, for the refused sign-in. */
refused: unknown;
/** The address half of every rate-limit key consumed across both sign-ins. */
limiterKeys: string[];
/** ADR-0069 D7 `last_login_ip` after the successful sign-in. */
lastLoginIp: unknown;
}

/** One deployment, served through the REAL mount chain. */
async function serve(deployment: { trustProxy?: boolean }) {
const engine = createMemoryEngine();
const events: Array<{ action: string; ipAddress?: string }> = [];
const consumedKeys: string[] = [];
const manager = new AuthManager({
secret: SECRET,
baseUrl: ORIGIN,
dataEngine: engine,
getAuditSink: () => ({
recordAuthEvent: async (event: { action: string; ipAddress?: string }) => {
events.push(event);
},
}),
// The library's limiter is on by default only in production; switch it on
// and record every key it consumes, admitting every request.
rateLimit: { enabled: true },
rateLimitStorage: {
consume: async (key: string) => {
consumedKeys.push(key);
return { allowed: true, retryAfter: null };
},
},
...deployment,
} as any);

const ctx = mockCtx();
const plugin = new AuthPlugin({ secret: SECRET });
await plugin.init(ctx);
(plugin as any).authManager = manager;
const app = new Hono();
(plugin as any).registerAuthRoutes({ getRawApp: () => app, getPort: () => 0 }, ctx);

const signIn = (peer: string | undefined, headers: Record<string, string>, password: string) =>
app.request(
`${ORIGIN}${BASE}/sign-in/email`,
{
method: 'POST',
headers: { 'content-type': 'application/json', origin: ORIGIN, ...headers },
body: JSON.stringify({ email: EMAIL, password }),
},
peer ? { incoming: { socket: { remoteAddress: peer } } } : {},
);

await inviteForAudienceGate(manager, EMAIL);
const signUp = await manager.handleRequest(
new Request(`${ORIGIN}${BASE}/sign-up/email`, {
method: 'POST',
headers: { 'content-type': 'application/json', origin: ORIGIN },
body: JSON.stringify({ email: EMAIL, password: PASSWORD, name: 'Client Address Member' }),
}),
);
expect(signUp.status, `sign-up: ${await signUp.clone().text()}`).toBe(200);

/** A refused, then a successful, sign-in from `peer` carrying `headers`. */
const observe = async (peer: string | undefined, headers: Record<string, string>): Promise<Observed> => {
const sessionsBefore = (engine.tables.get('sys_session') ?? []).length;
const eventsBefore = events.length;
const keysBefore = consumedKeys.length;

const refusedRes = await signIn(peer, headers, `${PASSWORD}-wrong`);
expect(await verdict(refusedRes)).toEqual(CREDENTIAL_REFUSAL);
const okRes = await signIn(peer, headers, PASSWORD);
expect(okRes.status, await okRes.clone().text()).toBe(200);

const minted = ((engine.tables.get('sys_session') ?? []) as any[]).slice(sessionsBefore);
expect(minted, 'one session minted by the successful sign-in').toHaveLength(1);
const refused = events.slice(eventsBefore).filter((e) => e.action === 'login_failed');
expect(refused, 'one login_failed event for the refused sign-in').toHaveLength(1);
const limiterKeys = consumedKeys.slice(keysBefore).map((key) => key.split('|')[0]!);
expect(limiterKeys, 'the library limiter consumed a key for each sign-in').toHaveLength(2);
const user = ((engine.tables.get('sys_user') ?? []) as any[]).find((r) => r.email === EMAIL);

return {
session: minted[0]?.ip_address,
refused: refused[0]?.ipAddress,
limiterKeys,
lastLoginIp: user?.last_login_ip,
};
};

return { observe };
}

beforeEach(() => {
vi.spyOn(console, 'warn').mockImplementation(() => {});
vi.spyOn(console, 'error').mockImplementation(() => {});
});
afterEach(() => {
vi.restoreAllMocks();
});

describe('the vendored auth library keys on the client address the server established, and no other', () => {
it('behind a declared trusted proxy, it records and keys on the forwarded client address', async () => {
const d = await serve({ trustProxy: true });

const seen = await d.observe(PROXY_HOP, {
'x-forwarded-for': `${CLIENT}, ${PROXY_HOP}`,
[VENDOR_CLIENT_ADDRESS_HEADER]: OTHER,
});

expect(seen.session).toBe(CLIENT);
expect(seen.refused).toBe(CLIENT);
expect(seen.limiterKeys).toEqual([CLIENT, CLIENT]);
// The same address the D7 login audit stamped.
expect(seen.lastLoginIp).toBe(CLIENT);
});

it('without a declared trusted proxy, it records and keys on the connection peer, whatever the request headers say', async () => {
const d = await serve({});

const seen = await d.observe(CLIENT, {
'x-forwarded-for': OTHER,
'x-real-ip': OTHER,
[VENDOR_CLIENT_ADDRESS_HEADER]: OTHER,
});

expect(seen.session).toBe(CLIENT);
expect(seen.refused).toBe(CLIENT);
expect(seen.limiterKeys).toEqual([CLIENT, CLIENT]);
expect(seen.lastLoginIp).toBe(CLIENT);
});

it('when the server established no address, it is handed none, whatever the request headers say', async () => {
// What the library answers when it is handed no address is its own,
// environment-dependent behaviour (nothing in production, a loopback
// placeholder in development and test) — so each deployment's answer to a
// request carrying no header at all is the control, never a literal.
for (const deployment of [{}, { trustProxy: true }]) {
const d = await serve(deployment);
const control = await d.observe(undefined, {});
const seen = await d.observe(undefined, {
...(deployment.trustProxy ? {} : { 'x-forwarded-for': OTHER, 'x-real-ip': OTHER }),
[VENDOR_CLIENT_ADDRESS_HEADER]: OTHER,
});

const label = deployment.trustProxy ? 'trusted proxy, nothing forwarded' : 'no trusted proxy';
expect(seen.session, label).toEqual(control.session);
expect(seen.refused, label).toEqual(control.refused);
expect(seen.limiterKeys, label).toEqual(control.limiterKeys);
expect([seen.session, seen.refused, ...seen.limiterKeys], label).not.toContain(OTHER);
// …and the D7 login audit stamped none either.
expect(seen.lastLoginIp, label).toBeUndefined();
}
});
});
Loading
Loading