diff --git a/.changeset/adr-0069-shared-ratelimit-store.md b/.changeset/adr-0069-shared-ratelimit-store.md new file mode 100644 index 0000000000..aeb3cf32a4 --- /dev/null +++ b/.changeset/adr-0069-shared-ratelimit-store.md @@ -0,0 +1,23 @@ +--- +"@objectstack/plugin-auth": minor +--- + +feat(auth): shared cross-node rate-limit + session store via the cache service (ADR-0069 D2) + +Multi-node deployments previously rate-limited **per process** — better-auth's +default `rateLimit` store is in-memory, so each node counted independently and +an attacker could rotate nodes to bypass the limit. `AuthPlugin` now wires the +kernel `cache` service as better-auth's `secondaryStorage` and flips +`rateLimit.storage` to `'secondary-storage'`, so rate-limit counters (and the +session cache) are enforced against **one shared store across every node** — +shared iff the cache service is (Redis adapter in a cluster; memory single-node, +where behavior is unchanged). When no cache service is registered the plugin +logs a warning that a multi-node deployment needs a shared cache (ADR-0069 +honesty — no silent per-process limiting presented as global). + +New `cacheSecondaryStorage(cache)` adapter (`ICacheService` → better-auth +`SecondaryStorage`). Note: the cache has no atomic increment, so under high +concurrency the get→set counter path can slightly over-count — acceptable for a +rate limiter and strictly better than independent per-node counters; a future +cache adapter exposing atomic INCR can add an `increment` method for exact +counting. diff --git a/docs/adr/0069-enterprise-authentication-hardening.md b/docs/adr/0069-enterprise-authentication-hardening.md index c783901f3f..9c87b9e511 100644 --- a/docs/adr/0069-enterprise-authentication-hardening.md +++ b/docs/adr/0069-enterprise-authentication-hardening.md @@ -139,7 +139,7 @@ Each row in D1-D6 names exactly one of these seams. No setting is introduced wit | Phase | Status | Notes | |---|---|---| | **P1** (D1/D2/D3 + D7 fields) | ✅ **implemented** | Password complexity/history/expiry (`assertPasswordComplexity`/`assertPasswordNotReused`/`stampPasswordChangedAt`), HIBP (`haveIBeenPwned` plugin), account lockout (`assertAccountNotLocked`/`recordSignInOutcome` + `unlock_user` action), enforced MFA + grace (`computeAuthGate` → `MFA_REQUIRED`, per-org `require_mfa`), rate-limit tuning (`customRules`). All settings in `auth.manifest.ts`, bound via `bindAuthSettings`. Login-audit fields `last_login_at`/`last_login_ip` stamped on sign-in (`stampLastLogin`). | -| **P2** (D4/D5) | 🟡 **mostly implemented** | Session idle/absolute/concurrent (`enforceSessionControls`/`enforceConcurrentCap`) and the **global** IP allow-list (`isClientIpAllowed`, `auth.allowed_ip_ranges`) are landed. **Remaining:** per-org `sys_organization.allowed_ip_ranges` (+ optional `sys_user.allowed_ip_ranges` override); a **shared/Redis rate-limit store** for multi-node (current store is in-memory). | +| **P2** (D4/D5) | 🟡 **mostly implemented** | Session idle/absolute/concurrent (`enforceSessionControls`/`enforceConcurrentCap`), the **global** IP allow-list (`isClientIpAllowed`, `auth.allowed_ip_ranges`), and the **shared multi-node rate-limit + session store** (better-auth `secondaryStorage` bound to the kernel cache service via `cacheSecondaryStorage`; shared iff the cache is — Redis adapter in a cluster) are landed. **Remaining:** per-org `sys_organization.allowed_ip_ranges` (+ optional `sys_user.allowed_ip_ranges` override) — tracked in #2571. | | **P2/P3** (D6) | 🟡 partial | Generic OIDC RP wired (`genericOAuth`/`sso`); admin OIDC **trust-list settings UI** still env/`sys_sso_provider`-only. | | **P3** (SAML, broader social) | 🟡 partial | `@better-auth/sso` present (SAML now better-auth-native — see Addendum); broader settings-driven social providers pending. | diff --git a/packages/plugins/plugin-auth/src/auth-manager.test.ts b/packages/plugins/plugin-auth/src/auth-manager.test.ts index 54c2735ebd..7421f5eace 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.test.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.test.ts @@ -1806,6 +1806,36 @@ describe('AuthManager', () => { warn.mockRestore(); expect(captured).not.toHaveProperty('rateLimit'); }); + + // ── ADR-0069 D2 — shared secondaryStorage → cross-node rate limiting ── + it('wires secondaryStorage and flips rateLimit.storage to "secondary-storage"', async () => { + let captured: any; + (betterAuth as any).mockImplementation((cfg: any) => { captured = cfg; return { handler: vi.fn(), api: {} }; }); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const ss = { get: vi.fn(), set: vi.fn(), delete: vi.fn() }; + const m = new AuthManager({ + secret: SECRET, baseUrl: 'http://localhost:3000', + rateLimit: { enabled: true, window: 60, max: 10 } as any, + secondaryStorage: ss as any, + }); + await m.getAuthInstance(); + warn.mockRestore(); + expect(captured.secondaryStorage).toBe(ss); + expect(captured.rateLimit.storage).toBe('secondary-storage'); + expect(captured.rateLimit).toMatchObject({ enabled: true, max: 10, window: 60 }); + }); + + it('flips rateLimit.storage even when no explicit rateLimit config is given (secondaryStorage alone)', async () => { + let captured: any; + (betterAuth as any).mockImplementation((cfg: any) => { captured = cfg; return { handler: vi.fn(), api: {} }; }); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const ss = { get: vi.fn(), set: vi.fn(), delete: vi.fn() }; + const m = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000', secondaryStorage: ss as any }); + await m.getAuthInstance(); + warn.mockRestore(); + expect(captured.secondaryStorage).toBe(ss); + expect(captured.rateLimit.storage).toBe('secondary-storage'); + }); }); // ADR-0069 D1: password complexity validator (custom; better-auth only does diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 254161c006..f1b08865ba 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -348,6 +348,17 @@ export interface AuthManagerOptions extends Partial { * `/reset-password`). Multi-node deployments need a shared `storage`. */ rateLimit?: BetterAuthOptions['rateLimit']; + + /** + * ADR-0069 D2 — shared KV store for cross-node state. When set, better-auth + * uses it for **rate-limit counters** (the manager also flips + * `rateLimit.storage` to `'secondary-storage'`) and session caching, so both + * are enforced against ONE store across every node — closing the multi-node + * rate-limit-bypass hole (each node otherwise counts independently). Wired by + * `AuthPlugin` from the kernel `cache` service (memory single-node, Redis in + * a cluster). Absent → better-auth keeps its per-process in-memory store. + */ + secondaryStorage?: BetterAuthOptions['secondaryStorage']; } /** @@ -643,8 +654,20 @@ export class AuthManager { // ADR-0069 D2 — per-IP rate limiting (native). Only set when configured // so better-auth keeps its own defaults otherwise. The settings bind - // supplies stricter `customRules` for the auth endpoints. - ...(this.config.rateLimit ? { rateLimit: this.config.rateLimit } : {}), + // supplies stricter `customRules` for the auth endpoints. When a shared + // secondaryStorage is wired, flip the rate-limit store to it so counters + // are enforced across nodes (default 'memory' is per-process). + ...(this.config.rateLimit || this.config.secondaryStorage + ? { + rateLimit: { + ...(this.config.rateLimit ?? {}), + ...(this.config.secondaryStorage ? { storage: 'secondary-storage' as const } : {}), + }, + } + : {}), + + // ADR-0069 D2 — shared KV for cross-node rate-limit + session state. + ...(this.config.secondaryStorage ? { secondaryStorage: this.config.secondaryStorage } : {}), // better-auth plugins — registered based on AuthPluginConfig flags plugins, diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 8d2b09afc4..710e1fcc51 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -157,6 +157,36 @@ export class AuthPlugin implements Plugin { ...this.options, dataEngine, }; + + // ADR-0069 D2 — wire the kernel `cache` service as better-auth's shared + // secondaryStorage (rate-limit counters + session cache). Shared across + // nodes iff the cache service is (Redis adapter in a cluster; memory + // single-node). An explicit `secondaryStorage` on the options wins. Skipped + // when no cache service is registered — with a warning, because a multi-node + // deployment then silently rate-limits per-process (ADR-0069 D2 honesty). + if (!authConfig.secondaryStorage) { + // The `cache` service is registered ASYNC — `getService` throws for it, + // so resolve via `getServiceAsync` and treat any failure (not registered, + // or not yet ready) as "no shared cache". + let cache: any; + try { + cache = await (ctx as { getServiceAsync?: (n: string) => Promise }).getServiceAsync?.('cache'); + } catch { + cache = undefined; + } + if (cache && typeof cache.get === 'function' && typeof cache.set === 'function') { + const { cacheSecondaryStorage } = await import('./secondary-storage.js'); + authConfig.secondaryStorage = cacheSecondaryStorage(cache); + ctx.logger.info( + '[auth] rate-limit + session store bound to the kernel cache service — shared across nodes iff the cache is (ADR-0069 D2)', + ); + } else { + ctx.logger.warn( + '[auth] no cache service registered — rate-limit counters use a per-process in-memory store; a multi-node deployment needs a shared cache (Redis) to enforce limits globally (ADR-0069 D2)', + ); + } + } + this.applyEnvSocialProviderFallbacks(authConfig); // Open extension point for packages that contribute auth providers diff --git a/packages/plugins/plugin-auth/src/secondary-storage.test.ts b/packages/plugins/plugin-auth/src/secondary-storage.test.ts new file mode 100644 index 0000000000..35c310b376 --- /dev/null +++ b/packages/plugins/plugin-auth/src/secondary-storage.test.ts @@ -0,0 +1,50 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, vi } from 'vitest'; +import { cacheSecondaryStorage } from './secondary-storage.js'; + +/** Minimal in-memory ICacheService stand-in. */ +function makeCache() { + const store = new Map(); + return { + store, + get: vi.fn(async (k: string) => (store.has(k) ? store.get(k) : undefined)), + set: vi.fn(async (k: string, v: unknown, _ttl?: number) => { store.set(k, v); }), + delete: vi.fn(async (k: string) => store.delete(k)), + has: vi.fn(async (k: string) => store.has(k)), + clear: vi.fn(async () => store.clear()), + stats: vi.fn(async () => ({ hits: 0, misses: 0, keys: store.size })), + }; +} + +describe('cacheSecondaryStorage (ADR-0069 D2 — shared rate-limit/session store)', () => { + it('round-trips a value through the cache service', async () => { + const cache = makeCache(); + const ss = cacheSecondaryStorage(cache as any); + await ss.set('k1', '{"count":1}', 60); + expect(cache.set).toHaveBeenCalledWith('k1', '{"count":1}', 60); + expect(await ss.get('k1')).toBe('{"count":1}'); + }); + + it('maps a cache MISS (undefined) to null (better-auth contract)', async () => { + const cache = makeCache(); + const ss = cacheSecondaryStorage(cache as any); + expect(await ss.get('absent')).toBeNull(); + }); + + it('forwards the TTL (seconds) to the cache set', async () => { + const cache = makeCache(); + const ss = cacheSecondaryStorage(cache as any); + await ss.set('k', 'v', 10); + expect(cache.set).toHaveBeenCalledWith('k', 'v', 10); + }); + + it('deletes via the cache service', async () => { + const cache = makeCache(); + const ss = cacheSecondaryStorage(cache as any); + await ss.set('k', 'v'); + await ss.delete('k'); + expect(cache.delete).toHaveBeenCalledWith('k'); + expect(await ss.get('k')).toBeNull(); + }); +}); diff --git a/packages/plugins/plugin-auth/src/secondary-storage.ts b/packages/plugins/plugin-auth/src/secondary-storage.ts new file mode 100644 index 0000000000..40439a5a2a --- /dev/null +++ b/packages/plugins/plugin-auth/src/secondary-storage.ts @@ -0,0 +1,47 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import type { BetterAuthOptions } from 'better-auth'; +import type { ICacheService } from '@objectstack/spec/contracts'; + +type SecondaryStorage = NonNullable; + +/** + * ADR-0069 D2 — adapt the kernel `cache` service into a better-auth + * `secondaryStorage`. When wired, better-auth uses it for **rate-limit + * counters** (`rateLimit.storage: 'secondary-storage'`) and session caching — + * so both become **shared across nodes iff the cache service is shared**. + * + * In a single-node deployment the cache is memory-backed and this behaves like + * the default per-process store. In a multi-node deployment the operator + * configures the cache service with the Redis adapter (already supported by + * `@objectstack/service-cache`), and rate limiting is then enforced against a + * single shared counter — closing the "each node counts independently, so an + * attacker rotates nodes to bypass the limit" hole (ADR-0069 D2). + * + * better-auth's `secondaryStorage` contract is string-valued: `get` returns the + * stored string (or null), `set` takes a string value + optional TTL (seconds), + * `delete` removes it. We map straight onto `ICacheService`, translating + * `undefined` (miss) → `null`. + * + * NOTE on atomicity: better-auth's secondary-storage rate-limit path uses + * get→compute→set (not an atomic increment) unless the storage exposes + * `increment`. `ICacheService` has no atomic increment, so under high + * concurrency two nodes can read the same counter and both admit a request — a + * small over-count, acceptable for a rate limiter and still strictly better + * than the per-node independent counters it replaces. A future cache adapter + * exposing atomic INCR can add an `increment` method here for exact counting. + */ +export function cacheSecondaryStorage(cache: ICacheService): SecondaryStorage { + return { + get: async (key: string): Promise => { + const v = await cache.get(key); + return v === undefined ? null : v; + }, + set: async (key: string, value: string, ttl?: number): Promise => { + await cache.set(key, value, ttl); + }, + delete: async (key: string): Promise => { + await cache.delete(key); + }, + }; +}