Repository navigation
fix(plugin-auth): disabling an OAuth client refuses its already-issued tokens on the next call, and an administrator's API-key suspension holds against the owner - #31622
Conversation
Claude-Session: https://claude.ai/code/session_01QUs3BuLtRmSkjNPDYrwA9T Co-authored-by: Claude <noreply@anthropic.com>
… API key holds against its owner Claude-Session: https://claude.ai/code/session_01QUs3BuLtRmSkjNPDYrwA9T Co-authored-by: Claude <noreply@anthropic.com>
…he changeset Claude-Session: https://claude.ai/code/session_01QUs3BuLtRmSkjNPDYrwA9T Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 3 package(s): 35 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 51 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 80bd44b9796cf10147afb3360855512c6ae3531a && git checkout 80bd44b9796cf10147afb3360855512c6ae3531a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin dced781f8720643deb63d96d2f901452aceadf78 6beaf69cf09607f6f33ce629d6c4dedd886b7085 && git checkout -B drift-repro dced781f8720643deb63d96d2f901452aceadf78 && git merge --no-ff 6beaf69cf09607f6f33ce629d6c4dedd886b7085
node scripts/docs-audit/affected-docs.mjs --json dced781f8720643deb63d96d2f901452aceadf78
|
…arts at Claude-Session: https://claude.ai/code/session_01QUs3BuLtRmSkjNPDYrwA9T Co-authored-by: Claude <noreply@anthropic.com>
…administrator's key suspension Claude-Session: https://claude.ai/code/session_01QUs3BuLtRmSkjNPDYrwA9T Co-authored-by: Claude <noreply@anthropic.com>
⛔ merge queue 构建失败 — 先分诊,再决定要不要重排队列构建 42507456241 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集), 分类: 失败的 job(日志抽取,best effort):
跨 PR 相同签名(24h,按失败测试文件聚合):
历史信号:
分诊清单:
Generated by Claude Code · merge-queue-triage workflow (#4859) |
…the inherited admin cookie (objectstack-ai#31871) <!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi96VGP7vMPA9C54zyjz2LMch)_ Fixes objectstack-ai#31827 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:夜间 live 层的成员夹具改为只用成员自己的凭据发 API 请求(成员自己的会话 cookie 加它的 Bearer),成员登录也从空状态开始;`read-only-member-record.spec.ts` 只换成调用新助手,断言一条未动。 - 为什么改:在 `@playwright/test` 里,不指定 `storageState` 新建的请求上下文会继承配置里的管理员 cookie,于是成员请求同时带着管理员 cookie 和成员 Bearer。objectstack-ai#31469 起服务端拒绝这种「两个账号」的请求,数据路由回 401,夜间层因此变红。席位裁定:改夹具,不改产品。 - 风险与回滚:只动 `ui/e2e/live/` 下两个测试文件,不发布任何包,不动 `packages/`。回滚即 revert。 - 先看:`ui/e2e/live/member.ts` 的 `memberRequestContext` 和模块注释里的 ⛔ 段。 - 两分钟验证:起后端后跑 `OS_TEST_TIERS=nightly pnpm --filter @objectstack/example-showcase exec vitest run test/console-live-specs.live.test.ts`,预期 `28 passed`。 - 需要你做的:无,席位落地。 ## What changes - `ui/e2e/live/member.ts` gains `memberRequestContext(member)`: a request context carrying the member's own session cookie and its Bearer, with its `storageState` named so the config's administrator cookie is not inherited. - The member's sign-in context starts from an empty `storageState` (`{ cookies: [], origins: [] }`) instead of the inherited administrator cookie, so the sign-in request names the member only. - `read-only-member-record.spec.ts` builds its premise context with the helper. That is its only change: the `playwright` fixture it no longer uses leaves the destructuring, and no assertion moves. - The module comment states the trap: inside `@playwright/test`, `playwright.request.newContext()` and the imported `request.newContext()` both start with the config's `use.storageState`. No test was skipped, renamed or loosened. No change under `packages/`. ## Why The nightly live tier went red at `6be71abc` on `read-only-member-record.spec.ts:48`, premise step: the member's `GET /api/v1/data/showcase_inquiry/<id>` answered 401 where 200 was expected. Step 0 (seat ruling 6735238125): - **Bisect.** Each commit measured on a fresh backend booted by `ui/e2e/live/ci/start-backend.sh`, with the spec's request shape: | Commit | Result | |:--|:--| | `44a030bf` (last green nightly) | green, read 200 / write 403 | | `133f75aec0~1` | green, 200 / 403 | | `133f75aec0` objectstack-ai#31469 | **red**, 401 / 401 | | `ed4b83226b` objectstack-ai#31443 | red | | `59630d9357` objectstack-ai#31622 | red | | `8c9260f79c` objectstack-ai#31657 | red | | `f127b04e07` objectstack-ai#31686 | red | | `6be71abc` (red nightly) | red | - **Cause.** The member's context was `playwright.request.newContext({ extraHTTPHeaders: { Authorization: 'Bearer <member>' } })`. Inside `@playwright/test` that context inherits the config's `use.storageState`, which is the global setup's administrator cookie. Measured: the context's cookie equals the administrator's. So each premise request carried two accounts' credentials. Before objectstack-ai#31469 the bearer won. Since objectstack-ai#31469 the session read refuses such a request with `CREDENTIALS_NAME_DIFFERENT_ACCOUNTS`, and the data route answers its own anonymous 401. objectstack-ai#31469's acceptance notes describe exactly that data-route answer, so the product behaves as ruled and the fixture relied on what objectstack-ai#31469 ruled out. ## Sweep (owed by the ruling) Every live spec and helper under `ui/e2e/live/` that builds a request context or sends a credential: | Site | Accounts it sends | Action | |:--|:--|:--| | `read-only-member-record.spec.ts` premise context | admin cookie (inherited) + member Bearer | **moved onto `memberRequestContext`** | | `member.ts` sign-in context (`request.newContext()`) | admin cookie (inherited) on the member's sign-in | **moved onto an empty `storageState`** | | `read-only-member-record.spec.ts` member browser context | member `storageState` named explicitly | one account, unchanged | | `global-setup.ts` sign-in context | none (global setup runs before any `use` state exists) | one account, unchanged | | `inline-edit-polish-2572.spec.ts` `request` + Bearer | admin cookie + admin token from the same `state.json` | one account, unchanged | | `studio-customize-journey.spec.ts` `request` override | `storageState` passed by name (admin), no other credential | one account, unchanged | | `studio-object-designer.spec.ts` in-page `fetch` | admin page's own cookie + its own localStorage token | one account, unchanged | | every other spec's `request.get/post/…` (cascading-options, grid-file-upload, list-row-action-cel, master-detail, record-form-direct-url, record-history-display) | admin `request` fixture alone | one account, unchanged | Measured for the two inherited sites: the imported `request.newContext()` and `playwright.request.newContext()` each start with 1 cookie under this config; with `storageState: { cookies: [], origins: [] }` they start with 0. ## Verification All heavy runs went through `scripts/pm/os-verify-lock.sh`. Head `86082eb9b5`, backend closure and `pnpm console:build` rebuilt at the head first. - **Nightly live file, head:** `OS_TEST_TIERS=nightly pnpm exec vitest run test/console-live-specs.live.test.ts` in `examples/app-showcase`: Playwright `28 passed (1.3m)`, vitest `Test Files 1 passed (1)`, `Tests 1 passed (1)`. - **Ablation:** `scripts/ablation-replace.mjs` deletes the helper's `storageState` line in `member.ts` (anchor 1 → 0, blob `d4e1a39d33f5` → `dc6c2ff0d686`), so the inherited administrator cookie is back. The same run: `✘ read-only-member-record.spec.ts:48`, `Expected: 200`, `Received: 401`, `Test Files 1 failed (1)`. Restored: blob equals HEAD's (`d4e1a39d33f5`), `git diff HEAD` empty, `git status --porcelain` empty. - **Before the fix, on main `3487ccc1`:** the same nightly run was red, 1 of 28, the same 401. - **Dispatch gates:** see the os-dev-report on objectstack-ai#31827 for each command's exit code and the `--ran` reconciliation; `pnpm lint` and `eslint` over the two touched files are reported there too. No changeset: the diff publishes nothing (`ui/e2e/live/` only), so this PR carries `skip-changeset`. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No `packages/spec` change.) - [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. (Publishes nothing; `skip-changeset`.) - [x] 6. `packages/spec` touched: n/a. - [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. (The 维护者速读 section is the seat's required Chinese summary.) - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01ADHkmPpwZNVKwJLQGotdJy --- _Generated by [Claude Code](https://claude.ai/code/session_01ADHkmPpwZNVKwJLQGotdJy)_ Co-authored-by: Claude <noreply@anthropic.com>
Requested by Sam · project thread
Closes #31606
Clause-②: yes (widening)
Metadata-protocol: none
维护者速读
packages/plugins/plugin-auth/src/mcp-token-revocation.ts(mcpConsentAdmits),packages/plugins/plugin-auth/src/api-key-restore-guard.ts(stampSuspension)。pnpm --filter @objectstack/plugin-auth exec vitest run src/mcp-token-revocation.test.ts src/api-key-restore-guard.test.ts,预期全部通过,其中新增的 card 2 用例共 12 个。What changes
OAuth clients: a disabled client's live tokens. The MCP door's per-call grant read (
mcpConsentAdmits,mcp-token-revocation.ts) now reads the client row first, through the same better-auth adapter it reads the consent through. A token is refused (the verifier answersnull, so the door answers the existing401) when the client is disabled, when its row is gone, or when the read fails. A server-onlyskip_consentclient's flag is read off that same row, so the consent-less path costs no extra read. The provider already refused a disabled client new tokens (invalid_clientat the token endpoint,client_disabledat authorize, inactive at introspection) by the same truthy test on the same column; only this door read none of it. Disabling is a pause, not an end: the consent and the refresh family stay, so enabling the client again serves the same grant. Ending a grant stays the withdraw or/oauth2/revoke. Cost: two indexed reads per MCP call (client by uniqueclient_id, then consent) instead of one.API keys: an administrator's stop holds against the owner. This corrects the card's premise (agreed in the cloud#3820 PM thread):
sys_api_key.revokedis not terminal. The Revoke API Key / Restore API Key row actions already make it reversible, and onlyorganization_membership_endedmakes it final. The real gap was that an owner could restore a key a platform administrator had revoked the moment it was revoked. So, per the ruling in that thread (option A, no new column, no new state):revoked_reasongains a third value,administrator_suspended(sys-api-key.object.ts, label and description translated in all nine shipped locales).api-key-restore-guard.ts, rule 4) stamps it on a user-context revoke by a caller with platform-admin standing who is not the row'suser_id, when the row carries no reason yet. A host that stops a key itself (cloud's console, in the system context) writes the same value besiderevoked: true; system writes are never stamped or refused.403 PERMISSION_DENIED, naming the reason and both roads. Restore clears the reason, as before, and the same key works with no re-issue.organization_membership_endedstays final.400 VALIDATION_ERROR, in either row order, before the driver writes. A batch of plain keys is stamped on every row. Over-stamping an administrator's own key in such a batch is harmless, because the administrator can restore it.403 API_KEY_REVOKEDtext (@objectstack/coreapi-key.ts) gets an arm that names a platform administrator as the only restorer.Step-1 measurement (before the fix)
Real routes, in-process HTTP, on objectstack
158cc2aa(main when measured): the showcase dogfood stack withMCPServerPlugin,OS_OIDC_PROVIDER_ENABLED=true. Flow: DCR public client, sign-in, authorize (PKCE,resource= the MCP URL), consent, code exchange.OAuth client disable: the old token kept working
expires_in3600,exp − iat= 3600 sPOST /mcpinitialize, before the disablePOST /auth/admin/oauth2/toggle-disabled {disabled: true}as the platform adminPOST /mcpinitialize, same tokenrefresh_tokengrant while disabledinvalid_client"client is disabled"API key: the owner undid an administrator's revoke
POST /api/v1/keys), key worksPATCH /data/sys_api_key/{id} {revoked: true}API_KEY_REVOKEDPATCH /data/sys_api_key/{id} {revoked: false}Tests
Two commits after this section was written seed a client row in three existing verifier test files (26 cases that read no client row) and update three docs pages; the os-dev-report on #31606 covers both.
New pins, each as a top-level
describein that component's existing harness file:plugin-authmcp-token-revocation.test.ts, on the realAuthManagerand the installed provider (4 cases): a token minted before the disable is refused, its refresh is refused by the provider, the grant rows stay, and once enabled again the same token and a refreshed one verify; a disabledskip_consentclient is refused; a token whose client row is gone is refused; a store error in the client read refuses. All 4 red on the old verifier (restored fromorigin/mainwhile the tests stayed):expected { …(3) } to be null.plugin-authapi-key-restore-guard.test.ts, on the real engine,SecurityPlugin, D2 guard and key door (8 cases): the stamp, the owner's 403, the admin's restore and the key working again; CONTROL: an admin's own key gets no reason; an owner-revoked key re-revoked by an admin is stamped; an owner's re-revoke and smuggled clear keep it; an admin's revoke keepsorganization_membership_endedandadmin_password_reset; a host's system suspension holds; a plain multi-row revoke stamps every row; a mixed multi-row revoke is refused whole in both orders. 6 of 8 red on the old guard. The 2 that stay green are the controls that hold today too.plugin-authforced-reset-revoke-reason-vocabulary.test.ts: the option list is exactly the three values, and the description names the new one.coreapi-key-lifecycle.test.ts: the door text foradministrator_suspendednames only a platform administrator.dogfoodauthorized-client-withdraw.dogfood.test.ts, over HTTP: a member's toggle is refused 403 and moves nothing; the admin's disable makes the same bearer 401 at/mcpand its refresh400 invalid_client, while the consent stays; enabled again, the same token, a refreshed token and a new person's consent on the same client are all served. Red against the oldplugin-authbuild (expected 200 to be 401), green after rebuilding.dogfoodapi-key-owner-revoke.dogfood.test.ts, over HTTP: the admin'srevoke_api_keyPATCH refuses the member's key on its next call and storesadministrator_suspended; the member'srestore_api_keyPATCH is 403PERMISSION_DENIED; the admin's restore brings the same key back and clears the reason; CONTROL: the member's own revoke and restore work as before.Both dogfood files had their boot and helpers hoisted from inside the existing
describeto module scope, so the newdescribeshares the one showcase boot.git diff -wshows that move as a few lines.Acceptance notes
patchfor all three packages, precedent objectstack#31435) are at the top of this body.API_KEY_REVOKED, and the restore refusalPERMISSION_DENIED. The mixed-batch refusal isVALIDATION_ERROR, reachable only from an in-process multi-row user-context write, becausesys_api_keyhas no batch route.hasPlatformAdminStanding) who is not the row's owner. The shipped permission sets let no one else write another person's key (sys_api_key_self), so in a shipped composition that is every non-owner revoke.Verification
See the os-dev-report on the card for package suites, typecheck and the dispatch-gate sweep at the final head.
Human Minimum
packages/specholds no runtime logic. (Nopackages/specchange.)pnpm lintpasses.main, one pull request.packages/spectouched: n/a.content/docs/references/,packages/*/CHANGELOG.mdorcontent/docs/releases/.Generated by Claude Code