Repository navigation
Commit 256a4b7
fix(plugin-auth): an MFA-held sign-in records the hold on its own login row (#30950)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} -->
_Requested by **Elon** · [project
thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DNZtDC3xZoX5DtBJm5N9yWS)_
domain:server#3 · os-elon-musk · session_016WV4cwTfQ1ifwQY2EUnSUt
Refs #30924
Clause-②: no
Metadata-protocol: none
## 维护者速读
- **改了什么**:因为没开双重验证而被挂起的登录,它自己的那条 `login` 审计记录现在会写上 `reason:
MFA_REQUIRED`、`rule: mfa_required`。
-
**为什么改**:企业版的「谁、从哪里、因为哪条规则被拦」报表要用这份数据(cloud#3668)。实测下来,这种登录其实是成功的:会话已经签发,`login`
记录也已经写了,所以挂起记在这条记录上,不另写一条互相矛盾的 `login_failed`。
-
**风险与回滚**:每次登录在会话创建时多做一次挂起判定,只在装了审计插件、并且开着某项门槛功能时才会多读库。判定失败时不记挂起,登录照常。其余审计记录一律不变。回滚就是
revert。
- **先看**:① `session-policy-hold.ts`(要记哪些挂起、模拟登录的会话怎么判);②
`auth-manager-database-hooks.ts` 的 `sessionCreateAfter`;③
`auth-manager.ts` 增补的两行(席位批准)。
- **两分钟验证**:`pnpm --filter @objectstack/plugin-auth exec vitest run
src/auth-account-event-audit.test.ts`,预期 `[#30924]` 那组 4 个用例全绿。
- **需要你做的**:无,席位落地。
## What changes
**A sign-in held by ADR-0069 D3 enforced MFA now records the hold on its
own `login` row.** The condition: the requirement is on, the account
never enrolled, and its grace window has lapsed. The row's `metadata`
gains `reason: "MFA_REQUIRED"` (the posture's machine code, the same
vocabulary a refused sign-in's `reason` uses) and `rule: "mfa_required"`
(the policy's name), beside `endpoint`.
- **Where it is asked:** in `session.create.after`
(`auth-manager-database-hooks.ts`), the hook that already writes the
`login` row, once per session created. It asks the same posture
computation the session gate asks: `AuthManager.resolveUserAuthGates`,
for the session's user in the session's active organization.
- **Which postures hold which session:** `session-policy-hold.ts`
follows the split `computeAuthGate` makes. Every posture holds a session
the user signed in to themselves; only an `account`-scope posture holds
an impersonation session.
- **What is recorded:** only the postures in
`AUDITED_SESSION_HOLD_RULES`, which today is just `MFA_REQUIRED` →
`mfa_required`. Adding another posture is one more entry. A user held by
both the forced reset and MFA has the MFA hold recorded.
- **Failure and cost:** a posture read that fails records no hold, which
is the direction the gate itself fails open in. A deployment without the
audit plugin makes no posture read.
- **What does not change:** every other `login`, `logout`,
`login_failed` and `account_locked` row. None of them carries the
attempted credential.
- `session-policy-hold.ts` is package-internal, and `loginEventFor`'s
signature is unchanged, so the published surface of
`auth-session-audit.ts` does not move (`Clause-②: no`).
**Why the `login` row, not a `login_failed`** (seat ruling, fork 2
option B). Measured on `origin/main` `0e882086`, an MFA-held
`/sign-in/email` answers 200 with a session token and writes a `login`
row. The hold is computed on every session read: `resolveUserAuthGates`
ran 0 times during the sign-in and once for each of 3 `get-session`
calls. So:
- a `login_failed` beside the `login` would contradict it, and a
brute-force reader counting `login_failed` would count users who are
signed in;
- a row written where the gate is computed would be one row per request,
not one per sign-in.
**For the cloud#3668 consumer:**
- The MFA hold is read from the `login` row: `action = 'login'`,
`metadata.reason = 'MFA_REQUIRED'`, `metadata.rule = 'mfa_required'`,
with `user_id`, `tenant_id`, `ip_address`, `user_agent` and
`created_at`.
- The IP allow-list refusal will be read from `login_failed` rows with
`metadata.rule = 'ip_allow_list'`. They arrive in PR 2 of this series.
**Seat-approved surface addendum:** in `auth-manager.ts`, only the
`composeDatabaseHooks` host literal changes. One member joins the
`DatabaseHooksHost` Pick union and one forwarding closure is added for
`resolveUserAuthGates`. Nothing else in the file is touched.
**Overlap with #30922:** that card's WIP commit `15ec27e3` on
`claude/issue-30922-per-org-access-policy` adds `judgeSessionScoping` to
the same Pick union line, plus a closure beside `linkingContext`, in
`auth-manager.ts`. It does the same in the `DatabaseHooksHost` interface
and the `composeDatabaseHooks` `this` Pick in
`auth-manager-database-hooks.ts`. Whichever PR lands second gets a
one-line textual conflict, and the resolution keeps both members.
**Changeset:** `'@objectstack/plugin-auth': patch`.
## Why
#30924: plugin-auth writes no audit row when policy holds or refuses a
sign-in, so the Enterprise enforcement report (cloud#3668) has nothing
to read. This is PR 1 of the two-PR series in the card's Claim. PR 2
carries the IP allow-list rows: the deployment gate and #30922's
per-organization refusal. It also carries the plugin-audit change that
admits a row with no resolvable account, and the flood bound. PR 2 opens
after #30922 merges.
## Tests
- `auth-session-audit.test.ts`, new top-level describe, at the
composition seam (the real `composeDatabaseHooks`, with the posture
answer stubbed on the manager):
- an MFA-held session: the event carries `reason` + `rule`, and the
posture is asked once with `('usr_1', { organizationId: 'org_1' })`;
- controls: no posture, and a posture the table does not record
(password expiry), both leave the row exactly as before;
- reset + MFA records MFA;
- an impersonation session records MFA (account scope) beside
`impersonated_by` and `actor`;
- a throwing posture read still writes the row, with no hold;
- no audit plugin means no posture read;
- no subject means no read and no row.
- `auth-account-event-audit.test.ts`, new top-level describe, on a real
`AuthManager` over the in-memory engine and real better-auth:
- an MFA-held sign-in writes one `login` event carrying the hold, no
account event, and no password material;
- 3 `get-session` reads, each answered `authGate: MFA_REQUIRED`, still
leave exactly one event;
- control: inside the grace window, the row is as before;
- control: a wrong password on a held account is still one
`login_failed` with `INVALID_EMAIL_OR_PASSWORD` and no hold.
- Dogfood, `auth-session-audit-trail.dogfood.test.ts`, new top-level
describe on a booted showcase with `AuditPlugin`. `auth.mfa_required` is
turned on through the settings service and the account's grace is
lapsed. Then:
- `get-session` answers `MFA_REQUIRED`;
- exactly one `login` row in `sys_audit_log` names the hold (`metadata`
has `endpoint`, `reason`, `rule`), with no `login_failed` and no
credential anywhere in the member's rows.
## Acceptance notes
- **Boot window on the per-organization requirement.**
`resolveUserAuthGates` sees an organization's own `require_mfa` through
the lazily refreshed "does any org require MFA" cache. The session gate
reads the same cache. A sign-in in the moment after boot, before the
first refresh lands, records no hold for a per-org requirement, exactly
as the gate does not hold that first read. The global requirement has no
such window.
- **The table records MFA alone.** Password expiry and the forced reset
are holds too, but they were not asked for. Adding either is one entry
in `AUDITED_SESSION_HOLD_RULES`; a `credential`-scope entry would then
be skipped on impersonation sessions by the scope filter already in
place.
## Verification
All readings on head `60164041` (merge base `4d56914a` on `main`), in
this checkout.
- **Pins:** `auth-session-audit.test.ts` +
`auth-account-event-audit.test.ts` → 45 passed (45).
- **Unit ablation** (`scripts/ablation-replace.mjs`,
`withSessionPolicyHold(login, hold)` → `login` in `sessionCreateAfter`):
4 failed | 41 passed. The 4 are the MFA-held, reset+MFA, impersonation
and real-sign-in cases; every control stayed green. Restored, blob back,
tree clean.
- **Dogfood:** `auth-session-audit-trail.dogfood.test.ts` → 6 passed
(6). Same ablation, rebuilt, with `ablation-dist-preflight.mjs --absent`
proving the marker left `dist/`: the `#30924` case failed (1 failed | 5
skipped under `-t 30924`). Restored, rebuilt, marker back in `dist/`,
`git status --porcelain` empty.
- **Package suites:** `@objectstack/plugin-auth` full suite 228 files,
4108 passed | 10 skipped. `@objectstack/plugin-audit`
`auth-event-audit.test.ts` 17 passed (17).
- **Types / lint:** `@objectstack/plugin-auth` typecheck (source +
tests) exit 0. `eslint --no-inline-config` on the 7 changed source and
test files exit 0.
- **Build:** whole tree under `os-verify-lock`, `turbo run build` → 119
successful, 119 total.
- **Dispatch gates:** `scripts/pm/dispatch-gates.mjs --ran` → 67
derived, 67 run, 0 NOT-MEASURED; all 67 exit 0. These include
`check:auth-mount-ledger`, `check:tenant-audit-census`,
`check:durability-log-level`, `check:dts-closure`,
`check:published-files`, `check:issue-citations` and the changeset
gates.
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(No spec change.)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes. (CI owns the repo-wide
lint run.)
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request (PR 1 of the claimed two-PR series).
- [x] 5. A change that publishes carries a changeset, and both
declaration lines are filled in here and in the changeset.
- [x] 6. `packages/spec` touched: not touched.
- [x] 7. No hand edits to `content/docs/references/`,
`packages/*/CHANGELOG.md` or `content/docs/releases/`.
- [x] 8. Title, body and comments are in English.
---
_Generated by [Claude
Code](https://claude.ai/code/session_016WV4cwTfQ1ifwQY2EUnSUt)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent b58beb8 commit 256a4b7
8 files changed
Lines changed: 420 additions & 6 deletions
File tree
- .changeset
- packages
- plugins/plugin-auth/src
- qa/dogfood/test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
268 | 268 | | |
269 | 269 | | |
270 | 270 | | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
55 | 55 | | |
56 | 56 | | |
57 | 57 | | |
| 58 | + | |
58 | 59 | | |
59 | 60 | | |
60 | 61 | | |
| |||
75 | 76 | | |
76 | 77 | | |
77 | 78 | | |
| 79 | + | |
78 | 80 | | |
79 | 81 | | |
80 | 82 | | |
| |||
83 | 85 | | |
84 | 86 | | |
85 | 87 | | |
86 | | - | |
| 88 | + | |
87 | 89 | | |
88 | 90 | | |
89 | 91 | | |
| |||
319 | 321 | | |
320 | 322 | | |
321 | 323 | | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
322 | 333 | | |
323 | 334 | | |
324 | 335 | | |
325 | | - | |
326 | | - | |
327 | | - | |
328 | | - | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
329 | 344 | | |
330 | 345 | | |
331 | 346 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5306 | 5306 | | |
5307 | 5307 | | |
5308 | 5308 | | |
5309 | | - | |
| 5309 | + | |
5310 | 5310 | | |
5311 | 5311 | | |
5312 | 5312 | | |
5313 | 5313 | | |
5314 | 5314 | | |
| 5315 | + | |
5315 | 5316 | | |
5316 | 5317 | | |
5317 | 5318 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
287 | 287 | | |
288 | 288 | | |
289 | 289 | | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
81 | 106 | | |
82 | 107 | | |
83 | 108 | | |
| |||
0 commit comments