Skip to content

Commit 94c273b

Browse files
committed
fix(plugin-auth): resolve the kernel cache at counting time, not at init (#4772)
`AuthPlugin.init()` probed `getServiceAsync('cache')` and froze the answer for the life of the process. It runs BEFORE `CacheServicePlugin` registers the service (21ms earlier in a showcase cold start), so the probe resolved `undefined` in deployments that have a cache configured — and the warning it printed told the operator to provision Redis for a problem they did not have. The misdiagnosis was the visible half. The real defect: better-auth is built lazily but from the config captured at init, so the "no cache" conclusion was permanent. Rate-limit counters never reached the shared store even after it came up, meaning a multi-node deployment's limits were never enforced globally (ADR-0069 D2 declared a capability the runtime did not deliver). `createLazyCacheRateLimitStorage()` implements better-auth's `rateLimit.customStorage` and resolves the `cache` service when a counter is actually consumed — strictly after `kernel:ready`, therefore independent of plugin start order. The warning is kept but now fires only when a counter genuinely has nowhere shared to count, once per process; without a cache the limit is still enforced, in-process (degraded, never disabled). Deliberately `customStorage`, not `secondaryStorage`: the latter also moves the session of record into the cache (`createSession` skips the `sys_session` row, `findSession` answers from the snapshot without reading the database), which silently disables the ADR-0069 D4 session controls — idle timeout, absolute max and concurrent cap all revoke by writing that row. The cache is therefore no longer auto-bound as `secondaryStorage`; `cacheSecondaryStorage` is exported for a host that opts into that trade knowingly. Where the session of record belongs is #4785. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
1 parent 9f601e8 commit 94c273b

10 files changed

Lines changed: 805 additions & 80 deletions
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix(plugin-auth): 限流计数器改为惰性解析 kernel cache —— 修掉误报的告警,也修掉「共享限流从未生效」的功能洞 (#4772)
6+
7+
`pnpm dev`(showcase)每次冷启都会打一条:
8+
9+
```
10+
WARN [auth] no cache service registered — rate-limit counters use a per-process in-memory
11+
store; a multi-node deployment needs a shared cache (Redis) to enforce limits globally
12+
```
13+
14+
`CacheServicePlugin` 就在 **21ms 后**注册好了,它本来就在已加载插件列表里。这条告警把运维引向「你需要 Redis」,接完 Redis 还是同一条告警 —— 因为缺的不是 Redis。
15+
16+
**这不只是日志误报。** `AuthPlugin.init()` 里那次 `getServiceAsync('cache')`
17+
探测的结论会被**冻结整个进程生命周期**:better-auth 实例是懒创建的,但它读的是 init
18+
时定下的 config。所以标准组合下 auth 这一侧永远拿着「没有 cache」这个结论,限流计数器
19+
**从未**用上共享存储 —— 多节点部署的限额从来没有被全局强制过,每个节点各算各的,轮换
20+
节点即可绕过。ADR-0069 D2 声明的能力与运行时不一致。
21+
22+
**修法:把「取 cache 服务」放回真正用到它的那一刻。** 新增
23+
`createLazyCacheRateLimitStorage()`,实现 better-auth 的 `rateLimit.customStorage`
24+
计数器被消费时才解析 `cache` 服务(这一刻必然在 `kernel:ready` 之后,因此与插件启动
25+
顺序无关),解析到就一直用它。告警保留,但只在**计数器真的要用共享存储、而此刻确实
26+
一个 cache 服务都没有**时才打一次 —— 那时它才是真信号,「加一个共享缓存」也才是对的
27+
建议。真没有 cache 的部署仍然限流,只是退化成进程内计数(降级,不是关闭)。
28+
29+
**刻意走 `rateLimit.customStorage` 而不是 `secondaryStorage`** 后者会连带把**会话
30+
的记录之处**搬进缓存:better-auth 的 `createSession` 不再写 `sys_session` 行,
31+
`findSession` 直接从缓存快照作答、根本不查库;而 ADR-0069 D4 的空闲超时 / 绝对时长
32+
上限 / 并发上限**全部靠写那一行来撤销会话**。所以自动把 cache 绑成 `secondaryStorage`
33+
会静默废掉 D4 的三个管控。本次因此不再从 cache 服务自动派生 `secondaryStorage`
34+
它回归「宿主显式提供才生效」,`cacheSecondaryStorage()` 改为从包根导出,供知情的宿主
35+
自行选用。会话到底该存哪,是一个需要维护者裁定的架构问题,记录在 #4785
36+
37+
对使用者的影响:
38+
39+
- 配了 cache 插件的部署不再出现那条 warn,改为一条 info(计数器已绑定到 cache 服务);
40+
- 多节点 + Redis cache 的部署,限流计数**现在真的**是全局的;
41+
- 新增 `AuthManagerOptions.rateLimitStorage`(counters-only,不迁移会话);宿主自己
42+
提供的 `secondaryStorage` 行为不变,仍然优先并继续走
43+
`rateLimit.storage: 'secondary-storage'`

docs/adr/0069-enterprise-authentication-hardening.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ Each row in D1-D6 names exactly one of these seams. No setting is introduced wit
139139
| Phase | Status | Notes |
140140
|---|---|---|
141141
| **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`). |
142-
| **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. |
142+
| **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 counters** (better-auth `rateLimit.customStorage` fed by the kernel cache service through `createLazyCacheRateLimitStorage`; shared iff the cache is — Redis adapter in a cluster) are landed. **Correction (#4772):** this row previously claimed a shared **session** store via `secondaryStorage` as landed. It was not: the binding was taken in `AuthPlugin.init()`, which runs *before* `CacheServicePlugin` registers `cache`, so it never fired in the standard composition — and the counters it was supposed to share never reached the cache either. The counters now ride `rateLimit.customStorage`, resolved at counting time. The **session** half is deliberately NOT auto-wired: better-auth answers `findSession` from a `secondaryStorage` snapshot without reading the database, while D4 above revokes by writing the `sys_session` row, so a cache-backed session store silently disables idle-timeout / absolute-max / concurrent-cap enforcement. `cacheSecondaryStorage` remains exported for a host that opts into that trade knowingly. **Remaining:** per-org `sys_organization.allowed_ip_ranges` (+ optional `sys_user.allowed_ip_ranges` override) — tracked in #2571; the session-store question — tracked in #4785. |
143143
| **P2/P3** (D6) | 🟡 partial | Generic OIDC RP wired (`genericOAuth`/`sso`); admin OIDC **trust-list settings UI** still env/`sys_sso_provider`-only. |
144144
| **P3** (SAML, broader social) | 🟡 partial | `@better-auth/sso` present (SAML now better-auth-native — see Addendum); broader settings-driven social providers pending. |
145145

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2782,6 +2782,82 @@ describe('AuthManager', () => {
27822782
expect(captured.secondaryStorage).toBe(ss);
27832783
expect(captured.rateLimit.storage).toBe('secondary-storage');
27842784
});
2785+
2786+
// ── #4772 — counters-only store, no session relocation ────────────────
2787+
it('passes rateLimitStorage through as rateLimit.customStorage, without a secondaryStorage', async () => {
2788+
let captured: any;
2789+
(betterAuth as any).mockImplementation((cfg: any) => { captured = cfg; return { handler: vi.fn(), api: {} }; });
2790+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
2791+
const rls = { consume: vi.fn(async () => ({ allowed: true, retryAfter: null })) };
2792+
const m = new AuthManager({
2793+
secret: SECRET,
2794+
baseUrl: 'http://localhost:3000',
2795+
rateLimitStorage: rls as any,
2796+
});
2797+
await m.getAuthInstance();
2798+
warn.mockRestore();
2799+
expect(captured.rateLimit.customStorage).toBe(rls);
2800+
// The session store is untouched: ADR-0069 D4 revokes by writing the
2801+
// `sys_session` row, which better-auth stops reading once it has a
2802+
// secondaryStorage snapshot to answer from.
2803+
expect(captured).not.toHaveProperty('secondaryStorage');
2804+
});
2805+
2806+
it('keeps the operator-tuned rateLimit rules alongside the custom storage', async () => {
2807+
let captured: any;
2808+
(betterAuth as any).mockImplementation((cfg: any) => { captured = cfg; return { handler: vi.fn(), api: {} }; });
2809+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
2810+
const rls = { consume: vi.fn(async () => ({ allowed: true, retryAfter: null })) };
2811+
const m = new AuthManager({
2812+
secret: SECRET,
2813+
baseUrl: 'http://localhost:3000',
2814+
rateLimit: { enabled: true, window: 60, max: 10 } as any,
2815+
rateLimitStorage: rls as any,
2816+
});
2817+
await m.getAuthInstance();
2818+
warn.mockRestore();
2819+
expect(captured.rateLimit).toMatchObject({ enabled: true, window: 60, max: 10 });
2820+
expect(captured.rateLimit.customStorage).toBe(rls);
2821+
});
2822+
2823+
it('a host-supplied secondaryStorage wins — no customStorage is added', async () => {
2824+
let captured: any;
2825+
(betterAuth as any).mockImplementation((cfg: any) => { captured = cfg; return { handler: vi.fn(), api: {} }; });
2826+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
2827+
const ss2 = { get: vi.fn(), set: vi.fn(), delete: vi.fn() };
2828+
const rls = { consume: vi.fn(async () => ({ allowed: true, retryAfter: null })) };
2829+
const m = new AuthManager({
2830+
secret: SECRET,
2831+
baseUrl: 'http://localhost:3000',
2832+
secondaryStorage: ss2 as any,
2833+
rateLimitStorage: rls as any,
2834+
});
2835+
await m.getAuthInstance();
2836+
warn.mockRestore();
2837+
expect(captured.rateLimit.storage).toBe('secondary-storage');
2838+
expect(captured.rateLimit.customStorage).toBeUndefined();
2839+
});
2840+
2841+
// A settings patch replaces `rateLimit` wholesale (bindAuthSettings builds
2842+
// a fresh object); the counter store must survive that, or tuning the
2843+
// limits in Setup would silently un-share them again.
2844+
it('survives an applyConfigPatch that replaces rateLimit wholesale', async () => {
2845+
let captured: any;
2846+
(betterAuth as any).mockImplementation((cfg: any) => { captured = cfg; return { handler: vi.fn(), api: {} }; });
2847+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
2848+
const rls = { consume: vi.fn(async () => ({ allowed: true, retryAfter: null })) };
2849+
const m = new AuthManager({
2850+
secret: SECRET,
2851+
baseUrl: 'http://localhost:3000',
2852+
rateLimitStorage: rls as any,
2853+
});
2854+
await m.getAuthInstance();
2855+
m.applyConfigPatch({ rateLimit: { enabled: true, window: 30, max: 5 } as any });
2856+
await m.getAuthInstance();
2857+
warn.mockRestore();
2858+
expect(captured.rateLimit).toMatchObject({ enabled: true, window: 30, max: 5 });
2859+
expect(captured.rateLimit.customStorage).toBe(rls);
2860+
});
27852861
});
27862862

27872863
// ADR-0069 D1: password complexity validator (custom; better-auth only does

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

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -594,11 +594,32 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
594594
* uses it for **rate-limit counters** (the manager also flips
595595
* `rateLimit.storage` to `'secondary-storage'`) and session caching, so both
596596
* are enforced against ONE store across every node — closing the multi-node
597-
* rate-limit-bypass hole (each node otherwise counts independently). Wired by
598-
* `AuthPlugin` from the kernel `cache` service (memory single-node, Redis in
599-
* a cluster). Absent → better-auth keeps its per-process in-memory store.
597+
* rate-limit-bypass hole (each node otherwise counts independently).
598+
*
599+
* HOST-SUPPLIED ONLY (#4772). `AuthPlugin` no longer derives this from the
600+
* kernel `cache` service: better-auth also makes `secondaryStorage` the
601+
* session store (`createSession` skips the `sys_session` row, `findSession`
602+
* answers from the snapshot without reading the database), which silently
603+
* disables the ADR-0069 D4 session controls that revoke by writing that row.
604+
* The counters ride {@link rateLimitStorage} instead. Absent → better-auth
605+
* keeps its own session + rate-limit defaults.
600606
*/
601607
secondaryStorage?: BetterAuthOptions['secondaryStorage'];
608+
609+
/**
610+
* ADR-0069 D2 (#4772) — better-auth `rateLimit.customStorage`: the store the
611+
* per-IP counters are consumed from, and NOTHING else (no session
612+
* relocation, unlike {@link secondaryStorage}). `AuthPlugin` supplies a
613+
* lazily-cache-resolving implementation (`rate-limit-storage.ts`) so the
614+
* counters reach the kernel `cache` service regardless of plugin start
615+
* order. Ignored when `secondaryStorage` is set — better-auth's own
616+
* `storage: 'secondary-storage'` already routes counters there.
617+
*
618+
* Kept as its own option rather than inside {@link rateLimit} so a settings
619+
* patch that replaces `rateLimit` wholesale (`bindAuthSettings`) can never
620+
* drop the store.
621+
*/
622+
rateLimitStorage?: NonNullable<BetterAuthOptions['rateLimit']>['customStorage'];
602623
}
603624

604625
/**
@@ -919,14 +940,24 @@ export class AuthManager {
919940

920941
// ADR-0069 D2 — per-IP rate limiting (native). Only set when configured
921942
// so better-auth keeps its own defaults otherwise. The settings bind
922-
// supplies stricter `customRules` for the auth endpoints. When a shared
923-
// secondaryStorage is wired, flip the rate-limit store to it so counters
924-
// are enforced across nodes (default 'memory' is per-process).
925-
...(this.config.rateLimit || this.config.secondaryStorage
943+
// supplies stricter `customRules` for the auth endpoints.
944+
//
945+
// Where the counters live, in precedence order:
946+
// 1. a host-supplied `secondaryStorage` → better-auth's own
947+
// `storage: 'secondary-storage'` (unchanged behaviour);
948+
// 2. otherwise `rateLimitStorage` → `customStorage`, the counters-only
949+
// seam AuthPlugin fills with the lazily-resolved kernel cache
950+
// (#4772). better-auth ignores `storage` when `customStorage` is set.
951+
// Neither → better-auth's per-process 'memory' default.
952+
...(this.config.rateLimit || this.config.secondaryStorage || this.config.rateLimitStorage
926953
? {
927954
rateLimit: {
928955
...(this.config.rateLimit ?? {}),
929-
...(this.config.secondaryStorage ? { storage: 'secondary-storage' as const } : {}),
956+
...(this.config.secondaryStorage
957+
? { storage: 'secondary-storage' as const }
958+
: this.config.rateLimitStorage
959+
? { customStorage: this.config.rateLimitStorage }
960+
: {}),
930961
},
931962
}
932963
: {}),

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

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,4 +1209,115 @@ describe('AuthPlugin', () => {
12091209
expect(ql.tables.sys_member.find((m: any) => m.user_id === 'seeded_u2')).toBeUndefined();
12101210
});
12111211
});
1212+
1213+
// ── #4772 — the kernel `cache` service is resolved at COUNTING time ─────────
1214+
//
1215+
// AuthPlugin.init() runs before CacheServicePlugin registers `cache` (21ms
1216+
// earlier in a showcase cold start). The eager probe that used to live in
1217+
// init() therefore concluded "no cache" in deployments that had one, printed
1218+
// a warning telling the operator to provision Redis, and — the part that was
1219+
// not just noise — froze that conclusion, so the rate-limit counters never
1220+
// reached the shared store even after it came up.
1221+
describe('rate-limit counter store (ADR-0069 D2 / #4772)', () => {
1222+
const makeCache = () => {
1223+
const store = new Map<string, unknown>();
1224+
return {
1225+
store,
1226+
get: vi.fn(async (k: string) => (store.has(k) ? store.get(k) : undefined)),
1227+
set: vi.fn(async (k: string, v: unknown, _ttl?: number) => { store.set(k, v); }),
1228+
delete: vi.fn(async (k: string) => store.delete(k)),
1229+
has: vi.fn(async (k: string) => store.has(k)),
1230+
clear: vi.fn(async () => store.clear()),
1231+
stats: vi.fn(async () => ({ hits: 0, misses: 0, keys: store.size })),
1232+
};
1233+
};
1234+
1235+
/** Boot the plugin against a context whose `cache` resolution is under test control. */
1236+
const bootWith = async (resolveCache: () => Promise<unknown>) => {
1237+
const ctx = {
1238+
...mockContext,
1239+
getServiceAsync: vi.fn(async (name: string) => {
1240+
if (name === 'cache') return await resolveCache();
1241+
return undefined;
1242+
}),
1243+
} as unknown as PluginContext;
1244+
const plugin = new AuthPlugin({
1245+
secret: 'test-secret-at-least-32-chars-long',
1246+
baseUrl: 'http://localhost:3000',
1247+
});
1248+
await plugin.init(ctx);
1249+
const manager = (ctx.registerService as any).mock.calls.find(
1250+
([name]: [string]) => name === 'auth',
1251+
)?.[1] as AuthManager;
1252+
return { ctx, manager, storage: (manager as any).config.rateLimitStorage };
1253+
};
1254+
1255+
it('does not warn during init when the cache has not registered yet', async () => {
1256+
// Exactly the showcase cold start: `cache` is not in the registry when
1257+
// plugin-auth initializes.
1258+
const { ctx } = await bootWith(async () => undefined);
1259+
const warned = (ctx.logger.warn as any).mock.calls.map((c: any[]) => String(c[0]));
1260+
expect(warned.some((m: string) => m.includes('no cache service registered'))).toBe(false);
1261+
expect(warned.some((m: string) => m.includes('rate-limit'))).toBe(false);
1262+
});
1263+
1264+
it('counts in the cache registered AFTER init — the counters really are shared', async () => {
1265+
const cache = makeCache();
1266+
let registered = false;
1267+
const { ctx, storage } = await bootWith(async () => (registered ? cache : undefined));
1268+
expect(storage).toBeDefined();
1269+
1270+
// CacheServicePlugin comes up 21ms later.
1271+
registered = true;
1272+
1273+
const decision = await storage.consume('ip:203.0.113.7', { window: 60, max: 3 });
1274+
expect(decision).toEqual({ allowed: true, retryAfter: null });
1275+
expect(cache.store.size).toBe(1);
1276+
const info = (ctx.logger.info as any).mock.calls.map((c: any[]) => String(c[0]));
1277+
expect(info.some((m: string) => m.includes('rate-limit counters bound to the kernel cache service'))).toBe(true);
1278+
});
1279+
1280+
it('warns at counting time — and only then — when there is genuinely no cache service', async () => {
1281+
const { ctx, storage } = await bootWith(async () => undefined);
1282+
const rateLimitWarnings = () =>
1283+
(ctx.logger.warn as any).mock.calls
1284+
.map((c: any[]) => String(c[0]))
1285+
.filter((m: string) => m.includes('rate-limit counters'));
1286+
expect(rateLimitWarnings()).toEqual([]);
1287+
1288+
await storage.consume('ip:198.51.100.9', { window: 60, max: 3 });
1289+
expect(rateLimitWarnings()).toHaveLength(1);
1290+
1291+
const warned = (ctx.logger.warn as any).mock.calls.map((c: any[]) => String(c[0]));
1292+
expect(warned.some((m: string) => m.includes('no `cache` service registered at all'))).toBe(true);
1293+
});
1294+
1295+
it('leaves the counters to better-auth when the host supplies its own secondaryStorage', async () => {
1296+
const ctx = {
1297+
...mockContext,
1298+
getServiceAsync: vi.fn(async () => undefined),
1299+
} as unknown as PluginContext;
1300+
const plugin = new AuthPlugin({
1301+
secret: 'test-secret-at-least-32-chars-long',
1302+
baseUrl: 'http://localhost:3000',
1303+
secondaryStorage: { get: vi.fn(), set: vi.fn(), delete: vi.fn() },
1304+
} as any);
1305+
await plugin.init(ctx);
1306+
const manager = (ctx.registerService as any).mock.calls.find(
1307+
([name]: [string]) => name === 'auth',
1308+
)?.[1] as AuthManager;
1309+
expect((manager as any).config.rateLimitStorage).toBeUndefined();
1310+
expect((manager as any).config.secondaryStorage).toBeDefined();
1311+
});
1312+
1313+
it('never binds the kernel cache as better-auth secondaryStorage (sessions stay in sys_session)', async () => {
1314+
// ADR-0069 D4's session controls revoke by writing the `sys_session` row;
1315+
// better-auth answers `findSession` from a secondaryStorage snapshot
1316+
// without reading the database, so a cache-backed session store would
1317+
// silently disable them. The cache reaches the COUNTERS only.
1318+
const cache = makeCache();
1319+
const { manager } = await bootWith(async () => cache);
1320+
expect((manager as any).config.secondaryStorage).toBeUndefined();
1321+
});
1322+
});
12121323
});

0 commit comments

Comments
 (0)