You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 3465ad4
Browse filesBrowse the repository at this point in the historyBrowse files
fix(plugin-audit,plugin-auth): a refused sign-in and an account lockout each write an audit row (#24797)
Security fix (maintainer-ordered; no public card; detail withheld under
the disclosure ruling).
Clause-②: no
## The invariant this restores
A refused sign-in and an account lockout each write a `sys_audit_log`
row, as a successful sign-in already writes `login`. Each row records
the account, the outcome, the time and the client IP. The rows show
under Setup → Audit Log's *Auth* preset, and `sys_audit_log.action`
declares their actions.
## What changed
**`@objectstack/plugin-audit`**
- `sys_audit_log.action` gains `login_failed` and `account_locked`,
writer-first. The `auth_events` list view (the *Auth* preset) now
filters on `login`, `logout`, `login_failed` and `account_locked`. The
writer inventory in `sys-audit-log-retired-actions.test.ts` names their
writer.
- The `audit` service sink (`createAuthEventAuditSink`) takes two event
classes:
- **Session events** (`login`, `logout`) are unchanged. They name
`sys_session` and the session id, and `actor` is the subject.
- **Account events** (`login_failed`, `account_locked`) name the
account: `user_id` is the account, and the target is `sys_user` and its
id. `actor` is null, because a refused attempt authenticated nobody. The
ledger's record read gate judges these rows by that `sys_user` record.
The lost-row report no longer says a refused sign-in succeeded.
- New exported types: `AuthEventAuditAction` and
`AuthAccountAuditAction`. `AuthSessionAuditEvent.action` widens to
`AuthEventAuditAction`.
- Translation bundles carry the two new options. The README action and
view tables and `content/docs/kernel/runtime-services/audit-service.mdx`
describe the account events. The README table also gains the
`platform_admin_standing_change` row it was missing, so its value count
is correct.
**`@objectstack/plugin-auth`**, at the `/sign-in/email` lockout seam
(ADR-0069 D2):
- A refused sign-in that names an existing account emits `login_failed`.
Its `reason` is the refusal's machine code (`INVALID_EMAIL_OR_PASSWORD`,
…), taken only when it is shaped as an error code. This happens whether
or not `lockoutThreshold` is set.
- The refused sign-in that stamps the lock also emits `account_locked`,
with `locked_until` and `threshold`. `recordSignInOutcome` now returns
the lock it actually wrote, so a lock write that did not land is never
reported as a lock.
- A sign-in refused by the lock gate emits `login_failed` with `reason:
ACCOUNT_LOCKED`.
- The client fingerprint is read by better-auth's own rule,
`getIP(headers, options)` plus `user-agent`. That is the rule
`internalAdapter.createSession` uses to stamp the session row a `login`
row copies. The IP source is unchanged; no new IP rule is introduced.
- The account is resolved the way the credential check resolves it. The
submitted identifier is never recorded, and an identifier that resolves
to no account emits nothing. The password is never read.
A successful sign-in is unchanged: the session hook records `login`, and
no account event is emitted.
## Tests
- `packages/plugins/plugin-auth/src/auth-account-event-audit.test.ts`
(new). It runs a real `AuthManager` and better-auth pipeline through
`handleRequest`, with a recording audit sink, and checks:
- one refused sign-in yields one `login_failed` naming the account, the
code and the client, with no password material in any event;
- its IP and user agent equal those of the `login` event for the same
client;
- it is recorded with lockout off;
- it names the account the credential check resolved;
- an unresolvable identifier yields nothing;
- reaching the threshold yields `login_failed` ×3, then
`account_locked`, whose `locked_until` equals the stored lock;
- a sign-in refused while locked yields `login_failed` with
`ACCOUNT_LOCKED`, and no second lock;
- refusals below the threshold yield no lock;
- a successful sign-in yields `login` and no account event.
- `packages/plugins/plugin-audit/src/auth-event-audit.test.ts`: account
events land as rows about the account, with `actor` null and the outcome
and client recorded. Session events still name their session.
- `sys-audit-log-retired-actions.test.ts`: the declared actions are
exactly the actions with a writer, set equality in both directions.
**Ablation**, run once on the committed implementation with
`scripts/ablation-replace.mjs` in wrap mode. Every leg was restored,
with blob equal to HEAD and an empty `git diff HEAD`.
| Leg | Mutation | Result |
|---|---|---|
| A | after-hook `login_failed` emission disabled | 6 cases red |
| B | `account_locked` emission disabled | 1 case red, the threshold
case |
| C | lock-gate `login_failed` emission disabled | 1 case red, the
locked-refusal case |
| D | sink credits the subject as `actor` on an account event | 2 cases
red |
## Gates (local, at `789c68ff9`, after merging `origin/main` at
`74f78159a`)
- **Build.** The dependency closure was built with `turbo run build
--filter=@objectstack/plugin-auth^...
--filter=@objectstack/plugin-audit^...`. After the merge,
`@objectstack/cli...` was built too, since `check:i18n` runs the built
CLI. Both exited 0.
- **Typecheck.** Both exited 0:
- `pnpm --filter @objectstack/plugin-audit typecheck`;
- `pnpm --filter @objectstack/plugin-auth typecheck`, which includes
`tsconfig.examples.json` and `check:test-typecheck`.
- **Tests.** All exited 0:
- `@objectstack/plugin-audit`: 59 files, 966 tests, re-run after the
merge;
- `@objectstack/plugin-auth`: 166 files, 3187 passed and 10 skipped
before the merge; the three touched suites (`auth-account-event-audit`,
`auth-session-audit`, `auth-manager`, 332 tests) re-run after it.
`auth-account-event-audit` was re-run again after the last commit.
- **Gate families.** `node scripts/pm/dispatch-gates.mjs --commands`
derived 101 families for this change set. All were run at the final
head, plus `pnpm --filter @objectstack/spec check:generated` and `node
scripts/check-nul-bytes.mjs`. `--ran` reconciliation: 101 derived, 100
run with exit 0, 1 NOT MEASURED, 0 unrun.
- NOT MEASURED: `node scripts/check-plugin-teardown-shape.mjs
--self-test` exited 3 (PREREQUISITE NOT MET). Its positive-control
fixture is pinned to a commit this shallow checkout cannot reach. CI
runs it on a full checkout.
- The first battery round caught one real finding:
`check:query-options-erasure` reported the new account lookup erasing
its query type. That is fixed in `789c68ff9`, and the ratchet holds at
64 sites.
- CI check-runs on this PR are the authority for every other family.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
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
7
+
8
+
Clause-②: no
9
+
10
+
-**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.
11
+
-**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.
12
+
-**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.
13
+
-**Recorded with or without a lockout.** A refused sign-in writes its row whether or not `lockoutThreshold` is set.
14
+
-**`@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.
|`login`|`createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth`| session start |
110
110
|`logout`|`createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth`| session end |
111
+
|`login_failed`|`createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth`| a refused password sign-in (`/sign-in/email`) that names an existing account |
112
+
|`account_locked`|`createAuthEventAuditSink` (this package), called by `@objectstack/plugin-auth`| the refused sign-in that stamps the account lockout (`lockoutThreshold`, ADR-0069 D2) |
111
113
|`config_change`|`@objectstack/service-settings`| every successful settings write |
112
114
|`import`|`@objectstack/plugin-auth` admin user import | one run-level row, with `record_id: null`|
115
+
|`platform_admin_standing_change`|`@objectstack/plugin-security`| a change of who holds platform-administrator standing, plus a first-boot baseline |
113
116
114
117
Action values are snake_case. There is no namespaced `domain:action` spelling — the enum
115
118
above is the complete accepted set, and a value outside it is not a form this object takes.
116
119
120
+
**Session events and account events.** A `login` / `logout` row is about a session: it names
121
+
`sys_session` and the session id, and `actor` is the subject (or the administrator who
122
+
started an impersonation session). A `login_failed` / `account_locked` row is about an
123
+
account and exists without any session: `user_id` is the account the sign-in named, the row
124
+
names `sys_user` and that account's id, and `actor` is null, because a refused attempt
125
+
authenticated nobody. Its `metadata` carries the endpoint and the refusal's machine code
126
+
(`INVALID_EMAIL_OR_PASSWORD`, `ACCOUNT_LOCKED`, …), plus the lock's expiry and threshold on
127
+
an `account_locked` row. It never carries the attempted credential, and an identifier that
128
+
resolves to no account writes no row.
129
+
117
130
## `sys_audit_log` fields
118
131
119
132
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
123
136
|---|---|---|
124
137
|`id`| text | Audit log entry id |
125
138
|`created_at`| datetime | When the action occurred (`NOW()` default) |
126
-
|`action`| select | One of the eight values above |
139
+
|`action`| select | One of the eleven values above |
127
140
|`user_id`| lookup → `sys_user`| Null for non-user / service actions |
128
141
|`actor`| text | Principal label: a user id, `svc:<name>`, or null. Attributes service-token writes that `user_id` structurally cannot hold |
129
142
|`object_name`| text | Target object, e.g. `sys_user`|
@@ -144,7 +157,9 @@ without the secret itself reaching the ledger.
144
157
**`ip_address` / `user_agent` are populated on auth events only.** Neither the
145
158
record-level writer nor the record-view writer stamps them: a `create` / `update` /
146
159
`delete` / `read` row records who and what, not from where. Do not read a null client
147
-
fingerprint on such a row as "the request had none".
160
+
fingerprint on such a row as "the request had none". On every auth event they are
161
+
resolved by better-auth's own rule: a session event copies the session row's values, and
162
+
an account event applies the same resolution to the sign-in request.
148
163
149
164
**`old_value` / `new_value` are null on every `read` row**, deliberately and not as an
150
165
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:
0 commit comments