Repository navigation
Commit ea62933
feat(plugin-auth, plugin-audit): a sign-in the IP allow-list refuses writes one login_failed row, bounded per address (#31249)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1XFicVTXXKLtFurhMN9bwcxw)_
domain:server#6 · os-project-manager · session_01FEkQ5pfyn4wSGLdTuVZctK
Fixes #30924
Clause-②: yes (widening)
Metadata-protocol: none
## 维护者速读
- **改了什么**:因为 IP 不在允许范围而被拒的登录,现在每次在审计日志里写一条 `login_failed`,带上 `reason:
IP_NOT_ALLOWED`、`rule:
ip_allow_list`、客户端地址、能认出的账号,以及组织(组织自己的名单拒绝时)。部署级名单和组织级名单两道门都记。
-
**为什么改**:企业版「谁、从哪里、因为哪条规则被拦」的报表(cloud#3668)要读这份数据;之前部署级名单拒绝时什么都不写。这是本卡两个
PR 的第二个,第一个(MFA 挂起)已合并。
- **风险与回滚**:扫描器狂刷时有上限:同一地址 10 分钟内只在第 1、10、100 次各写一行,全局每 10 分钟最多 1000
行,超出只打一行告警。部署级名单的写入不阻塞拒绝响应,响应内容和耗时与账号是否存在无关。回滚就是 revert。
- **先看**:① `ip-allow-list-refusal-audit.ts`(限流规则和行的形状);②
`auth-plugin.ts` 的 `auditAllowListRefusal`(部署级名单);③
`auth-manager-database-hooks.ts` 的
`refuseUnadmittedScoping`(组织级名单,切换组织不记)。
- **两分钟验证**:`pnpm --filter @objectstack/plugin-auth exec vitest run
src/ip-allow-list-refusal-audit.test.ts
src/ip-allow-list-refusal-audit-bound.test.ts`,预期 17 个用例全绿。
- **需要你做的**:无,席位落地。
## What changes
**A sign-in refused by the ADR-0069 D5 IP allow-list now writes one
`login_failed` row**, at both doors that refuse a sign-in by address.
The row follows the incumbent shape of a refused sign-in
(`signInFailureEventFor`, `auth-session-audit.ts`): `action:
'login_failed'` with `metadata.endpoint` and `metadata.reason`. It adds
`metadata.rule: 'ip_allow_list'`, the way an MFA-held `login` row names
`mfa_required` (PR #30950), and `metadata.attempts` (the bound, below).
The client address and user agent come from `signInClientFingerprint`,
the rule every `login` row's address is read by. The lane kept the
incumbent `login_failed` action rather than a dedicated one: a reader
counting refused sign-ins already reads it, and `rule` separates the
policy refusals from the password ones.
- **The deployment's list** (`auth-plugin.ts`, the auth-route
middleware, new private `auditAllowListRefusal`). It refuses before
better-auth resolves any account. The row names the account when the
request's JSON `email` resolves to one, through the new public forwarder
`AuthManager.resolveRefusedSignInAccountId(email)` (open question 1,
option A: the password sign-in's own `findSignInAccount` question).
Otherwise the row is written with no subject (ruling 1A). Only
`/sign-in/*` endpoints write a row (ruling 4); a refused `get-session`
or `sign-out` does not.
- **The answer does not change.** The bound is judged synchronously. The
body read, the account lookup and the write run detached from the
response (`recordDeploymentListRefusal`), so the `403 IP_NOT_ALLOWED`
status, body and timing do not depend on whether the identifier names an
account. That keeps the refusal from becoming an account-existence
oracle for a client outside the ranges. The body read is capped at 16
KiB; a larger or unparsable body writes the row with no subject.
- **An organization's own list** (`auth-manager-database-hooks.ts`,
`refuseUnadmittedScoping`, the `SESSION_SCOPING_REFUSAL` site). The
credential check has passed there, so the row names the account and the
organization (`tenant_id`), and `impersonated_by` on an impersonation
session. Only the session's creation writes the row. A refused switch of
the active organization (`session.update.before`) is not a sign-in and
writes none (open question 2, option A).
- **No second row.** On `main` the password sign-in's outcome already
wrote a `login_failed` for this refusal (reason `IP_NOT_ALLOWED`, no
rule, no organization, unbounded), because better-auth hands the
after-hook the refusal the session hook threw. `signInFailureEventFor`
now answers `null` for `IP_NOT_ALLOWED`
(`REFUSALS_RECORDED_AT_THEIR_DOOR`), so the door's own row is the only
one. That code reaches the after-hook only from this door; the
deployment gate refuses before better-auth runs.
- **The bound** (`IpRefusalAuditBound`, ruling 5). Per client address, a
10-minute window from its first refusal: rows at the 1st, 10th and 100th
refusal, each carrying `attempts`. Across all addresses, at most 1000
rows per 10-minute window, and one `warn` line per window once the cap
drops a row. An undetermined address counts as one address. The tracked
addresses are capped at 10,000 (expired first, then the stalest), so
rotating addresses cannot grow memory. Both doors share one bound per
audit sink.
- **Across replicas:** the bound lives in process memory, so N server
processes write up to N times as many rows. No shared store is used, and
none was ruled.
- **plugin-audit** (`auth-event-audit.ts`):
`AuthSessionAuditEvent.userId` is optional. A `login_failed` with no
subject is written with `user_id`, `record_id` and `actor` null. It is a
row about no record, read under the ledger's own grant
(`audit-log-read-visibility.ts`); with no organization it is served to
platform administrators only. Every other event with no subject still
writes nothing.
- **plugin-auth's `AuthEventAuditSurface`**: the event's `userId` is
optional to match. `Clause-②: yes (widening)`.
- ⛔ **No row carries the credential.** The identifier is read only to
find the account and is never recorded; the password is never read.
- **Docs:** `content/docs/kernel/runtime-services/audit-service.mdx` now
shows the optional `userId`, `reason`/`rule` on an MFA-held `login` row
and on an IP refusal, and the bound.
- **Changeset:** `@objectstack/plugin-auth` and
`@objectstack/plugin-audit`, `patch`.
**For the cloud#3668 consumer:** read IP refusals as `action =
'login_failed'` with `metadata.rule = 'ip_allow_list'`. Use `user_id`
(may be null), `tenant_id` (set when an organization's list refused),
`ip_address`, `user_agent`, `created_at` and `metadata.attempts`. A row
with `attempts = 100` means at least a hundred refusals from that
address in its window.
## Why
#30924: plugin-auth wrote no audit row when policy refused a sign-in, so
the Enterprise enforcement report (objectstack-ai/cloud#3668) had
nothing to read. PR 1 (#30950) recorded the MFA hold on the held
sign-in's `login` row. This PR 2 carries the IP allow-list rows, after
#30922 (PR #31013) added the organization's own list.
## Tests
- `ip-allow-list-refusal-audit.test.ts` (new). The deployment half
mounts the real `AuthPlugin.registerAuthRoutes` on a real Hono app in
front of a real `AuthManager`. The organization half goes through
`AuthManager.handleRequest` with the transport peer.
- deployment, existing account: `403 IP_NOT_ALLOWED`; one event with the
account id, the address, the user agent, `{endpoint, reason, rule,
attempts: 1}`; neither the password nor the email appears.
- deployment, unknown identifier: one event, no `userId`.
- `get-session` / `sign-out` refused: no event; `/sign-in/magic-link`
refused: an event.
- control: an admitted sign-in writes no refusal event.
- flood: 120 refusals from one address give rows at attempts 1, 10, 100;
a second address starts at 1.
- the answer does not change: same status and body with and without the
sink, and the response arrives before a 150 ms account lookup settles.
- no audit plugin: the forwarder is never called.
- organization, refused at creation: one event with the account,
`organizationId`, the address and the rule (one, not two).
- control: an admitted sign-in writes only its `login`.
- a refused switch of the active organization: `403 IP_NOT_ALLOWED`, no
event.
- 12 organization refusals from one address: attempts 1 and 10.
- `ip-allow-list-refusal-audit-bound.test.ts` (new): the ruled
constants; the per-address window restart; the undetermined address; the
global cap with one `warn` per window; the tracked-address cap; the
event shape.
- `plugin-audit/src/auth-event-audit.test.ts`: a subject-less
`login_failed` lands about no record, credits nobody, and keeps the
address and the rule. Control: `login`, `logout` and `account_locked`
with no subject write nothing.
- Dogfood `auth-session-audit-trail.dogfood.test.ts`, new describe on a
booted showcase with `AuditPlugin`. `auth.allowed_ip_ranges` is set
through the settings service. Twelve `/auth/sign-in/email` requests
answer `403 IP_NOT_ALLOWED` and leave exactly two `sys_audit_log` rows
for the account (attempts 1 and 10), each about `sys_user`, with the
rule, the reason and the user agent, and no credential.
## Acceptance notes
- **Found while measuring, not fixed here (reported to the seat):** on
`main`, a sign-in refused by an organization's own IP list counts toward
the ADR-0069 D2 lockout, although the password was right. Measured with
`lockoutThreshold: 3`: three `/sign-in/email` attempts with the correct
password from outside Alpha's list answer `403 IP_NOT_ALLOWED`, and the
fourth, from inside the list, answers `403 ACCOUNT_LOCKED`
(`failed_login_count` 3). The cause is the password after-hook
(`auth-manager-after-hook.ts`) reading every returned error as a failed
credential. That file is outside this card's surface, so it goes to the
seat to file.
- **Across replicas:** the bound is per process (module note in
`ip-allow-list-refusal-audit.ts`). N replicas write up to N times as
many rows; no shared store was ruled.
- **Detached write:** the deployment gate's row is written after the
`403` is returned. A runtime that ends the process right after the
response could lose that row. The Node server this ships on does not.
- **Which sign-ins name an account at the deployment gate:** only a JSON
body with an `email` (`/sign-in/email`, `/sign-in/magic-link`,
`/sign-in/email-otp`). Social, SSO, passkey and username sign-ins are
written with no subject. All refusals whose address could not be
established count as one address in the bound, and their rows carry no
`ip_address`.
- **A published function changes behaviour:** `signInFailureEventFor`
(exported from `auth-session-audit.ts`) now answers `null` for `reason:
'IP_NOT_ALLOWED'`. The door that refused writes that row.
- **File surface:** the new module `ip-allow-list-refusal-audit.ts` and
its two test files sit in the claimed
`packages/plugins/plugin-auth/src/`, package-internal (not re-exported).
`organization-access-policy.ts` is not touched; the refusal site edited
is in `auth-manager-database-hooks.ts`.
- **Merged `origin/main` once** (`2aeb9e05`): #31194 (SSO refusal codes)
touched `auth-manager.ts` in a different region and the merge was clean.
The plugin-auth suite was run again on the merged head.
## Verification
Readings on head `2aeb9e05` (the implementation `148bac5f` plus one
merge of `origin/main` at `065bba72`) unless named, in this checkout.
<details><summary>Red first, on origin/main 658d84f (production sources
restored, tests kept)</summary>
- `ip-allow-list-refusal-audit.test.ts`: 8 failed | 3 passed. The
deployment cases fail with `expected [] to have a length of 1` (the sink
receives nothing), and the flood case with `expected [] to have a length
of 3`. The forwarder cases fail with `resolveRefusedSignInAccountId is
not defined`. The organization case receives the password outcome's row,
`{endpoint, reason: IP_NOT_ALLOWED}`, with no `rule`, no
`organizationId` and no `attempts`. Twelve organization refusals give
twelve rows. The three controls pass: an admitted sign-in, the refused
switch, and the bound file not loaded.
- `plugin-audit` `auth-event-audit.test.ts`: 2 failed | 17 passed (the
subject-less row is dropped).
</details>
<details><summary>Ablations (scripts/ablation-replace.mjs, each
restored: blob == HEAD, git diff HEAD empty)</summary>
| # | Mutation | Red |
|---|---|---|
| A1 | the gate's `auditAllowListRefusal` call deleted | 5 deployment
cases |
| A2 | the per-address marks ignored | 4 (flood, organization bound, 2
bound unit cases) |
| A3 | the switch hands `creating` in | the switch case (negative pin) |
| A4 | the `IP_NOT_ALLOWED` suppression in `signInFailureEventFor`
deleted | 2 (two rows instead of one) |
| A5 | `isSignInEndpoint` admits every path | the `get-session` /
`sign-out` case (negative pin) |
| A6 | the write awaited before the 403 | the answer-does-not-wait case
(negative pin) |
| B1 | plugin-audit refuses a subject-less `login_failed` | 2 |
| B2 | plugin-audit admits any subject-less event | 2 (negative pin,
incl. the #8144 case) |
- A6's first attempt was a no-op: an unbalanced replacement failed to
parse, the integration file did not load, and the reading was discarded.
It was re-run as three held mutations (`async` method, `await` call,
`await` at the gate) and read red.
- Dogfood, on `dist/`: A1 rebuilt. The DTS step failed TS6133 (the
method was left unused) after tsup wrote the mutated JS.
`ablation-dist-preflight.mjs --absent` showed the marker gone from all
14 built files, and the new dogfood case failed (1 failed | 6 skipped).
Restored and rebuilt: marker present in 2 built files, dogfood 7 passed
(7).
</details>
<details><summary>Suites, types, lint, gates</summary>
- `@objectstack/plugin-auth` full suite on `2aeb9e05`: 236 of 237 files
pass, 4199 passed | 1 failed | 10 skipped. The one red is
`organization-access-policy.test.ts` "the absolute limit is judged
too…", the known exact-60-minute boundary flake that #31175 carries.
Re-run alone: 19 passed (19), three times out of three. On `148bac5f`
before the merge: 236 files, 4185 passed | 10 skipped.
- `@objectstack/plugin-audit`: full suite 1203 passed (1203) on
`148bac5f`; `auth-event-audit.test.ts` 19 passed (19) on `2aeb9e05`.
- Dogfood `auth-session-audit-trail.dogfood.test.ts`: 7 passed (7) on
`2aeb9e05`.
- Typecheck (source + tests): `@objectstack/plugin-auth` exit 0,
`@objectstack/plugin-audit` exit 0 on `2aeb9e05`; `@objectstack/dogfood`
exit 0 on `148bac5f`.
- `eslint --no-inline-config` on the 10 changed `.ts` files: exit 0.
- Build: `turbo run build` under `os-verify-lock`, 120 successful, 120
total (VERDICT command-exit 0).
- `scripts/pm/dispatch-gates.mjs --ran` on `2aeb9e05`: 99 derived, 99
run, 0 NOT-MEASURED, all 99 exit 0. Three families first answered exit 3
(PREREQUISITE) and were re-run after their prerequisites:
`check-plugin-teardown-shape --self-test` after fetching its fixture
commit; `check:skill-examples` and `check:dual-build-cjs-loads` after
the full build.
</details>
## 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; `eslint --no-inline-config` passes on the
changed files. (CI owns the repo-wide lint run.)
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request (PR 2 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_01FEkQ5pfyn4wSGLdTuVZctK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent abc7a94 commit ea62933
12 files changed
Lines changed: 1082 additions & 25 deletions
File tree
- .changeset
- content/docs/kernel/runtime-services
- packages
- plugins
- plugin-audit/src
- 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 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
32 | | - | |
| 32 | + | |
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
50 | | - | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
51 | 52 | | |
52 | | - | |
53 | | - | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
54 | 60 | | |
55 | 61 | | |
56 | 62 | | |
| |||
67 | 73 | | |
68 | 74 | | |
69 | 75 | | |
70 | | - | |
71 | | - | |
72 | | - | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
73 | 93 | | |
74 | 94 | | |
75 | 95 | | |
| |||
80 | 100 | | |
81 | 101 | | |
82 | 102 | | |
83 | | - | |
84 | | - | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
85 | 106 | | |
86 | 107 | | |
87 | 108 | | |
| |||
98 | 119 | | |
99 | 120 | | |
100 | 121 | | |
101 | | - | |
102 | | - | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
103 | 125 | | |
104 | 126 | | |
105 | 127 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
620 | 620 | | |
621 | 621 | | |
622 | 622 | | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
| 670 | + | |
| 671 | + | |
| 672 | + | |
| 673 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
78 | 83 | | |
79 | 84 | | |
80 | 85 | | |
| |||
130 | 135 | | |
131 | 136 | | |
132 | 137 | | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
133 | 144 | | |
134 | | - | |
| 145 | + | |
135 | 146 | | |
136 | 147 | | |
137 | 148 | | |
| |||
206 | 217 | | |
207 | 218 | | |
208 | 219 | | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
209 | 229 | | |
210 | 230 | | |
211 | 231 | | |
| |||
354 | 374 | | |
355 | 375 | | |
356 | 376 | | |
357 | | - | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
358 | 380 | | |
359 | 381 | | |
360 | 382 | | |
| |||
367 | 389 | | |
368 | 390 | | |
369 | 391 | | |
370 | | - | |
371 | | - | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
372 | 395 | | |
373 | 396 | | |
374 | 397 | | |
375 | 398 | | |
376 | | - | |
| 399 | + | |
377 | 400 | | |
378 | 401 | | |
379 | 402 | | |
| |||
393 | 416 | | |
394 | 417 | | |
395 | 418 | | |
396 | | - | |
| 419 | + | |
397 | 420 | | |
398 | 421 | | |
399 | 422 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
54 | 54 | | |
55 | 55 | | |
56 | 56 | | |
57 | | - | |
| 57 | + | |
| 58 | + | |
58 | 59 | | |
59 | 60 | | |
60 | 61 | | |
| |||
314 | 315 | | |
315 | 316 | | |
316 | 317 | | |
317 | | - | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
318 | 324 | | |
319 | 325 | | |
320 | | - | |
| 326 | + | |
| 327 | + | |
321 | 328 | | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
322 | 344 | | |
323 | 345 | | |
324 | 346 | | |
| |||
329 | 351 | | |
330 | 352 | | |
331 | 353 | | |
332 | | - | |
| 354 | + | |
333 | 355 | | |
334 | 356 | | |
335 | 357 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6165 | 6165 | | |
6166 | 6166 | | |
6167 | 6167 | | |
| 6168 | + | |
| 6169 | + | |
| 6170 | + | |
| 6171 | + | |
| 6172 | + | |
| 6173 | + | |
| 6174 | + | |
| 6175 | + | |
| 6176 | + | |
| 6177 | + | |
| 6178 | + | |
| 6179 | + | |
| 6180 | + | |
| 6181 | + | |
| 6182 | + | |
| 6183 | + | |
6168 | 6184 | | |
6169 | 6185 | | |
6170 | 6186 | | |
| |||
0 commit comments