Skip to content

Commit cb5b393

Browse files
os-zhuangclaude
andauthored
feat(plugin-auth): account lockout + rate-limit tuning (ADR-0069 D2, P1) (#2365)
Per-identity brute-force protection, the second ADR-0069 slice (after HIBP), reusing the setting -> enforcement-seam pattern. - Account lockout [custom][field]: sys_user.failed_login_count / locked_until columns; `lockout_threshold` (0=off) + `lockout_duration_minutes` settings. Enforced in the /sign-in/email before/after hooks: failures increment, crossing the threshold stamps locked_until, and a locked account is rejected even with the correct password (survives IP rotation). Success resets both. - Admin Unlock: admin-guarded POST /api/v1/auth/admin/unlock-user + sys_user `unlock_user` action. - Rate-limit tuning [native]: `rate_limit_max` / `rate_limit_window_seconds` wire better-auth core `rateLimit` with stricter customRules on the auth endpoints. Default-off / additive (no upgrade behavior change); ADR-0049 (enforcement ships with the setting); timestamps written as Date, never epoch-ms (ADR-0074). Verified live (dogfood): 3 failed sign-ins lock the account; the correct password is then rejected with ACCOUNT_LOCKED while OTHER users (incl. admin) sign in normally; admin unlock restores access; unauthenticated unlock 401; rate-limit burst trips 429 after the cap. Unit: 158 plugin-auth + 129 service-settings + 63 platform-objects green; the lockout mock honours the ObjectQL `where` key so the filter/where bug dogfood caught cannot regress. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 22b32c1 commit cb5b393

8 files changed

Lines changed: 525 additions & 0 deletions

File tree

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
'@objectstack/platform-objects': minor
3+
'@objectstack/service-settings': minor
4+
'@objectstack/plugin-auth': minor
5+
'@objectstack/cli': minor
6+
---
7+
8+
Auth: account lockout + rate-limit tuning (ADR-0069 D2, P1)
9+
10+
Second slice of ADR-0069 — per-identity brute-force protection, reusing the setting→enforcement pattern from the HIBP PR.
11+
12+
- **Account lockout** `[custom][field]`: new `sys_user.failed_login_count` / `sys_user.locked_until` columns; `auth` settings `lockout_threshold` (0 = off) + `lockout_duration_minutes`. Enforced in the `/sign-in/email` before/after hooks — failures increment the counter, crossing the threshold stamps `locked_until`, and a locked account is rejected **even with the correct password** (survives IP rotation, unlike rate limiting). A successful sign-in resets both.
13+
- **Admin Unlock**: new admin-guarded `POST /api/v1/auth/admin/unlock-user` route + an `unlock_user` action on `sys_user`.
14+
- **Rate-limit tuning** `[native]`: `auth` settings `rate_limit_max` / `rate_limit_window_seconds` wire better-auth's core `rateLimit` with stricter `customRules` for `/sign-in/email`, `/sign-up/email`, `/request-password-reset`, `/reset-password`.
15+
16+
All settings default off / to safe values; additive (no upgrade behavior change). Per ADR-0049 each setting ships with its enforcement. Timestamps are written as `Date` (never epoch-ms) per ADR-0074.

packages/platform-objects/src/identity/sys-user.object.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,21 @@ export const SysUser = ObjectSchema.create({
9797
successMessage: 'User unbanned',
9898
refreshAfter: true,
9999
},
100+
{
101+
// ADR-0069 D2 — clear a brute-force lockout early (locked_until auto-
102+
// expires, but an admin can release a user immediately). Hits the
103+
// plugin-auth custom route, which is admin-guarded server-side.
104+
name: 'unlock_user',
105+
label: 'Unlock Account',
106+
icon: 'lock-open',
107+
variant: 'secondary',
108+
locations: ['list_item'],
109+
type: 'api',
110+
target: '/api/v1/auth/admin/unlock-user',
111+
recordIdParam: 'userId',
112+
successMessage: 'Account unlocked',
113+
refreshAfter: true,
114+
},
100115
{
101116
name: 'set_user_password',
102117
label: 'Set Password',
@@ -410,6 +425,28 @@ export const SysUser = ObjectSchema.create({
410425
description: 'When set, the ban auto-clears at this time.',
411426
}),
412427

428+
// ── Anti-brute-force (ADR-0069 D2) — owned by objectql, better-auth is
429+
// oblivious. The auth manager's sign-in hooks maintain these: failures
430+
// increment the counter; crossing `lockout_threshold` stamps
431+
// `locked_until`; a successful sign-in resets both. Admins can clear
432+
// them early via the Unlock action.
433+
failed_login_count: Field.number({
434+
label: 'Failed Login Count',
435+
required: false,
436+
defaultValue: 0,
437+
readonly: true,
438+
group: 'Admin',
439+
description: 'Consecutive failed sign-in attempts; reset to 0 on success. Maintained by the auth manager.',
440+
}),
441+
442+
locked_until: Field.datetime({
443+
label: 'Locked Until',
444+
required: false,
445+
readonly: true,
446+
group: 'Admin',
447+
description: 'When set and in the future, sign-in is rejected (brute-force lockout). Auto-clears past this time; an admin can clear it early via Unlock.',
448+
}),
449+
413450
ai_access: Field.boolean({
414451
label: 'AI Access',
415452
defaultValue: false,

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

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1594,4 +1594,141 @@ describe('AuthManager', () => {
15941594
}
15951595
});
15961596
});
1597+
1598+
// ADR-0069 D2: per-identity account lockout + native rate-limit passthrough.
1599+
// The lockout state machine is exercised directly via the AuthManager helpers
1600+
// with a mocked data engine (deterministic; the live multi-failure path is
1601+
// covered by the dogfood smoke).
1602+
describe('account lockout + rate limiting (ADR-0069 D2)', () => {
1603+
const SECRET = 'test-secret-at-least-32-chars-long';
1604+
// findOne honours the `where` clause (the ObjectQL engine key) — so a
1605+
// regression to the wrong key (`filter`, silently ignored → returns the
1606+
// first/arbitrary row) makes these tests fail, not pass. Caught a real bug
1607+
// in dogfood that a query-agnostic mock had masked.
1608+
const makeEngine = (user: any) => ({
1609+
findOne: vi.fn(async (_obj: string, q: any) => {
1610+
const w = q?.where ?? {};
1611+
if (!user) return null;
1612+
const matches = Object.entries(w).every(([k, v]) => (user as any)[k] === v);
1613+
return matches ? user : null;
1614+
}),
1615+
update: vi.fn(async () => ({ ...(user ?? {}) })),
1616+
count: vi.fn(),
1617+
});
1618+
const mgr = (engine: any, extra: any = {}) => {
1619+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
1620+
const m = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000', dataEngine: engine, ...extra });
1621+
warn.mockRestore();
1622+
return m;
1623+
};
1624+
1625+
it('assertAccountNotLocked is a no-op when lockout is disabled (threshold 0)', async () => {
1626+
const engine = makeEngine({ id: 'u1', email: 'a@b.com', locked_until: new Date(Date.now() + 60_000).toISOString() });
1627+
const m = mgr(engine, { lockoutThreshold: 0 });
1628+
await expect((m as any).assertAccountNotLocked('a@b.com')).resolves.toBeUndefined();
1629+
expect(engine.findOne).not.toHaveBeenCalled();
1630+
});
1631+
1632+
it('assertAccountNotLocked throws ACCOUNT_LOCKED while locked_until is in the future', async () => {
1633+
const engine = makeEngine({ id: 'u1', email: 'a@b.com', locked_until: new Date(Date.now() + 60_000).toISOString() });
1634+
const m = mgr(engine, { lockoutThreshold: 3 });
1635+
await expect((m as any).assertAccountNotLocked('a@b.com')).rejects.toMatchObject({
1636+
body: { code: 'ACCOUNT_LOCKED' },
1637+
});
1638+
});
1639+
1640+
it('assertAccountNotLocked allows sign-in once the lock has expired', async () => {
1641+
const engine = makeEngine({ id: 'u1', email: 'a@b.com', locked_until: new Date(Date.now() - 60_000).toISOString() });
1642+
const m = mgr(engine, { lockoutThreshold: 3 });
1643+
await expect((m as any).assertAccountNotLocked('a@b.com')).resolves.toBeUndefined();
1644+
});
1645+
1646+
it('recordSignInOutcome increments below threshold without locking', async () => {
1647+
const engine = makeEngine({ id: 'u1', email: 'a@b.com', failed_login_count: 0, locked_until: null });
1648+
const m = mgr(engine, { lockoutThreshold: 3 });
1649+
await (m as any).recordSignInOutcome('a@b.com', false);
1650+
const patch = engine.update.mock.calls[0][1];
1651+
expect(patch.failed_login_count).toBe(1);
1652+
expect(patch.locked_until).toBeUndefined();
1653+
});
1654+
1655+
it('recordSignInOutcome stamps locked_until (a Date) once the threshold is reached', async () => {
1656+
const engine = makeEngine({ id: 'u1', email: 'a@b.com', failed_login_count: 2, locked_until: null });
1657+
const m = mgr(engine, { lockoutThreshold: 3, lockoutDurationMinutes: 15 });
1658+
await (m as any).recordSignInOutcome('a@b.com', false);
1659+
const patch = engine.update.mock.calls[0][1];
1660+
expect(patch.failed_login_count).toBe(3);
1661+
expect(patch.locked_until instanceof Date).toBe(true);
1662+
// ~15 minutes out (datetime stored as a Date, never epoch-ms — see ADR-0074).
1663+
expect((patch.locked_until as Date).getTime()).toBeGreaterThan(Date.now() + 14 * 60_000);
1664+
});
1665+
1666+
it('recordSignInOutcome resets counter + lock on success when there is state to clear', async () => {
1667+
const engine = makeEngine({ id: 'u1', email: 'a@b.com', failed_login_count: 2, locked_until: null });
1668+
const m = mgr(engine, { lockoutThreshold: 3 });
1669+
await (m as any).recordSignInOutcome('a@b.com', true);
1670+
expect(engine.update).toHaveBeenCalledWith(
1671+
'sys_user',
1672+
{ id: 'u1', failed_login_count: 0, locked_until: null },
1673+
expect.anything(),
1674+
);
1675+
});
1676+
1677+
it('recordSignInOutcome skips the write on success when nothing needs clearing', async () => {
1678+
const engine = makeEngine({ id: 'u1', email: 'a@b.com', failed_login_count: 0, locked_until: null });
1679+
const m = mgr(engine, { lockoutThreshold: 3 });
1680+
await (m as any).recordSignInOutcome('a@b.com', true);
1681+
expect(engine.update).not.toHaveBeenCalled();
1682+
});
1683+
1684+
it('recordSignInOutcome is a no-op when lockout is disabled', async () => {
1685+
const engine = makeEngine({ id: 'u1', email: 'a@b.com', failed_login_count: 5 });
1686+
const m = mgr(engine, { lockoutThreshold: 0 });
1687+
await (m as any).recordSignInOutcome('a@b.com', false);
1688+
expect(engine.update).not.toHaveBeenCalled();
1689+
});
1690+
1691+
it('unlockUser clears failed_login_count and locked_until', async () => {
1692+
const engine = makeEngine({ id: 'u1' });
1693+
const m = mgr(engine);
1694+
await expect(m.unlockUser('u1')).resolves.toBe(true);
1695+
expect(engine.update).toHaveBeenCalledWith(
1696+
'sys_user',
1697+
{ id: 'u1', failed_login_count: 0, locked_until: null },
1698+
expect.anything(),
1699+
);
1700+
});
1701+
1702+
it('unlockUser returns false for an unknown user', async () => {
1703+
const engine = makeEngine(null);
1704+
const m = mgr(engine);
1705+
await expect(m.unlockUser('nope')).resolves.toBe(false);
1706+
expect(engine.update).not.toHaveBeenCalled();
1707+
});
1708+
1709+
it('passes a configured rateLimit through to betterAuth', async () => {
1710+
let captured: any;
1711+
(betterAuth as any).mockImplementation((cfg: any) => { captured = cfg; return { handler: vi.fn(), api: {} }; });
1712+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
1713+
const m = new AuthManager({
1714+
secret: SECRET,
1715+
baseUrl: 'http://localhost:3000',
1716+
rateLimit: { enabled: true, window: 60, max: 10, customRules: { '/sign-in/email': { window: 60, max: 10 } } } as any,
1717+
});
1718+
await m.getAuthInstance();
1719+
warn.mockRestore();
1720+
expect(captured.rateLimit).toMatchObject({ enabled: true, max: 10, window: 60 });
1721+
expect(captured.rateLimit.customRules['/sign-in/email']).toEqual({ window: 60, max: 10 });
1722+
});
1723+
1724+
it('omits rateLimit from the betterAuth config when unset (keeps library defaults)', async () => {
1725+
let captured: any;
1726+
(betterAuth as any).mockImplementation((cfg: any) => { captured = cfg; return { handler: vi.fn(), api: {} }; });
1727+
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
1728+
const m = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000' });
1729+
await m.getAuthInstance();
1730+
warn.mockRestore();
1731+
expect(captured).not.toHaveProperty('rateLimit');
1732+
});
1733+
});
15971734
});

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

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,26 @@ export interface AuthManagerOptions extends Partial<AuthConfig> {
269269
* "create organization" screen.
270270
*/
271271
databaseHooks?: BetterAuthOptions['databaseHooks'];
272+
273+
/**
274+
* ADR-0069 D2 — account lockout (anti-brute-force). After this many
275+
* consecutive failed sign-ins the account is locked for
276+
* {@link lockoutDurationMinutes}. `0` (default) disables lockout.
277+
* Enforced per-identity in the `/sign-in/email` before/after hooks
278+
* (survives IP rotation, unlike the per-IP {@link rateLimit}).
279+
*/
280+
lockoutThreshold?: number;
281+
282+
/** Minutes an account stays locked once the threshold is crossed. Default 15. */
283+
lockoutDurationMinutes?: number;
284+
285+
/**
286+
* ADR-0069 D2 — better-auth-native per-IP rate limiting, passed through to
287+
* better-auth's core `rateLimit`. The settings bind tightens `customRules`
288+
* for the auth endpoints (`/sign-in/email`, `/sign-up/email`,
289+
* `/reset-password`). Multi-node deployments need a shared `storage`.
290+
*/
291+
rateLimit?: BetterAuthOptions['rateLimit'];
272292
}
273293

274294
/**
@@ -531,6 +551,11 @@ export class AuthManager {
531551
updateAge: this.config.session?.updateAge || 60 * 60 * 24, // 1 day default
532552
},
533553

554+
// ADR-0069 D2 — per-IP rate limiting (native). Only set when configured
555+
// so better-auth keeps its own defaults otherwise. The settings bind
556+
// supplies stricter `customRules` for the auth endpoints.
557+
...(this.config.rateLimit ? { rateLimit: this.config.rateLimit } : {}),
558+
534559
// better-auth plugins — registered based on AuthPluginConfig flags
535560
plugins,
536561

@@ -704,6 +729,15 @@ export class AuthManager {
704729
// fall through to better-auth's own handler
705730
}
706731

732+
// ── ADR-0069 D2: account lockout (gate) ─────────────────────
733+
// Reject a sign-in for a locked identity BEFORE better-auth checks
734+
// the password — a lock must hold even against the correct password.
735+
if (ctx?.path === '/sign-in/email') {
736+
const email = typeof ctx?.body?.email === 'string' ? ctx.body.email : '';
737+
if (email) await this.assertAccountNotLocked(email);
738+
return;
739+
}
740+
707741
if (ctx?.path !== '/sign-up/email') return;
708742
const ep = ctx?.context?.options?.emailAndPassword;
709743
if (!ep?.disableSignUp) return;
@@ -719,6 +753,25 @@ export class AuthManager {
719753
}
720754
}),
721755
after: createAuthMiddleware(async (ctx: any) => {
756+
// ── ADR-0069 D2: account lockout (counter) ──────────────────
757+
// better-auth catches an INVALID_EMAIL_OR_PASSWORD APIError and runs
758+
// the after-hook with it on `ctx.context.returned`; a success leaves
759+
// the session payload there. Count failures, reset on success.
760+
if (ctx?.path === '/sign-in/email') {
761+
const email = typeof ctx?.body?.email === 'string' ? ctx.body.email : '';
762+
if (email) {
763+
let succeeded = true;
764+
try {
765+
const { isAPIError } = await import('better-auth/api');
766+
succeeded = !isAPIError(ctx?.context?.returned);
767+
} catch {
768+
succeeded = !(ctx?.context?.returned instanceof Error);
769+
}
770+
await this.recordSignInOutcome(email, succeeded);
771+
}
772+
return;
773+
}
774+
722775
if (ctx?.path !== '/sign-up/email') return;
723776
const ep = ctx?.context?.options?.emailAndPassword;
724777
if (ep && ctx.context.__osDisableSignUpOrig !== undefined) {
@@ -2008,6 +2061,103 @@ export class AuthManager {
20082061
}
20092062
}
20102063

2064+
/**
2065+
* ADR-0069 D2 — throw `ACCOUNT_LOCKED` when the identity is currently locked
2066+
* out (brute-force protection). No-op when lockout is disabled
2067+
* (`lockoutThreshold <= 0`) or no data engine is wired. Fails OPEN on a
2068+
* lookup error: an infra hiccup must never block every login.
2069+
*/
2070+
private async assertAccountNotLocked(email: string): Promise<void> {
2071+
const threshold = Number(this.config.lockoutThreshold) || 0;
2072+
if (threshold <= 0) return;
2073+
const engine = this.getDataEngine();
2074+
if (!engine) return;
2075+
let locked = false;
2076+
try {
2077+
const SYSTEM_CTX = { isSystem: true, roles: [], permissions: [] };
2078+
const u = await engine.findOne('sys_user', {
2079+
where: { email }, fields: ['id', 'locked_until'], context: SYSTEM_CTX,
2080+
} as any);
2081+
const lu = u?.locked_until;
2082+
locked = !!(lu && new Date(lu).getTime() > Date.now());
2083+
} catch {
2084+
return; // fail-open
2085+
}
2086+
if (locked) {
2087+
const { APIError } = await import('better-auth/api');
2088+
throw new APIError('FORBIDDEN', {
2089+
message:
2090+
'This account is temporarily locked after too many failed sign-in ' +
2091+
'attempts. Try again later or ask an administrator to unlock it.',
2092+
code: 'ACCOUNT_LOCKED',
2093+
});
2094+
}
2095+
}
2096+
2097+
/**
2098+
* ADR-0069 D2 — record a sign-in outcome for lockout accounting. On failure
2099+
* increments `failed_login_count` and, once it reaches `lockoutThreshold`,
2100+
* stamps `locked_until = now + lockoutDurationMinutes`. On success resets
2101+
* both (only writing when there is something to clear, to avoid a no-op
2102+
* history row on every login). No-op when lockout is disabled. Never throws —
2103+
* a counter write must not turn a valid login into an error.
2104+
*/
2105+
private async recordSignInOutcome(email: string, success: boolean): Promise<void> {
2106+
const threshold = Number(this.config.lockoutThreshold) || 0;
2107+
if (threshold <= 0) return;
2108+
const engine = this.getDataEngine();
2109+
if (!engine) return;
2110+
try {
2111+
const SYSTEM_CTX = { isSystem: true, roles: [], permissions: [] };
2112+
const u = await engine.findOne('sys_user', {
2113+
where: { email },
2114+
fields: ['id', 'failed_login_count', 'locked_until'],
2115+
context: SYSTEM_CTX,
2116+
} as any);
2117+
if (!u?.id) return;
2118+
if (success) {
2119+
if ((Number(u.failed_login_count) || 0) !== 0 || u.locked_until) {
2120+
await engine.update(
2121+
'sys_user',
2122+
{ id: u.id, failed_login_count: 0, locked_until: null },
2123+
{ context: SYSTEM_CTX } as any,
2124+
);
2125+
}
2126+
return;
2127+
}
2128+
const next = (Number(u.failed_login_count) || 0) + 1;
2129+
const patch: Record<string, unknown> = { id: u.id, failed_login_count: next };
2130+
if (next >= threshold) {
2131+
const mins = Number(this.config.lockoutDurationMinutes) || 15;
2132+
patch.locked_until = new Date(Date.now() + mins * 60_000);
2133+
}
2134+
await engine.update('sys_user', patch, { context: SYSTEM_CTX } as any);
2135+
} catch {
2136+
// Lockout accounting is best-effort — never break the auth response.
2137+
}
2138+
}
2139+
2140+
/**
2141+
* ADR-0069 D2 — clear a user's lockout state (admin "Unlock" action).
2142+
* Resets `failed_login_count` and `locked_until`. Returns false when no data
2143+
* engine is wired or the user does not exist.
2144+
*/
2145+
public async unlockUser(userId: string): Promise<boolean> {
2146+
const engine = this.getDataEngine();
2147+
if (!engine || !userId) return false;
2148+
const SYSTEM_CTX = { isSystem: true, roles: [], permissions: [] };
2149+
const u = await engine.findOne('sys_user', {
2150+
where: { id: userId }, fields: ['id'], context: SYSTEM_CTX,
2151+
} as any);
2152+
if (!u?.id) return false;
2153+
await engine.update(
2154+
'sys_user',
2155+
{ id: userId, failed_login_count: 0, locked_until: null },
2156+
{ context: SYSTEM_CTX } as any,
2157+
);
2158+
return true;
2159+
}
2160+
20112161
/**
20122162
* Returns the data engine wired into this auth manager. Used by route
20132163
* handlers (e.g. bootstrap-status) that need to query identity tables

0 commit comments

Comments
 (0)