From 42ea8e91a366030b555a8ab59edbb0c83ec3a4a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 05:07:52 +0000 Subject: [PATCH 1/5] fix(plugin-audit,plugin-auth): a refused sign-in and an account lockout each write an audit row A refused password sign-in now writes a `login_failed` row and the refusal that stamps the ADR-0069 D2 lockout also writes an `account_locked` row, as a successful sign-in already writes `login`. Each names the account, the outcome, the time and the client fingerprint (resolved by the same rule as a login row), and never the attempted credential. Both actions join `sys_audit_log.action` with their writer and the Auth list view. Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r Co-authored-by: Claude --- .../src/audit-log-field-redaction.ts | 4 +- .../src/audit-log-read-visibility.ts | 6 +- .../plugin-audit/src/auth-event-audit.test.ts | 81 ++++++ .../plugin-audit/src/auth-event-audit.ts | 139 +++++++-- packages/plugins/plugin-audit/src/index.ts | 2 + .../sys-audit-log-retired-actions.test.ts | 10 + .../src/objects/sys-audit-log.object.ts | 26 +- .../src/auth-account-event-audit.test.ts | 267 ++++++++++++++++++ .../plugins/plugin-auth/src/auth-manager.ts | 138 ++++++++- .../plugin-auth/src/auth-session-audit.ts | 131 ++++++++- 10 files changed, 753 insertions(+), 51 deletions(-) create mode 100644 packages/plugins/plugin-auth/src/auth-account-event-audit.test.ts diff --git a/packages/plugins/plugin-audit/src/audit-log-field-redaction.ts b/packages/plugins/plugin-audit/src/audit-log-field-redaction.ts index dbda97d3e2d..fecc574ae22 100644 --- a/packages/plugins/plugin-audit/src/audit-log-field-redaction.ts +++ b/packages/plugins/plugin-audit/src/audit-log-field-redaction.ts @@ -48,8 +48,8 @@ * - A record-write row's snapshot that is not a JSON object cannot be judged * key by key, so it is dropped whole (fail closed). * - A row of any other action is served as written. Measured per writer on - * the tree this landed on: `read`, `login`, `logout` and `import` rows - * write no snapshot; a `config_change` row's snapshot is a DIGEST of the + * the tree this landed on: `read`, `login`, `logout`, `login_failed`, + * `account_locked` and `import` rows write no snapshot; a `config_change` row's snapshot is a DIGEST of the * setting, never its value (`service-settings`' `config-change-audit.ts`); * a `platform_admin_standing_change` row's is the deployment's * administrator roster (`plugin-security`). None is a parent record's field diff --git a/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts b/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts index 21c4620c200..592ed944867 100644 --- a/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts +++ b/packages/plugins/plugin-audit/src/audit-log-read-visibility.ts @@ -33,8 +33,10 @@ * - **Rows about a record** — they name `object_name` + `record_id`: the CRUD * mirror's `create` / `update` / `delete` (`audit-writers.ts`), record-view * `read` rows (`read-audit.ts`), plugin-auth's administrative `create` / - * `update` on a `sys_user`, and the auth-event sink's `login` / `logout`, - * which name the session (`auth-event-audit.ts`). Judged by the record gate; + * `update` on a `sys_user`, the auth-event sink's `login` / `logout`, + * which name the session, and its `login_failed` / `account_locked`, which + * name the account's `sys_user` row (`auth-event-audit.ts`). Judged by the + * record gate; * the session is in the audit writer's `AUDIT_EXCLUDED_OBJECTS`, so the gate * names it ({@link LEDGER_NAMED_PARENTS}) to keep judging it. * A record that no longer exists is read by no caller, so its rows are diff --git a/packages/plugins/plugin-audit/src/auth-event-audit.test.ts b/packages/plugins/plugin-audit/src/auth-event-audit.test.ts index c9b89a1c9d4..dc9ffe4d38a 100644 --- a/packages/plugins/plugin-audit/src/auth-event-audit.test.ts +++ b/packages/plugins/plugin-audit/src/auth-event-audit.test.ts @@ -365,6 +365,87 @@ describe('[#8144] createAuthEventAuditSink writes a login row that names its act * The helpers are `audit-writers.ts`'s, imported rather than re-spelled: a * second copy of this key is how the defect got here in the first place. */ +describe('a refused sign-in and a lockout each land as a ledger row about the account', () => { + it('login_failed names the account as user and target, credits no principal, and keeps outcome and client', async () => { + const { engine } = await boot(); + const sink = createAuthEventAuditSink({ getEngine: () => engine as any }); + + await sink.recordAuthEvent({ + action: 'login_failed', + userId: 'usr_1', + ipAddress: '203.0.113.7', + userAgent: 'Mozilla/5.0 (probe)', + context: { endpoint: '/sign-in/email', reason: 'INVALID_EMAIL_OR_PASSWORD' }, + }); + + const rows = await ledgerRows(engine, { action: 'login_failed' }); + expect(rows).toHaveLength(1); + const [row] = rows; + // The account attempted: the subject lookup, and the record the row is about. + expect(row.user_id).toBe('usr_1'); + expect(row.object_name).toBe('sys_user'); + expect(row.record_id).toBe('usr_1'); + // A refused attempt authenticated nobody, so no principal is credited with + // it — and in particular not the account holder. + expect(row.actor ?? null).toBeNull(); + // The outcome and the client. + expect(JSON.parse(String(row.metadata))).toEqual({ + endpoint: '/sign-in/email', + reason: 'INVALID_EMAIL_OR_PASSWORD', + }); + expect(row.ip_address).toBe('203.0.113.7'); + expect(row.user_agent).toBe('Mozilla/5.0 (probe)'); + // No session, no organization context, no field diff. + expect(row.tenant_id ?? null).toBeNull(); + expect(row.old_value ?? null).toBeNull(); + expect(row.new_value ?? null).toBeNull(); + }); + + it('account_locked is its own row about the account, carrying the lock', async () => { + const { engine } = await boot(); + const sink = createAuthEventAuditSink({ getEngine: () => engine as any }); + + await sink.recordAuthEvent({ + action: 'login_failed', + userId: 'usr_1', + context: { endpoint: '/sign-in/email', reason: 'INVALID_EMAIL_OR_PASSWORD' }, + }); + await sink.recordAuthEvent({ + action: 'account_locked', + userId: 'usr_1', + ipAddress: '203.0.113.7', + context: { endpoint: '/sign-in/email', locked_until: '2026-10-20T12:15:00.000Z', threshold: 3 }, + }); + + expect(await ledgerRows(engine, { action: 'login_failed' })).toHaveLength(1); + const locks = await ledgerRows(engine, { action: 'account_locked' }); + expect(locks).toHaveLength(1); + expect(locks[0].user_id).toBe('usr_1'); + expect(locks[0].object_name).toBe('sys_user'); + expect(locks[0].record_id).toBe('usr_1'); + expect(locks[0].actor ?? null).toBeNull(); + expect(locks[0].ip_address).toBe('203.0.113.7'); + expect(JSON.parse(String(locks[0].metadata))).toEqual({ + endpoint: '/sign-in/email', + locked_until: '2026-10-20T12:15:00.000Z', + threshold: 3, + }); + }); + + it('a session event still names its session and credits its subject', async () => { + // The account target must not leak onto the session class. + const { engine } = await boot(); + const sink = createAuthEventAuditSink({ getEngine: () => engine as any }); + + await sink.recordAuthEvent({ action: 'login', userId: 'usr_1', sessionId: 'ses_1' }); + + const [row] = await ledgerRows(engine, { action: 'login' }); + expect(row.object_name).toBe('sys_session'); + expect(row.record_id).toBe('ses_1'); + expect(row.actor).toBe('usr_1'); + }); +}); + describe('auth-event audit — reported once per CAUSE, not once per process (#17452)', () => { interface LogLine { level: string; diff --git a/packages/plugins/plugin-audit/src/auth-event-audit.ts b/packages/plugins/plugin-audit/src/auth-event-audit.ts index f136d1944e8..df456a06ed8 100644 --- a/packages/plugins/plugin-audit/src/auth-event-audit.ts +++ b/packages/plugins/plugin-audit/src/auth-event-audit.ts @@ -48,7 +48,7 @@ * either direction**: a row with a misspelled action is accepted silently and * no test that merely watches for a throw can see it. The only structural * protection available is at authoring time, which is why - * {@link AuthSessionAuditAction} is a closed literal union rather than + * {@link AuthEventAuditAction} is a closed literal union rather than * `string`, and why the caller hands over an EVENT (what happened) instead of a * row (what to store). * @@ -56,9 +56,28 @@ * * `AuditPlugin` registers this sink under the `audit` service slot; plugin-auth * resolves it lazily through a locally-declared structural surface and calls it - * from better-auth's session lifecycle hooks. Neither package depends on the - * other — the same shape `audit-writers.ts` uses to reach messaging/i18n - * without depending on those services. + * from better-auth's session lifecycle hooks and from its password sign-in + * outcome. Neither package depends on the other — the same shape + * `audit-writers.ts` uses to reach messaging/i18n without depending on those + * services. + * + * ## Two classes of auth event, two row targets + * + * - **Session events** (`login`, `logout`) are about a SESSION: the row names + * `sys_session` and the session id, and `actor` defaults to the subject, + * who acted for themselves unless an impersonating admin is named. + * - **Account events** (`login_failed`, `account_locked`) are about an + * ACCOUNT and exist without any session: a refused sign-in mints none, and + * a lockout is a state of the `sys_user` row. The row names `sys_user` and + * the account's id, so it is navigable to the account and judged by the + * ledger's record read gate exactly as any row about a user is. `actor` is + * left null: a refused attempt authenticated nobody, so no principal can be + * credited with it, and recording the account holder there would claim + * they made the attempt. + * + * ⛔ An account event never carries the attempted credential, in any form. + * Its `context` is the endpoint, the refusal's machine code and, for a + * lockout, the lock's expiry and threshold — nothing the caller typed. */ import type { IDataEngine } from '@objectstack/spec/contracts'; @@ -73,28 +92,67 @@ import { export type AuthSessionAuditAction = 'login' | 'logout'; /** - * One auth session event, in the vocabulary of what HAPPENED rather than of - * what to store. Everything but `action` and `userId` is optional: an event - * missing its client fingerprint is still worth recording, and a partial row - * beats no row on a compliance ledger. + * The two sign-in outcome events on an account (`sys_audit_log.action`): a + * password sign-in that was refused, and the transition of an account into + * the ADR-0069 D2 lockout. + */ +export type AuthAccountAuditAction = 'login_failed' | 'account_locked'; + +/** Every action this sink writes. */ +export type AuthEventAuditAction = AuthSessionAuditAction | AuthAccountAuditAction; + +/** + * The account-event actions, as a value — the one place that decides which + * actions take the account row target described in the module note. + */ +export const AUTH_ACCOUNT_AUDIT_ACTIONS: readonly AuthAccountAuditAction[] = [ + 'login_failed', + 'account_locked', +]; + +const ACCOUNT_ACTIONS = new Set(AUTH_ACCOUNT_AUDIT_ACTIONS); + +/** Whether `action` is an account event (module note: "two row targets"). */ +export function isAuthAccountAuditAction(action: unknown): action is AuthAccountAuditAction { + return typeof action === 'string' && ACCOUNT_ACTIONS.has(action); +} + +/** + * One auth event, in the vocabulary of what HAPPENED rather than of what to + * store. Everything but `action` and `userId` is optional: an event missing + * its client fingerprint is still worth recording, and a partial row beats no + * row on a compliance ledger. */ export interface AuthSessionAuditEvent { /** Which event. Closed union — see the module note on the unvalidated enum. */ - action: AuthSessionAuditAction; - /** The session's subject — a real `sys_user` id (the `user_id` lookup). */ + action: AuthEventAuditAction; + /** + * The event's subject — a real `sys_user` id (the `user_id` lookup): the + * session's owner for a session event, the account the sign-in named for an + * account event. + */ userId: string; - /** The `sys_session` row this event is about, when known. */ + /** + * The `sys_session` row a SESSION event is about, when known. An account + * event names its account instead (module note), so this is not read for + * one. + */ sessionId?: string; /** Tenant context — the session's active organization, when it has one. */ organizationId?: string; - /** Client fingerprint, as better-auth recorded it on the session row. */ + /** + * Client fingerprint, by better-auth's own rule: the session row's values + * for a session event, and the same resolution applied to the sign-in + * request for an account event, which has no session row. + */ ipAddress?: string; userAgent?: string; /** * The principal that CAUSED the event, when it is not the subject — an * admin's user id on an impersonation session. Lands on `actor`, which the * schema defines as "Principal that performed the action", independent of the - * `user_id` lookup. Absent → the subject acted for themselves. + * `user_id` lookup. Absent → the subject acted for themselves on a session + * event, and no principal is recorded on an account event (module note). */ actor?: string; /** @@ -130,9 +188,9 @@ export interface AuthEventAuditLogger { /** What {@link createAuthEventAuditSink} registers under the `audit` slot. */ export interface AuthEventAuditSink { /** - * Record one auth session event in `sys_audit_log`. Never throws — an audit - * write must never turn a valid sign-in into an error — and reports a lost - * row loudly (see {@link createAuthEventAuditSink}). + * Record one auth event in `sys_audit_log`. Never throws — an audit write + * must never change the answer a sign-in gets — and reports a lost row + * loudly (see {@link createAuthEventAuditSink}). */ recordAuthEvent(event: AuthSessionAuditEvent): Promise; } @@ -148,9 +206,17 @@ export interface AuthEventAuditSinkOptions { logger?: AuthEventAuditLogger; } -/** The object the auth-event rows name as their target. */ +/** The object a session event's row names as its target. */ const SESSION_OBJECT = 'sys_session'; +/** The object an account event's row names as its target (module note). */ +const ACCOUNT_OBJECT = 'sys_user'; + +/** The object the row for `action` is about. */ +function targetObjectFor(action: unknown): string { + return isAuthAccountAuditAction(action) ? ACCOUNT_OBJECT : SESSION_OBJECT; +} + /** JSON that cannot throw on a cyclic/odd value — same rule as the CRUD writer. */ function safeStringify(v: unknown): string { try { @@ -212,12 +278,13 @@ export function createAuthEventAuditSink(opts: AuthEventAuditSinkOptions): AuthE const reportAuthEventWriteFailure = (action: string, err: unknown): void => { const detail = String((err as any)?.message ?? err); try { - // The object dimension of the shared key is `sys_session` here — the - // object these rows are ABOUT (`object_name`), which is what - // `audit-writers.ts` passes too (`ctx.object`). It is constant on this - // seam, so the key reduces to the driver's code vocabulary: bounded by - // construction, and still the same key shape rather than a second one. - const cause = auditFailureCauseKey(SESSION_OBJECT, err); + // The object dimension of the shared key is the object these rows are + // ABOUT (`object_name`), which is what `audit-writers.ts` passes too + // (`ctx.object`). It takes two values on this seam (`sys_session` for a + // session event, `sys_user` for an account event), so the key reduces to + // the driver's code vocabulary twice over: bounded by construction, and + // still the same key shape rather than a second one. + const cause = auditFailureCauseKey(targetObjectFor(action), err); if (reportedAuthEventFailureCauses.has(cause)) { logger?.debug?.('Auth-event audit write failed (already reported)', { action, @@ -231,10 +298,16 @@ export function createAuthEventAuditSink(opts: AuthEventAuditSinkOptions): AuthE // question is asked about that one — unlike `persistAuditTrailRow`, which // writes the ledger row and its `sys_activity` mirror and asks about both. const missingTable = isMissingTableError(err, 'sys_audit_log'); + // What the caller saw is different for the two classes, and the line + // must not tell an operator a refused sign-in "succeeded". + const callerSaw = isAuthAccountAuditAction(action) + ? 'The sign-in attempt itself was answered exactly as it would have been without the ledger, so ' + + 'nothing downstream looks broken' + : 'The sign-in/sign-out itself SUCCEEDED and the user holds a valid session, so the API returned 200 ' + + 'and nothing downstream looks broken'; const message = `Auth-event audit write FAILED (${auditFailureCauseSummary(err, detail)}) — the compliance trail is ` + - 'now INCOMPLETE. The sign-in/sign-out itself SUCCEEDED and the user holds a valid session, so the API ' + - 'returned 200 and nothing downstream looks broken; only the `sys_audit_log` row recording the ' + + `now INCOMPLETE. ${callerSaw}; only the \`sys_audit_log\` row recording the ` + `${action} never landed, and nothing retries it. Every subsequent auth event failing THIS WAY is ` + 'losing its row the same way (this CAUSE is reported ONCE — raise the log level to `debug` to see ' + 'the rest; a DIFFERENT cause gets its own `error` line). The shipped `auth_events` list view and the ' + @@ -289,15 +362,18 @@ export function createAuthEventAuditSink(opts: AuthEventAuditSinkOptions): AuthE const objectHasField = probe; const tenantId = event.organizationId ?? null; + const accountEvent = isAuthAccountAuditAction(event.action); const row: Record = { action: event.action, // The whole point of the card: the event names its actor. `user_id` is // a strict `sys_user` lookup, and a session always has a real subject. + // On an account event it is the account the sign-in named. user_id: event.userId, - // The session this event is about, so the row is navigable from the - // ledger to the session row (and back) rather than being a bare verb. - object_name: SESSION_OBJECT, - record_id: event.sessionId ?? null, + // The record this event is about, so the row is navigable from the + // ledger to it (and back) rather than being a bare verb: the session + // for a session event, the account for an account event. + object_name: targetObjectFor(event.action), + record_id: accountEvent ? event.userId : (event.sessionId ?? null), // No before/after state: an auth event is not a field diff. Recording // `{}` here would be a claim about a record that never changed. old_value: null, @@ -315,8 +391,9 @@ export function createAuthEventAuditSink(opts: AuthEventAuditSinkOptions): AuthE // ADR-0014 D2's principal label. For an ordinary sign-in the subject IS // the principal; for an impersonation session the admin who started it // is, and recording the impersonated user as the sole principal there - // would be a WRONG record rather than a vague one. - row.actor = event.actor ?? event.userId; + // would be a WRONG record rather than a vague one. An account event + // authenticated nobody, so it credits no principal (module note). + row.actor = event.actor ?? (accountEvent ? null : event.userId); } try { diff --git a/packages/plugins/plugin-audit/src/index.ts b/packages/plugins/plugin-audit/src/index.ts index 51606773ee8..1a5c316a817 100644 --- a/packages/plugins/plugin-audit/src/index.ts +++ b/packages/plugins/plugin-audit/src/index.ts @@ -26,6 +26,8 @@ export type { ReadAuditWriterOptions, } from './read-audit.js'; export type { + AuthAccountAuditAction, + AuthEventAuditAction, AuthEventAuditLogger, AuthEventAuditSink, AuthEventAuditSinkOptions, diff --git a/packages/plugins/plugin-audit/src/objects/sys-audit-log-retired-actions.test.ts b/packages/plugins/plugin-audit/src/objects/sys-audit-log-retired-actions.test.ts index d5f87ae7883..67ac9194c8e 100644 --- a/packages/plugins/plugin-audit/src/objects/sys-audit-log-retired-actions.test.ts +++ b/packages/plugins/plugin-audit/src/objects/sys-audit-log-retired-actions.test.ts @@ -77,6 +77,16 @@ const ACTIONS_WITH_WRITERS: ReadonlyArray = {}) { + const engine = createMemoryEngine(); + const events: AuthSessionAuditEventInput[] = []; + const manager = new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + dataEngine: engine, + getAuditSink: () => ({ + recordAuthEvent: async (event: AuthSessionAuditEventInput) => { + events.push(event); + }, + }), + ...config, + } as any); + return { engine, manager, events }; +} + +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), + }), + ); + +async function signUp(manager: AuthManager): Promise { + inviteForAudienceGate(manager, EMAIL); + const res = await post(manager, '/sign-up/email', { email: EMAIL, password: PASSWORD, name: 'Holder' }); + expect(res.status, await res.clone().text()).toBe(200); +} + +const signIn = (manager: AuthManager, email: string, password: string) => + post(manager, '/sign-in/email', { email, password }); + +function userIdFor(engine: any): string { + const row = ((engine.tables.get('sys_user') ?? []) as any[]).find((r) => r.email === EMAIL); + if (!row) throw new Error(`no sys_user row for ${EMAIL}`); + return String(row.id); +} + +/** The account events, in order — `login` / `logout` filtered out. */ +const accountEvents = (events: AuthSessionAuditEventInput[]) => + events.filter((e) => e.action === 'login_failed' || e.action === 'account_locked'); + +/** No credential material anywhere in what the sink was handed. */ +function expectNoPasswordMaterial(events: AuthSessionAuditEventInput[]): void { + const serialized = JSON.stringify(events); + for (const fragment of [ + WRONG_PASSWORD, + PASSWORD, + WRONG_PASSWORD.slice(0, 8), + WRONG_PASSWORD.slice(-8), + PASSWORD.slice(0, 8), + PASSWORD.slice(-8), + ]) { + expect(serialized).not.toContain(fragment); + } + const allowed = ['action', 'userId', 'ipAddress', 'userAgent', 'context']; + for (const event of accountEvents(events)) { + expect( + Object.keys(event).filter((k) => !allowed.includes(k)), + 'an account event carries only the account, the outcome and the client fingerprint', + ).toEqual([]); + expect( + Object.keys(event.context ?? {}).filter((k) => !['endpoint', 'reason', 'locked_until', 'threshold'].includes(k)), + 'an account event context carries only the endpoint, the refusal code and the lock', + ).toEqual([]); + } +} + +describe('a refused password sign-in records one login_failed event for the account it named', () => { + it('names the account, the refusal code and the client, and carries no password material', async () => { + const { engine, manager, events } = deployment(); + await signUp(manager); + events.length = 0; + + const res = await signIn(manager, EMAIL, WRONG_PASSWORD); + expect(res.status).toBe(401); + + const recorded = accountEvents(events); + expect(recorded).toHaveLength(1); + expect(recorded[0]).toEqual({ + action: 'login_failed', + userId: userIdFor(engine), + ipAddress: CLIENT_IP, + userAgent: CLIENT_UA, + context: { endpoint: '/sign-in/email', reason: 'INVALID_EMAIL_OR_PASSWORD' }, + }); + expectNoPasswordMaterial(events); + }); + + it('its client fingerprint is the one a login event for the same client carries', async () => { + const { manager, events } = deployment(); + await signUp(manager); + events.length = 0; + + expect((await signIn(manager, EMAIL, WRONG_PASSWORD)).status).toBe(401); + expect((await signIn(manager, EMAIL, PASSWORD)).status).toBe(200); + + const failed = events.find((e) => e.action === 'login_failed'); + const login = events.find((e) => e.action === 'login'); + expect(failed, 'no login_failed event').toBeDefined(); + expect(login, 'no login event').toBeDefined(); + expect(failed!.ipAddress).toBe(login!.ipAddress); + expect(failed!.userAgent).toBe(login!.userAgent); + }); + + it('is recorded whether or not a lockout threshold is configured', async () => { + const { manager, events } = deployment({ lockoutThreshold: 0 }); + await signUp(manager); + events.length = 0; + + expect((await signIn(manager, EMAIL, WRONG_PASSWORD)).status).toBe(401); + expect(accountEvents(events).map((e) => e.action)).toEqual(['login_failed']); + }); + + it('names the account the credential check resolved, whatever the letter case of the address', async () => { + const { engine, manager, events } = deployment(); + await signUp(manager); + events.length = 0; + + expect((await signIn(manager, EMAIL.toUpperCase(), WRONG_PASSWORD)).status).toBe(401); + const recorded = accountEvents(events); + expect(recorded).toHaveLength(1); + expect(recorded[0]!.userId).toBe(userIdFor(engine)); + }); + + it('an identifier that resolves to no account records nothing, and is never handed to the sink', async () => { + const { manager, events } = deployment(); + await signUp(manager); + events.length = 0; + + const unknown = 'nobody.account-audit@example.com'; + expect((await signIn(manager, unknown, WRONG_PASSWORD)).status).toBe(401); + // An identifier that is not an address at all is refused before any account + // is looked up; it must not reach the ledger either. + expect((await signIn(manager, WRONG_PASSWORD, WRONG_PASSWORD)).status).toBe(400); + + expect(accountEvents(events)).toEqual([]); + expect(JSON.stringify(events)).not.toContain(unknown); + expectNoPasswordMaterial(events); + }); +}); + +describe('reaching the lockout threshold records one account_locked event', () => { + it('the refusal that stamps the lock records login_failed and then account_locked', async () => { + const { engine, manager, events } = deployment({ lockoutThreshold: 3, lockoutDurationMinutes: 15 }); + await signUp(manager); + events.length = 0; + + for (let i = 0; i < 3; i++) expect((await signIn(manager, EMAIL, WRONG_PASSWORD)).status).toBe(401); + + const recorded = accountEvents(events); + expect(recorded.map((e) => e.action)).toEqual([ + 'login_failed', + 'login_failed', + 'login_failed', + 'account_locked', + ]); + const locked = recorded[3]!; + expect(locked.userId).toBe(userIdFor(engine)); + expect(locked.ipAddress).toBe(CLIENT_IP); + expect(locked.context?.endpoint).toBe('/sign-in/email'); + expect(locked.context?.threshold).toBe(3); + const until = new Date(String(locked.context?.locked_until)).getTime(); + expect(until).toBeGreaterThan(Date.now() + 14 * 60_000); + // The lock the event reports is the lock the account holds. + const row = ((engine.tables.get('sys_user') ?? []) as any[]).find((r) => r.email === EMAIL); + expect(new Date(row.locked_until).getTime()).toBe(until); + expectNoPasswordMaterial(events); + }); + + it('a sign-in refused while locked records login_failed with the lock as its reason, and no second lock', async () => { + const { engine, manager, events } = deployment({ lockoutThreshold: 3 }); + await signUp(manager); + for (let i = 0; i < 3; i++) await signIn(manager, EMAIL, WRONG_PASSWORD); + events.length = 0; + + // Refused by the lock even with the right password. + const res = await signIn(manager, EMAIL, PASSWORD); + expect(res.status).toBe(403); + + expect(accountEvents(events)).toEqual([ + { + action: 'login_failed', + userId: userIdFor(engine), + ipAddress: CLIENT_IP, + userAgent: CLIENT_UA, + context: { endpoint: '/sign-in/email', reason: 'ACCOUNT_LOCKED' }, + }, + ]); + expect(events.some((e) => e.action === 'login')).toBe(false); + expectNoPasswordMaterial(events); + }); + + it('refusals below the threshold record no account_locked', async () => { + const { manager, events } = deployment({ lockoutThreshold: 3 }); + await signUp(manager); + events.length = 0; + + for (let i = 0; i < 2; i++) await signIn(manager, EMAIL, WRONG_PASSWORD); + expect(accountEvents(events).map((e) => e.action)).toEqual(['login_failed', 'login_failed']); + }); +}); + +describe('a successful sign-in is recorded as before', () => { + it('records login and no account event', async () => { + const { engine, manager, events } = deployment({ lockoutThreshold: 3 }); + await signUp(manager); + events.length = 0; + + expect((await signIn(manager, EMAIL, PASSWORD)).status).toBe(200); + + expect(accountEvents(events)).toEqual([]); + const logins = events.filter((e) => e.action === 'login'); + expect(logins).toHaveLength(1); + expect(logins[0]!.userId).toBe(userIdFor(engine)); + expect(logins[0]!.context).toEqual({ endpoint: '/sign-in/email' }); + expectNoPasswordMaterial(events); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index fc295fb6679..14f9f8ab273 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -79,9 +79,14 @@ import { permissionSetInEffect } from './catalog-set-in-effect.js'; import { recoverInternalFieldsForSystemRead } from './internal-field-readback.js'; import { runWithAuthActorScope, setAuthActorResolver } from './auth-actor-attribution.js'; import { + accountLockedEventFor, emitAuthSessionAuditEvent, loginEventFor, logoutEventFor, + refusalCodeOf, + signInClientFingerprint, + signInFailureEventFor, + SIGN_IN_EMAIL_PATH, type AuthEventAuditSurface, } from './auth-session-audit.js'; import { SESSION_ERASURE_PATHS } from './session-tombstone.js'; @@ -2733,9 +2738,11 @@ export class AuthManager { // ── ADR-0069 D2: account lockout (gate) ───────────────────── // Reject a sign-in for a locked identity BEFORE better-auth checks // the password — a lock must hold even against the correct password. - if (ctx?.path === '/sign-in/email') { + // The refusal is a refused sign-in, so it writes its `login_failed` + // row (`auth-session-audit.ts`, account events) before it throws. + if (ctx?.path === SIGN_IN_EMAIL_PATH) { const email = typeof ctx?.body?.email === 'string' ? ctx.body.email : ''; - if (email) await this.assertAccountNotLocked(email); + if (email) await this.assertAccountNotLocked(email, ctx); return; } @@ -2823,7 +2830,7 @@ export class AuthManager { // better-auth catches an INVALID_EMAIL_OR_PASSWORD APIError and runs // the after-hook with it on `ctx.context.returned`; a success leaves // the session payload there. Count failures, reset on success. - if (ctx?.path === '/sign-in/email') { + if (ctx?.path === SIGN_IN_EMAIL_PATH) { const email = typeof ctx?.body?.email === 'string' ? ctx.body.email : ''; if (email) { let succeeded = true; @@ -2833,7 +2840,11 @@ export class AuthManager { } catch { succeeded = !(ctx?.context?.returned instanceof Error); } - await this.recordSignInOutcome(email, succeeded); + const lockStamped = await this.recordSignInOutcome(email, succeeded); + // A refused sign-in writes `login_failed`, and the one that + // stamped the lockout also writes `account_locked`. A successful + // one is recorded as `login` by the session hook, unchanged. + if (!succeeded) await this.auditRefusedSignIn(ctx, email, lockStamped); if (succeeded) { const uid = ctx?.context?.returned?.user?.id; if (typeof uid === 'string') await this.enforceConcurrentCap(uid); @@ -9213,13 +9224,18 @@ export class AuthManager { * out (brute-force protection). No-op when lockout is disabled * (`lockoutThreshold <= 0`) or no data engine is wired. Fails OPEN on a * lookup error: an infra hiccup must never block every login. + * + * The refusal is a refused sign-in of the account it names, so — given the + * endpoint context `ctx` — it writes that account's `login_failed` row with + * the refusal's own code before throwing (`auth-session-audit.ts`). */ - private async assertAccountNotLocked(email: string): Promise { + private async assertAccountNotLocked(email: string, ctx?: unknown): Promise { const threshold = Number(this.config.lockoutThreshold) || 0; if (threshold <= 0) return; const engine = this.getDataEngine(); if (!engine) return; let locked = false; + let lockedUserId: string | undefined; try { const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] }; const u = await engine.findOne('sys_user', { @@ -9227,10 +9243,21 @@ export class AuthManager { } as any); const lu = u?.locked_until; locked = !!(lu && new Date(lu).getTime() > Date.now()); + if (locked && typeof u?.id === 'string' && u.id) lockedUserId = u.id; } catch { return; // fail-open } if (locked) { + if (ctx !== undefined && lockedUserId) { + await emitAuthSessionAuditEvent( + this.config.getAuditSink?.(), + signInFailureEventFor(lockedUserId, { + path: SIGN_IN_EMAIL_PATH, + reason: 'ACCOUNT_LOCKED', + fingerprint: signInClientFingerprint(ctx), + }), + ); + } const { APIError } = await import('better-auth/api'); throw new APIError('FORBIDDEN', { message: @@ -9248,12 +9275,21 @@ export class AuthManager { * both (only writing when there is something to clear, to avoid a no-op * history row on every login). No-op when lockout is disabled. Never throws — * a counter write must not turn a valid login into an error. + * + * Resolves to the lock it stamped — the account, the expiry and the + * threshold — when THIS call wrote `locked_until` and the write landed, and + * to `undefined` otherwise. That is the lockout transition the + * `account_locked` row records, so a lock that was never written is never + * reported as one. */ - private async recordSignInOutcome(email: string, success: boolean): Promise { + private async recordSignInOutcome( + email: string, + success: boolean, + ): Promise<{ userId: string; lockedUntil: Date; threshold: number } | undefined> { const threshold = Number(this.config.lockoutThreshold) || 0; - if (threshold <= 0) return; + if (threshold <= 0) return undefined; const engine = this.getDataEngine(); - if (!engine) return; + if (!engine) return undefined; try { const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] }; const u = await engine.findOne('sys_user', { @@ -9261,7 +9297,7 @@ export class AuthManager { fields: ['id', 'failed_login_count', 'locked_until'], context: SYSTEM_CTX, } as any); - if (!u?.id) return; + if (!u?.id) return undefined; if (success) { if ((Number(u.failed_login_count) || 0) !== 0 || u.locked_until) { await engine.update( @@ -9270,15 +9306,20 @@ export class AuthManager { { context: SYSTEM_CTX } as any, ); } - return; + return undefined; } const next = (Number(u.failed_login_count) || 0) + 1; const patch: Record = { id: u.id, failed_login_count: next }; + let lockedUntil: Date | undefined; if (next >= threshold) { const mins = Number(this.config.lockoutDurationMinutes) || 15; - patch.locked_until = new Date(Date.now() + mins * 60_000); + lockedUntil = new Date(Date.now() + mins * 60_000); + patch.locked_until = lockedUntil; } await engine.update('sys_user', patch, { context: SYSTEM_CTX } as any); + return lockedUntil && typeof u.id === 'string' + ? { userId: u.id, lockedUntil, threshold } + : undefined; } catch (e) { // Never break the auth response. [#12981] But lockout accounting that // silently stops accounting is brute-force protection that is OFF. @@ -9293,6 +9334,81 @@ export class AuthManager { e, { object: 'sys_user', threshold, success }, ); + return undefined; + } + } + + /** + * Write the account events for a refused `/sign-in/email`: one + * `login_failed` row for the account the sign-in named, carrying the + * refusal's machine code, and — when this attempt stamped the ADR-0069 D2 + * lockout (`lockStamped`, from {@link recordSignInOutcome}) — one + * `account_locked` row. Independent of the lockout configuration: a refused + * sign-in is recorded whether or not a threshold is set. + * + * ⛔ The submitted identifier is used only to FIND the account and is never + * recorded; an identifier that resolves to no account writes nothing + * (`auth-session-audit.ts`, account events). The password is never read. + * Never throws — the refusal is already decided and the sink swallows. + */ + private async auditRefusedSignIn( + ctx: any, + email: string, + lockStamped: { userId: string; lockedUntil: Date; threshold: number } | undefined, + ): Promise { + const sink = this.config.getAuditSink?.(); + if (!sink) return; + const userId = lockStamped?.userId ?? (await this.resolveSignInAccountId(email)); + if (!userId) return; + const fingerprint = signInClientFingerprint(ctx); + await emitAuthSessionAuditEvent( + sink, + signInFailureEventFor(userId, { + path: SIGN_IN_EMAIL_PATH, + reason: refusalCodeOf(ctx?.context?.returned), + fingerprint, + }), + ); + if (lockStamped) { + await emitAuthSessionAuditEvent( + sink, + accountLockedEventFor(lockStamped.userId, { + path: SIGN_IN_EMAIL_PATH, + lockedUntil: lockStamped.lockedUntil, + threshold: lockStamped.threshold, + fingerprint, + }), + ); + } + } + + /** + * The id of the account a password sign-in for `email` is checked against, + * or `undefined`. Resolved the way better-auth's `/sign-in/email` resolves + * it — the address lower-cased (`findUserByEmail(email.toLowerCase())`), + * matching the lower-cased identifier the adapter stores — so the + * `login_failed` row names the account the credential check actually judged. + */ + private async resolveSignInAccountId(email: string): Promise { + const engine = this.getDataEngine(); + if (!engine) return undefined; + try { + const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] }; + const u = await engine.findOne('sys_user', { + where: { email: email.toLowerCase() }, fields: ['id'], context: SYSTEM_CTX, + } as any); + return typeof u?.id === 'string' && u.id ? u.id : undefined; + } catch (e) { + // The refusal stands either way; what is lost is its ledger row. + this.logDurabilityDegradation( + 'AuthManager: a refused sign-in was NOT recorded in sys_audit_log — the account it named ' + + 'could not be resolved, so its `login_failed` row was not written. The refusal itself was ' + + 'answered normally, so nothing looks broken, and the Auth preset of Setup → Audit Log is ' + + 'missing this attempt. Remedy: make the sys_user read land — check driver connectivity.', + e, + { object: 'sys_user', action: 'login_failed' }, + ); + return undefined; } } diff --git a/packages/plugins/plugin-auth/src/auth-session-audit.ts b/packages/plugins/plugin-auth/src/auth-session-audit.ts index c843c4a6a23..822a497ed96 100644 --- a/packages/plugins/plugin-auth/src/auth-session-audit.ts +++ b/packages/plugins/plugin-auth/src/auth-session-audit.ts @@ -54,8 +54,34 @@ * it never could. The captured argument is the same value, taken at a moment it * is guaranteed present. No context at all ⇒ the cause is unknown ⇒ no row: * losing the record is the safe direction, inventing one is not. + * + * ## Account events: a refused sign-in, and a lockout + * + * A session hook cannot see a sign-in that did not succeed — no session row is + * ever minted for it. So the password sign-in's own outcome, observed in + * `AuthManager`'s `/sign-in/email` middleware where the ADR-0069 D2 lockout is + * enforced, is the seam for the two account events: + * + * - **`login_failed`** — one per refused `/sign-in/email` that names an + * existing account: refused by the credential check (the after-hook's + * returned error) or by the lockout gate before it (the before-hook). + * - **`account_locked`** — one each time the lockout is STAMPED on the + * account, i.e. the refused attempt that reaches the threshold. + * + * Both name the account the sign-in resolved to, the outcome (the action plus + * the refusal's machine code), and the client fingerprint read by the same rule + * better-auth applies when it stamps a session row — so a refused attempt and a + * successful one from the same client carry the same `ip_address`. + * + * ⛔ Neither event carries anything the caller typed. The password is never + * read here, and the submitted identifier is never recorded either: the row + * names the RESOLVED account's id, and an identifier that resolves to no + * account produces no row (an auth row without a subject is the shape this + * module already refuses to write, above). */ +import { getIP } from '@better-auth/core/utils/ip'; + /** * The audit sink's shape, declared LOCALLY so plugin-auth takes no runtime * dependency on plugin-audit — the mirror of `MessagingEmitSurface` / @@ -72,7 +98,7 @@ */ export interface AuthEventAuditSurface { recordAuthEvent(event: { - action: 'login' | 'logout'; + action: 'login' | 'logout' | 'login_failed' | 'account_locked'; userId: string; sessionId?: string; organizationId?: string; @@ -106,6 +132,9 @@ interface BetterAuthSessionRow { /** The endpoint whose whole job is ending the caller's own session. */ export const SIGN_OUT_PATH = '/sign-out'; +/** The password sign-in endpoint — the one the ADR-0069 D2 lockout governs. */ +export const SIGN_IN_EMAIL_PATH = '/sign-in/email'; + /** A non-empty string, or undefined — never `''`, never `String(undefined)`. */ function str(v: unknown): string | undefined { return typeof v === 'string' && v.length > 0 ? v : undefined; @@ -169,11 +198,107 @@ export function logoutEventFor( }; } +/** The client fingerprint an account event carries. */ +export interface SignInClientFingerprint { + ipAddress?: string; + userAgent?: string; +} + +/** + * Read the client fingerprint of a sign-in request by better-auth's OWN rule — + * the one `internalAdapter.createSession` applies when it stamps a session row + * (`getIP(headers, options)` over the endpoint's headers, and the + * `user-agent` header), which is where every `login` row's `ip_address` comes + * from. A refused sign-in mints no session row to copy them from, so the same + * resolution is applied to the same request instead. ⛔ Not a second IP rule: + * whatever better-auth's `advanced.ipAddress` configuration answers for a + * session row, it answers here. + */ +export function signInClientFingerprint(ctx: unknown): SignInClientFingerprint { + const c = ctx as { headers?: unknown; request?: { headers?: unknown }; context?: { options?: unknown } } | null; + const headers = (c?.headers ?? c?.request?.headers) as Headers | undefined; + if (!headers || typeof (headers as Headers).get !== 'function') return {}; + let ipAddress: string | undefined; + try { + const options = c?.context?.options as Parameters[1] | undefined; + ipAddress = options ? str(getIP(headers, options)) : undefined; + } catch { + ipAddress = undefined; + } + const userAgent = str(headers.get('user-agent')); + return { + ...(ipAddress ? { ipAddress } : {}), + ...(userAgent ? { userAgent } : {}), + }; +} + +/** A machine error code: SCREAMING_SNAKE, bounded — never free text. */ +const REFUSAL_CODE = /^[A-Z][A-Z0-9_]{0,63}$/; + +/** + * The refusal's machine code (`INVALID_EMAIL_OR_PASSWORD`, `ACCOUNT_LOCKED`, …) + * off better-auth's returned `APIError`, or undefined. Only a value shaped as + * an error code is taken, so nothing but a constant from the refusing + * endpoint's vocabulary can reach the row. + */ +export function refusalCodeOf(returned: unknown): string | undefined { + const body = (returned as { body?: { code?: unknown } } | null | undefined)?.body; + const code = typeof body?.code === 'string' ? body.code : undefined; + return code && REFUSAL_CODE.test(code) ? code : undefined; +} + +/** + * The `login_failed` event for a refused password sign-in, or `null` when no + * account was resolved (no subject ⇒ no row; see the module note). + */ +export function signInFailureEventFor( + userId: string | null | undefined, + outcome: { path: string; reason?: string; fingerprint?: SignInClientFingerprint }, +): AuthSessionAuditEventInput | null { + const subject = str(userId); + if (!subject) return null; + const context: Record = { endpoint: outcome.path }; + if (outcome.reason) context.reason = outcome.reason; + return { + action: 'login_failed', + userId: subject, + ...(outcome.fingerprint ?? {}), + context, + }; +} + +/** + * The `account_locked` event for the refused sign-in that stamped the ADR-0069 + * D2 lockout, or `null` when no account was resolved. + */ +export function accountLockedEventFor( + userId: string | null | undefined, + lock: { path: string; lockedUntil: Date; threshold: number; fingerprint?: SignInClientFingerprint }, +): AuthSessionAuditEventInput | null { + const subject = str(userId); + if (!subject) return null; + const until = lock.lockedUntil instanceof Date && !Number.isNaN(lock.lockedUntil.getTime()) + ? lock.lockedUntil.toISOString() + : undefined; + return { + action: 'account_locked', + userId: subject, + ...(lock.fingerprint ?? {}), + context: { + endpoint: lock.path, + ...(until ? { locked_until: until } : {}), + threshold: lock.threshold, + }, + }; +} + /** * Hand one event to the sink, swallowing everything. * - * Auth must never fail on bookkeeping: the sign-in has already happened, the - * session is on disk, and the user is holding a valid token. The sink itself + * Auth must never fail on bookkeeping: for a session event the sign-in has + * already happened, the session is on disk, and the user is holding a valid + * token; for an account event the refusal is already decided, and the audit + * row must not change it. The sink itself * reports a lost row at `error` (once per process) — that is where the * durability report belongs, and duplicating it here would double every line. */ From 90714662bfe1a037a34c732c77a3655abc392c77 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 05:10:09 +0000 Subject: [PATCH 2/5] docs(plugin-audit): the audit actions list and the audit service page name the account events; changeset Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r Co-authored-by: Claude --- .changeset/audit-auth-account-events.md | 14 ++++++ .../kernel/runtime-services/audit-service.mdx | 47 +++++++++++++------ packages/plugins/plugin-audit/README.md | 29 +++++++++--- 3 files changed, 69 insertions(+), 21 deletions(-) create mode 100644 .changeset/audit-auth-account-events.md diff --git a/.changeset/audit-auth-account-events.md b/.changeset/audit-auth-account-events.md new file mode 100644 index 00000000000..96c8e81c936 --- /dev/null +++ b/.changeset/audit-auth-account-events.md @@ -0,0 +1,14 @@ +--- +'@objectstack/plugin-audit': minor +'@objectstack/plugin-auth': patch +--- + +fix(plugin-audit,plugin-auth): a refused sign-in and an account lockout each write a `sys_audit_log` row, as a successful sign-in already does + +Clause-②: no + +- **Two new ledger actions, each with its writer.** `sys_audit_log.action` gains `login_failed` and `account_locked`. A refused `/sign-in/email` that names an existing account writes one `login_failed` row. The refused sign-in that stamps the account lockout (`lockoutThreshold`, ADR-0069 D2) also writes one `account_locked` row. A sign-in refused while the account is locked writes `login_failed` with the reason `ACCOUNT_LOCKED`. A successful sign-in still writes `login`, unchanged. +- **What each row records.** `user_id` is the account the sign-in named, and the row names `sys_user` and that account's id as its target. The outcome is the action, plus the refusal's machine code in `metadata` (`INVALID_EMAIL_OR_PASSWORD`, `ACCOUNT_LOCKED`, …), and the lock's `locked_until` and `threshold` on an `account_locked` row. `created_at` is the time. `ip_address` and `user_agent` are resolved by the rule better-auth applies to a session row, which is where a `login` row's values come from. `actor` is null, because a refused attempt authenticated nobody. A row never carries the attempted credential, and an identifier that resolves to no account writes no row. +- **Where they show.** Setup → Audit Log's *Auth* preset (the `auth_events` list view) now lists `login`, `logout`, `login_failed` and `account_locked`. Each row is about the account it names, so the ledger's record read gate judges it by that `sys_user` record, as for any row about a user. It carries no `tenant_id`: a refused attempt has no session to take an organization from, so it is a deployment-level row under the ledger's organization scope. +- **Recorded with or without a lockout.** A refused sign-in writes its row whether or not `lockoutThreshold` is set. +- **`@objectstack/plugin-audit` types.** `AuthSessionAuditEvent.action` widens to `AuthEventAuditAction` (`'login' | 'logout' | 'login_failed' | 'account_locked'`). `AuthEventAuditAction` and `AuthAccountAuditAction` are new exported types, and `AuthSessionAuditAction` is unchanged. diff --git a/content/docs/kernel/runtime-services/audit-service.mdx b/content/docs/kernel/runtime-services/audit-service.mdx index 6d294a5139b..91bd3f8eb96 100644 --- a/content/docs/kernel/runtime-services/audit-service.mdx +++ b/content/docs/kernel/runtime-services/audit-service.mdx @@ -1,7 +1,7 @@ --- title: services.audit — the audit log service API navTitle: services.audit -description: Write ingress for audit events that are not CRUD — today, auth session events. +description: Write ingress for audit events that are not CRUD — today, auth session events and sign-in outcome events on an account. --- - **Stability:** `experimental` @@ -10,7 +10,8 @@ description: Write ingress for audit events that are not CRUD — today, auth se `services.audit` is the kernel service slot that `@objectstack/plugin-audit` registers during `init()`. It is the **write ingress for audit events that are not CRUD** — today, -auth session events — and every call appends one row to `sys_audit_log`. +auth session events and sign-in outcome events on an account — and every call appends one +row to `sys_audit_log`. Record-level `create` / `update` / `delete` rows do **not** travel through this slot. The same plugin writes those from ObjectQL lifecycle hooks, with no service call involved, so @@ -27,7 +28,7 @@ The slot's entire surface is one method: ```ts services.audit.recordAuthEvent(event: { - action: 'login' | 'logout'; + action: 'login' | 'logout' | 'login_failed' | 'account_locked'; userId: string; sessionId?: string; organizationId?: string; @@ -43,18 +44,32 @@ services.audit.recordAuthEvent(event: { validates nothing in either direction — a misspelled action would be accepted silently. This union is the only structural protection an author gets, which is why the caller hands over an **event** (what happened) rather than a row (what to store). + - `login` / `logout` are **session events**: a sign-in that minted a session, and a + sign-out. + - `login_failed` / `account_locked` are **account events**: a refused password sign-in + that named an existing account, and the refused sign-in that stamped the account + lockout (`lockoutThreshold`, ADR-0069 D2). They exist without any session. - **`userId`** is required and must be a real `sys_user` id — it lands on the strict - `user_id` lookup. -- **`sessionId`** lands on `record_id`, with `object_name` fixed to `sys_session`, so the - ledger row is navigable to the session it describes. + `user_id` lookup. On an account event it is the account the sign-in named. +- **`sessionId`** lands on `record_id`, with `object_name` set to `sys_session`, so the + ledger row is navigable to the session it describes. An account event names its account + instead — `object_name` `sys_user` and `record_id` the account's id — and does not read + `sessionId`. - **`organizationId`** is the session's active organization. It stamps the row's tenant columns, and an unstamped row is one that non-administrator members can never see through the security layer's predicate. +- **`ipAddress`** / **`userAgent`** are the client fingerprint by better-auth's own rule: + the session row's values for a session event, and the same resolution applied to the + sign-in request for an account event. - **`actor`** is the principal that *caused* the event when that is not the subject — an administrator's user id on an impersonation session. Omitted, the subject is recorded as - having acted for themselves. + having acted for themselves on a session event; an account event records no principal, + because a refused attempt authenticated nobody. - **`context`** is free-form and is serialized into `metadata`, e.g. the endpoint that - produced the event (`/sign-in/email`, `/sign-out`). + produced the event (`/sign-in/email`, `/sign-out`). On an account event it is the + endpoint, the refusal's machine code (`reason`, e.g. `INVALID_EMAIL_OR_PASSWORD` or + `ACCOUNT_LOCKED`) and, for `account_locked`, the lock's `locked_until` and `threshold`. + It never carries the attempted credential. ## Notes @@ -64,8 +79,9 @@ services.audit.recordAuthEvent(event: { `kernel:ready`. The sink therefore resolves its engine per call, so it is safe to hold a reference from the moment the plugin initializes. - `@objectstack/plugin-auth` is the only in-repo consumer. It resolves the slot lazily - through a locally-declared structural surface and calls it from the session lifecycle, - so neither package depends on the other. + through a locally-declared structural surface and calls it from the session lifecycle + and from the password sign-in outcome, so neither package depends on the other. A + sign-in identifier that resolves to no account produces no call. ## Typical Errors @@ -76,11 +92,12 @@ caller's side: - A failed ledger insert is reported at `error` level **once per cause** — a cause is the error's `code` plus the object the row is about, so every uncoded failure collapses into - one bucket and the object is always `sys_session` on this seam — and the row is simply - lost. Nothing retries it. A repeat of an **already-reported** cause drops to `debug`; a - **new** cause gets its own `error` line, once. The sign-in itself succeeded and returned - 200, so the shipped `auth_events` list view and the system-overview widgets keep showing - an empty, healthy-looking screen. + one bucket per object, and the object is `sys_session` for a session event and + `sys_user` for an account event — and the row is simply lost. Nothing retries it. A + repeat of an **already-reported** cause drops to `debug`; a **new** cause gets its own + `error` line, once. The sign-in was answered exactly as it would have been without the + ledger, so the shipped `auth_events` list view and the system-overview widgets keep + showing an empty, healthy-looking screen. - The call **silently no-ops** when no data engine resolves, or when `userId` is absent. A caller that passes an event without a subject records nothing and is told nothing. - The most common cause of a failing insert is a datasource split rather than a broken diff --git a/packages/plugins/plugin-audit/README.md b/packages/plugins/plugin-audit/README.md index 6368087805a..5acc68b6089 100644 --- a/packages/plugins/plugin-audit/README.md +++ b/packages/plugins/plugin-audit/README.md @@ -108,12 +108,25 @@ these are the only values that are ever written: | `delete` | `installAuditWriters` (this package) | `afterDelete` | | `login` | `createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth` | session start | | `logout` | `createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth` | session end | +| `login_failed` | `createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth` | a refused password sign-in (`/sign-in/email`) that names an existing account | +| `account_locked` | `createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth` | the refused sign-in that stamps the account lockout (`lockoutThreshold`, ADR-0069 D2) | | `config_change` | `@objectstack/service-settings` | every successful settings write | | `import` | `@objectstack/plugin-auth` admin user import | one run-level row, with `record_id: null` | +| `platform_admin_standing_change` | `@objectstack/plugin-security` | a change of who holds platform-administrator standing, plus a first-boot baseline | Action values are snake_case. There is no namespaced `domain:action` spelling — the enum above is the complete accepted set, and a value outside it is not a form this object takes. +**Session events and account events.** A `login` / `logout` row is about a session: it names +`sys_session` and the session id, and `actor` is the subject (or the administrator who +started an impersonation session). A `login_failed` / `account_locked` row is about an +account and exists without any session: `user_id` is the account the sign-in named, the row +names `sys_user` and that account's id, and `actor` is null, because a refused attempt +authenticated nobody. Its `metadata` carries the endpoint and the refusal's machine code +(`INVALID_EMAIL_OR_PASSWORD`, `ACCOUNT_LOCKED`, …), plus the lock's expiry and threshold on +an `account_locked` row. It never carries the attempted credential, and an identifier that +resolves to no account writes no row. + ## `sys_audit_log` fields These are the columns the object declares. Every field is `readonly: true`; rows are @@ -123,7 +136,7 @@ written only by internal system hooks running under `sudo()`, never through UI f |---|---|---| | `id` | text | Audit log entry id | | `created_at` | datetime | When the action occurred (`NOW()` default) | -| `action` | select | One of the eight values above | +| `action` | select | One of the eleven values above | | `user_id` | lookup → `sys_user` | Null for non-user / service actions | | `actor` | text | Principal label: a user id, `svc:`, or null. Attributes service-token writes that `user_id` structurally cannot hold | | `object_name` | text | Target object, e.g. `sys_user` | @@ -144,7 +157,9 @@ without the secret itself reaching the ledger. **`ip_address` / `user_agent` are populated on auth events only.** Neither the record-level writer nor the record-view writer stamps them: a `create` / `update` / `delete` / `read` row records who and what, not from where. Do not read a null client -fingerprint on such a row as "the request had none". +fingerprint on such a row as "the request had none". On every auth event they are +resolved by better-auth's own rule: a session event copies the session row's values, and +an account event applies the same resolution to the sign-in request. **`old_value` / `new_value` are null on every `read` row**, deliberately and not as an omission — see [Record-view auditing](#record-view-auditing--the-read-action). @@ -336,7 +351,7 @@ the standard object API or `services.data`, and through the shipped list views: |---|---| | `recent` | Everything, newest first | | `writes_only` | `create` / `update` / `delete` | -| `auth_events` | `login` / `logout` | +| `auth_events` | `login` / `logout` / `login_failed` / `account_locked` | | `record_views` | `read` — who opened which record | | `config_changes` | `config_change` / `import` | | `all_events` | Everything, larger page size | @@ -355,9 +370,10 @@ interface AuthEventAuditSink { } ``` -`AuthSessionAuditEvent.action` is the closed union `'login' | 'logout'`. `@objectstack/plugin-auth` -resolves this slot lazily and calls it from the session lifecycle. It is not a query API: -to read audit history, query the `sys_audit_log` object. +`AuthSessionAuditEvent.action` is the closed union `'login' | 'logout' | 'login_failed' | +'account_locked'` (`AuthEventAuditAction`). `@objectstack/plugin-auth` resolves this slot +lazily and calls it from the session lifecycle and from the password sign-in outcome. It is +not a query API: to read audit history, query the `sys_audit_log` object. ## Retention and archival @@ -430,6 +446,7 @@ export type { // Auth-event ingress (the `audit` service slot) export { createAuthEventAuditSink }; export type { + AuthAccountAuditAction, AuthEventAuditAction, AuthEventAuditLogger, AuthEventAuditSink, AuthEventAuditSinkOptions, AuthSessionAuditAction, AuthSessionAuditEvent, }; From 017d793b208d02b8dbadbd16eb13c8f0f6ab7988 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 05:27:45 +0000 Subject: [PATCH 3/5] chore(plugin-audit): translation bundles carry the two new audit action options Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r Co-authored-by: Claude --- .../plugin-audit/src/translations/en.objects.generated.ts | 2 ++ .../plugin-audit/src/translations/es-ES.objects.generated.ts | 2 ++ .../plugin-audit/src/translations/ja-JP.objects.generated.ts | 2 ++ .../plugin-audit/src/translations/zh-CN.objects.generated.ts | 2 ++ 4 files changed, 8 insertions(+) diff --git a/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts index d8f38e3c76e..cae72a9412b 100644 --- a/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/en.objects.generated.ts @@ -33,6 +33,8 @@ export const enObjects: NonNullable = { delete: "delete", login: "login", logout: "logout", + login_failed: "login_failed", + account_locked: "account_locked", config_change: "config_change", import: "import", platform_admin_standing_change: "platform_admin_standing_change" diff --git a/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts index 5e2e4ad6224..6ab813f503d 100644 --- a/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/es-ES.objects.generated.ts @@ -33,6 +33,8 @@ export const esESObjects: NonNullable = { delete: "Eliminar", login: "Inicio de sesión", logout: "Cierre de sesión", + login_failed: "Inicio de sesión fallido", + account_locked: "Cuenta bloqueada", config_change: "Cambio de configuración", import: "Importar", platform_admin_standing_change: "Cambio de acceso de administrador de plataforma" diff --git a/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts index a768237a380..93d77b58f35 100644 --- a/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/ja-JP.objects.generated.ts @@ -33,6 +33,8 @@ export const jaJPObjects: NonNullable = { delete: "削除", login: "ログイン", logout: "ログアウト", + login_failed: "ログイン失敗", + account_locked: "アカウントロック", config_change: "構成変更", import: "インポート", platform_admin_standing_change: "プラットフォーム管理者権限の変更" diff --git a/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts b/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts index bf5f0be8b88..da09d32f932 100644 --- a/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts +++ b/packages/plugins/plugin-audit/src/translations/zh-CN.objects.generated.ts @@ -33,6 +33,8 @@ export const zhCNObjects: NonNullable = { delete: "删除", login: "登录", logout: "登出", + login_failed: "登录失败", + account_locked: "账户锁定", config_change: "配置变更", import: "导入", platform_admin_standing_change: "平台管理员权限变更" From 6eb4efa2864229494110beeb2cc01349ff341b6d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 05:38:54 +0000 Subject: [PATCH 4/5] test(plugin-auth): name the account-resolution case for its invariant Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r Co-authored-by: Claude --- .../plugins/plugin-auth/src/auth-account-event-audit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugins/plugin-auth/src/auth-account-event-audit.test.ts b/packages/plugins/plugin-auth/src/auth-account-event-audit.test.ts index 96db1fa0dc6..27feed41cc3 100644 --- a/packages/plugins/plugin-auth/src/auth-account-event-audit.test.ts +++ b/packages/plugins/plugin-auth/src/auth-account-event-audit.test.ts @@ -160,7 +160,7 @@ describe('a refused password sign-in records one login_failed event for the acco expect(accountEvents(events).map((e) => e.action)).toEqual(['login_failed']); }); - it('names the account the credential check resolved, whatever the letter case of the address', async () => { + it('names the account the credential check resolved the address to', async () => { const { engine, manager, events } = deployment(); await signUp(manager); events.length = 0; From 789c68ff9a26f98dead6f0914fa90331f41008b7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 20 Oct 2026 06:26:58 +0000 Subject: [PATCH 5/5] fix(plugin-auth): the account lookup for a refused sign-in carries the declared engine query type Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r Co-authored-by: Claude --- packages/plugins/plugin-auth/src/auth-manager.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 14f9f8ab273..637297d1fe1 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -53,6 +53,7 @@ import { } from '@objectstack/core'; import type { AuthGate } from '@objectstack/core'; import type { ExecutionContext } from '@objectstack/spec/kernel'; +import type { EngineQueryOptions } from '@objectstack/spec/data'; import type { ApiKeyLifecyclePolicy, IEmailService, @@ -9393,10 +9394,12 @@ export class AuthManager { const engine = this.getDataEngine(); if (!engine) return undefined; try { - const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] }; - const u = await engine.findOne('sys_user', { - where: { email: email.toLowerCase() }, fields: ['id'], context: SYSTEM_CTX, - } as any); + const query: EngineQueryOptions = { + where: { email: email.toLowerCase() }, + fields: ['id'], + context: { isSystem: true, positions: [], permissions: [] }, + }; + const u = await engine.findOne('sys_user', query); return typeof u?.id === 'string' && u.id ? u.id : undefined; } catch (e) { // The refusal stands either way; what is lost is its ledger row.