Repository navigation
Commit d463b99
fix(plugin-auth,plugin-audit): a SCIM-driven write names the SCIM connection that performed it, on its audit row (#30989)
<!-- claude-projects-attribution: {"github_login":"os-elon-musk"} -->
_Requested by **Elon** · [project
thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9D4o7Mmi48dsRAPhuMW15NwD)_
domain:server#5 · os-elon-musk · session_016WV4cyRYCwPvYJ68qTXzEP
Fixes #30960
Clause-②: yes (widening)
Metadata-protocol: none
## 维护者速读
- **改了什么**:IdP 通过 SCIM 开通、停用、删除用户时,平台写的每条审计记录现在标明是哪个 SCIM 连接做的:`actor`
记为 `svc:scim:连接ID`,`metadata` 里带 `scim_connection_id`,`user_id`
仍为空(没有人在操作)。
- **为什么改**:以前这些记录的「谁做的」三栏全空,审计员查不出离职员工是被哪个 IdP 停用的;企业版 SCIM
离职流程(cloud#3667)和审计导出(cloud#3665)都要靠这个归属。
- **风险与回滚**:只在 SCIM 请求内生效,其他写入不变;bearer
和它的摘要永远进不了记录;两个连接并发时各记各的。这三点都有测试钉住。回滚即 revert。
- **先看**:① `scim-connection-service.ts` 的
`ScimRequestScope.connectionId` 和
`recordScimRequestConnection`(槽位为什么能到达每一次写入);②
`auth-actor-attribution.ts` 中 `authSystemWriteContext` 开头的 SCIM 分支;③
plugin-audit `audit-writers.ts` 的 metadata 一段。
- **两分钟验证**:`pnpm --filter @objectstack/plugin-auth exec vitest run
src/scim-transaction-scope.test.ts`,预期 9 个测试全绿。
- **需要你做的**:无,席位落地。
## What changes
A SCIM protocol request is performed by the identity provider's
connection, which its bearer verifies as. Every `sys_audit_log` row such
a request writes now names that connection.
**`@objectstack/plugin-auth`**
1. **The slot.** `scimRequestScope`, the store `handleRequest` opens
with `run(...)` around every `/scim/v2/*` request, is now typed
`ScimRequestScope` and carries an optional `connectionId`.
`recordScimRequestConnection(verification)` writes the verified
connection's id into the CURRENT store (first verification wins; a no-op
outside the scope and on a failed verification) and returns the
verification unchanged. `scimRequestConnectionId()` reads it.
2. **The verifier records it.** The `verifyBearerToken` callback in
`auth-manager-plugin-list.ts` now returns
`recordScimRequestConnection(await verifyScimBearerToken(...))`.
`verifyScimBearerToken` itself is unchanged and still pure.
3. **Every write reads it.** `authSystemWriteContext()`, the context
builder the adapter's `withSystemContext` (and the membership
reconciler) uses for every better-auth-owned write, returns `{ isSystem:
true, actor: 'svc:scim:CONNECTION_ID' }` when the request has a verified
connection, and builds the context exactly as before otherwise. No
`attributedUserId`, so `user_id` stays null. The deactivation's ban
write (`reconcileScimUserLifecycle`) goes through the vendor-bound
adapter, so it carries the same actor.
4. `scimServicePrincipal(connectionId)` spells the label,
`SCIM_SERVICE_PRINCIPAL_PREFIX` is `svc:scim:`.
**`@objectstack/plugin-audit`**
5. New `scim-connection-attribution.ts` (`SCIM_CONNECTION_ID_KEY`,
`scimConnectionOf`), in the shape of `impersonation-attribution.ts`. In
`audit-writers.ts`, the CRUD ledger row's `metadata` gains
`scim_connection_id` when the row's actor label starts with `svc:scim:`.
The label is already recorded on `actor` by the existing
service-principal fallback (`userId ?? session.actor`). Rows of every
other actor are byte-identical.
## Why
The card's measurement on a licensed objectos-ee serve: every
SCIM-driven row had `actor`, `user_id` and `metadata` null. Reproduced
here first, on `main` `256a4b72`, through `AuthManager.handleRequest`
with a real bearer, over better-sqlite3, with plugin-audit's real writer
and `sys_audit_log` registered: a SCIM create, a deactivation, a second
create and a delete wrote 21 rows, and all 21 had `actor`, `user_id` and
`metadata` null. After the change, on this PR's head, the same sequence
wrote 21 rows. 20 have `actor: svc:scim:okta-30960`, `user_id: null` and
`metadata: {"scim_connection_id":"okta-30960"}`. The 21st is noted
below. Neither the bearer nor its digest occurs in any row, before or
after.
## Tests
Two homes, the split the package boundary already draws. plugin-auth
does not depend on plugin-audit, and `auth-account-event-audit.test.ts`
records that the row shape is plugin-audit's to pin.
**`packages/plugins/plugin-auth/src/scim-transaction-scope.test.ts`**: a
new top-level `describe('[#30960] …')`, in the shared harness of the
SCIM request scope. Each case drives `AuthManager.handleRequest()` with
a real bearer on a real `ObjectQL` over better-sqlite3, and records
every engine `insert` / `update` / `delete` with the context it reached
the RAW engine with, beneath `withSystemContext`, which is the context
the audit hooks are built from.
- **(e)** A create, a deactivation and a delete: each request's writes
all carry `isSystem: true` and `actor: 'svc:scim:okta-14522'`, and none
carries `attributedUserId`, `impersonatedBy` or `performedBy`. The
deactivation includes the `sys_user` ban write (`banned` set), and the
delete includes the `sys_scim_user` delete and the `sys_user` ban write.
- **(f)** Across all three, no write's data, query or context contains
the bearer or its digest. The control: the capture holds at least 5
writes and does contain the label.
- **(g)** A sign-up before a SCIM request and a sign-in after it carry
no `actor` at all, and `svc:scim:` occurs nowhere in their writes.
- **(h)** Isolation: a second connection (`entra-30960`) is minted. Both
requests' credential probes are held at a barrier until BOTH have
arrived, so both connections are verified before either request writes.
The `sys_user` insert for each email carries its own request's
connection. Every write the vendor stamps with a `connection_id` carries
the actor of that same connection, and no write carries any other actor.
**`packages/plugins/plugin-audit/src/audit-writers.test.ts`**: a new
top-level `describe('audit writers — a SCIM connection is the actor of
its writes (#30960)')`.
- The create, the ban update and the delete with `session.actor:
'svc:scim:okta-30960'`: `actor` is the label, `user_id` is null,
`metadata` is exactly `{"scim_connection_id":"okta-30960"}`.
- `svc:cloud-control`, `svc:flow:mirror_status` and a bare `svc:scim`:
actor recorded, no `metadata`.
- A user's write beside a SCIM label: the user is the actor and there is
no SCIM key.
**Ablations** (`node scripts/ablation-replace.mjs`, anchor hit 1 → 0,
run, restore proven blob == HEAD and `git diff HEAD` empty;
plugin-auth's and plugin-audit's tests import the edited modules from
`src/`, so no rebuild sits between the mutation and the run):
| Leg | Mutation | Red |
|:--|:--|:--|
| A1 | the verifier does not record the connection | (e) (f) (h) |
| A1b | the verifier records with `scimRequestScope.enterWith(...)`, the
rejected design | (e) (f) (h): the writes never see it |
| A2 | the bearer recorded as the connection id | (e) (f) (h) |
| A3 | a process-wide slot instead of the per-request store | (g) (h) |
| A5 | the `scim_connection_id` spread deleted | the three SCIM row
cases |
| A6 | any `svc:` label read as a SCIM connection | the other-principal
case |
Two first attempts did not apply as written. A5's empty replacement was
refused (the tool asks for `--delete`) and ran nothing. A3's held half
was refused because its replacement contained its own anchor, so only
the reading half landed and that run stayed green while testing nothing.
Both were re-run with corrected anchors, and the table reports the
re-runs. The ablations ran at `4cd757af`, before the cookie case was
dropped (see the acceptance notes); under A1, A1b and A2 that case also
went red, and the cases the table names are the ones still in the file.
A4 is in the acceptance notes.
## Acceptance notes
- **Why the slot reaches every write.** The verifier runs in a
continuation the endpoint handler does not descend from: the `enterWith`
note in `scim-connection-service.ts`, measured on 1.7.2, which this PR
does not re-open. A field written on the store that `handleRequest`'s
`run(...)` opened is visible to both frames, because both run inside
that one `run(...)`. Each request has its own object, so requests do not
share it. A1b is the measurement: recording through `enterWith` turns
(e), (f) and (h) red. The template is the existing (a)–(d) measurement
in the same file.
- **The actor form, `svc:scim:CONNECTION_ID`.** It follows the incumbent
`svc:KIND:NAME` shape (`svc:flow:FLOW_NAME`, ADR-0014 D2), as the triage
audit rules. It uses the connection id, not a name, because the id is
immutable and keys `sys_scim_connection_binding`, `sys_scim_user` and
the credential row. The credential row has no connection name; its
`label` names one credential, which a rotation replaces.
- **ADR tension, recorded and not resolved here.** ADR-0118 D5 says the
actor dimension is two-valued and that "which automation" is answered by
an association field. `ExecutionContext.performedBy`'s doc reads that as
keeping `sys_audit_log.actor` to a user id or null. The incumbent
`svc:flow:` label and `sys_audit_log.actor`'s own description ("a user
id, svc:NAME, or null") put service principals on `actor`. This PR
follows the triage ruling (the incumbent) and also writes D5's
association field, `metadata.scim_connection_id`.
- **Why the metadata key is read off the label.** A hook receives only
`session.actor` and the closed `provenance` copy list that
`@objectstack/objectql`'s `buildProvenance` builds. A typed carrier for
the connection id would need a key on `ExecutionContext` and
`HookContext.provenance` in `packages/spec` plus a line in objectql, and
both are outside this card's file surface. So the plugin-audit half
reads the one channel that already reaches it, and the column and the
key cannot disagree. The cost is that the `svc:scim:` prefix is spelled
in both packages (plugin-audit does not depend on plugin-auth). The two
test files' literals hold the spellings equal. The plugin-audit file
surface is used as the claim allows ("only if the row writer must read a
new context field"). The writer reads no new field. It reads the
existing `session.actor`, and its `metadata` block gains one key.
- **`Clause-②: yes (widening)`, against the claim's provisional `no`.**
Re-derived from the diff, by the criterion 「本卡放宽接受集或扩大公开面吗」.
`authSystemWriteContext` is published (`export * from
'./auth-actor-attribution.js'`), and the built
`packages/plugins/plugin-auth/dist/index.d.ts` declares its return type
with the new `actor?: string` member, which widens the public surface.
The SCIM helpers are not on the published surface (0 hits in
`dist/index.d.ts`), and neither is plugin-audit's new module (0 hits).
Nothing is narrowed. The changeset level is `patch` (`Metadata-protocol:
none`).
- **The one row that stays null.** The `sys_oauth_resource` create is
written when the first request builds the better-auth instance, before
any bearer is verified. It is the platform's boot write, not a
provisioning one. The pins warm the instance with a SCIM read first, for
that reason.
- **`sys_member`.** `reconcile-membership.ts` builds its write context
with the same `authSystemWriteContext()`, so a SCIM create that binds a
membership names the connection. It is not driven here, because the rig
assigns no organization.
- **A case removed as vacuous.** A SCIM request that also carries a
signed-in human's session cookie was pinned to credit no human. A4, the
ablation that let the human win, stayed GREEN: on a SCIM request the
session resolver credits no human even with a cookie. The SCIM-first
ordering in `authSystemWriteContext` is therefore not observable through
the door, and the case was deleted rather than kept as a pin that cannot
fail. The ordering stays: the bearer is the request's only principal,
and it saves a session resolution per write.
- **Length bound, not measured on PostgreSQL.** `sys_audit_log.actor` is
`maxLength: 255` (driver-sql makes it a `varchar(255)`), and so is
`sys_scim_connection_credential.connection_id`. A connection id longer
than 246 characters gives a label longer than 255. SQLite stores it. On
PostgreSQL the ledger insert would be refused and the row lost through
the writer's swallow-and-report path. Which connection ids are accepted
is the mint door's decision (#30958, seat 3), where a bound of 246 would
close it.
- **Activity timeline.** The `sys_activity` mirror row of these writes
still shows the system (`actor_id` null). The card is about
`sys_audit_log`, so this is noted and not changed.
- **Serial with #30958.** No PR existed for #30958 before this branch's
first push or at opening. `origin/main` was merged at `4876c8cf` (it
brought #30879's plugin-auth changes, and nothing conflicted).
## Verification
All at `727dacbe`, the head this PR opens with (`origin/main` `1aeeafd9`
merged in at `4876c8cf`; `origin/main` has since moved 11 commits, none
touching `packages/plugins/plugin-auth` or
`packages/plugins/plugin-audit`, so it is not merged again).
- `pnpm --filter @objectstack/plugin-auth test`: `Test Files 229 passed
(229)`, `Tests 4151 passed | 10 skipped (4161)` (lock `VERDICT
command-exit 0`).
- `pnpm --filter @objectstack/plugin-audit test`: `Test Files 75 passed
(75)`, `Tests 1181 passed (1181)` (lock `VERDICT command-exit 0`).
- `pnpm --filter @objectstack/plugin-auth typecheck` and `pnpm --filter
@objectstack/plugin-audit typecheck`: exit 0 (`check:test-typecheck: OK`
for both).
- The reproduction above, re-run on `ad43453b` (`727dacbe` adds one
comment): 20 of 21 rows named, the bearer and its digest absent from
every row.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 65 commands; all ran, plus `pnpm
lint`, `pnpm check:cli-command-ids` and `pnpm check:adr-symbol-anchors`:
all exit 0. Three first exited 3 (PREREQUISITE NOT MET):
`check-plugin-teardown-shape.mjs --self-test` on a shallow clone, exit 0
after `git fetch --unshallow origin main`; `check:dual-build-cjs-loads`
and `check:i18n` with no whole-tree `dist/`, exit 0 after `pnpm exec
turbo run build --concurrency=2` (lock `VERDICT command-exit 0`, 119/119
tasks). `dispatch-gates.mjs --ran`: exit 0, "65 derived famil(ies)
accounted for — 65 run, 0 NOT-MEASURED".
- `node scripts/check-commit-card-trailers.mjs --range
origin/main..HEAD`: exit 0.
- Built `packages/plugins/plugin-auth/dist/index.d.ts`:
`authSystemWriteContext()` returns a Promise of `{ isSystem: true;
actor?: string; … }`; `scimRequestConnectionId`,
`recordScimRequestConnection`, `scimServicePrincipal`: 0 hits. Built
`packages/plugins/plugin-audit/dist/index.d.ts`: `scimConnectionOf`,
`SCIM_CONNECTION_ID_KEY`: 0 hits.
## Human Minimum
- [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic.
(`packages/spec` untouched.)
- [x] 2. Every name I added uses the casing for its kind.
- [x] 3. No formatter ran; `pnpm lint` passes.
- [x] 4. One issue, one branch off an up-to-date `main`, one pull
request.
- [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.
- [x] The *Before Submitting* list in CONTRIBUTING.md is done.
---
_Generated by [Claude
Code](https://claude.ai/code/session_016WV4cyRYCwPvYJ68qTXzEP)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent d5106ae commit d463b99
9 files changed
Lines changed: 508 additions & 5 deletions
File tree
- .changeset
- packages/plugins
- plugin-audit/src
- plugin-auth/src
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2997 | 2997 | | |
2998 | 2998 | | |
2999 | 2999 | | |
| 3000 | + | |
| 3001 | + | |
| 3002 | + | |
| 3003 | + | |
| 3004 | + | |
| 3005 | + | |
| 3006 | + | |
| 3007 | + | |
| 3008 | + | |
| 3009 | + | |
| 3010 | + | |
| 3011 | + | |
| 3012 | + | |
| 3013 | + | |
| 3014 | + | |
| 3015 | + | |
| 3016 | + | |
| 3017 | + | |
| 3018 | + | |
| 3019 | + | |
| 3020 | + | |
| 3021 | + | |
| 3022 | + | |
| 3023 | + | |
| 3024 | + | |
| 3025 | + | |
| 3026 | + | |
| 3027 | + | |
| 3028 | + | |
| 3029 | + | |
| 3030 | + | |
| 3031 | + | |
| 3032 | + | |
| 3033 | + | |
| 3034 | + | |
| 3035 | + | |
| 3036 | + | |
| 3037 | + | |
| 3038 | + | |
| 3039 | + | |
| 3040 | + | |
| 3041 | + | |
| 3042 | + | |
| 3043 | + | |
| 3044 | + | |
| 3045 | + | |
| 3046 | + | |
| 3047 | + | |
| 3048 | + | |
| 3049 | + | |
| 3050 | + | |
| 3051 | + | |
| 3052 | + | |
| 3053 | + | |
| 3054 | + | |
| 3055 | + | |
| 3056 | + | |
| 3057 | + | |
| 3058 | + | |
| 3059 | + | |
| 3060 | + | |
| 3061 | + | |
| 3062 | + | |
| 3063 | + | |
| 3064 | + | |
| 3065 | + | |
| 3066 | + | |
| 3067 | + | |
| 3068 | + | |
| 3069 | + | |
| 3070 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
91 | 91 | | |
92 | 92 | | |
93 | 93 | | |
| 94 | + | |
94 | 95 | | |
95 | 96 | | |
96 | 97 | | |
| |||
2105 | 2106 | | |
2106 | 2107 | | |
2107 | 2108 | | |
| 2109 | + | |
| 2110 | + | |
| 2111 | + | |
| 2112 | + | |
| 2113 | + | |
2108 | 2114 | | |
2109 | 2115 | | |
2110 | 2116 | | |
| |||
2227 | 2233 | | |
2228 | 2234 | | |
2229 | 2235 | | |
2230 | | - | |
| 2236 | + | |
| 2237 | + | |
| 2238 | + | |
| 2239 | + | |
| 2240 | + | |
| 2241 | + | |
2231 | 2242 | | |
2232 | 2243 | | |
2233 | 2244 | | |
2234 | 2245 | | |
2235 | 2246 | | |
2236 | 2247 | | |
| 2248 | + | |
2237 | 2249 | | |
2238 | 2250 | | |
2239 | 2251 | | |
| |||
| 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 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
61 | 61 | | |
62 | 62 | | |
63 | 63 | | |
| 64 | + | |
64 | 65 | | |
65 | 66 | | |
66 | 67 | | |
| |||
212 | 213 | | |
213 | 214 | | |
214 | 215 | | |
| 216 | + | |
215 | 217 | | |
216 | 218 | | |
217 | 219 | | |
218 | 220 | | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
219 | 231 | | |
220 | 232 | | |
221 | 233 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
793 | 793 | | |
794 | 794 | | |
795 | 795 | | |
796 | | - | |
| 796 | + | |
| 797 | + | |
| 798 | + | |
797 | 799 | | |
798 | 800 | | |
799 | 801 | | |
| |||
811 | 813 | | |
812 | 814 | | |
813 | 815 | | |
814 | | - | |
| 816 | + | |
| 817 | + | |
| 818 | + | |
| 819 | + | |
| 820 | + | |
| 821 | + | |
| 822 | + | |
| 823 | + | |
| 824 | + | |
815 | 825 | | |
816 | 826 | | |
817 | 827 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
754 | 754 | | |
755 | 755 | | |
756 | 756 | | |
| 757 | + | |
| 758 | + | |
| 759 | + | |
757 | 760 | | |
758 | 761 | | |
759 | 762 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
77 | 80 | | |
78 | | - | |
| 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 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
79 | 114 | | |
80 | 115 | | |
81 | 116 | | |
82 | 117 | | |
83 | 118 | | |
84 | 119 | | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
85 | 164 | | |
86 | 165 | | |
87 | 166 | | |
| |||
0 commit comments