Skip to content

Commit 05cc3ee

Browse files
refactor(plugin-auth): move AuthManager's MCP access-token verifier into auth-manager-mcp-access-token.ts (#25224 cut 9) (#25461)
<!-- claude-projects-attribution: {"github_login":"os-sales"} --> _Requested by **Sales** · [project thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuMjbPDrsigsyqoKsV4WZQMz)_ Part of #25224 Clause-②: no Metadata-protocol: none Ruling-ref: 6348891160, 6364006498 The basis for the `Clause-②: no` line: this is a move. No accept set, route, refusal, code or answer changes, and the published surface is measured unchanged at both commits: - `@objectstack/plugin-auth`'s `dist/index.d.ts` and `dist/index.d.mts` are raw byte-identical to base at commit 1 and at head (0 diff lines, so the union-order normalisation has nothing to do). The positive control is the one ruling `6348891160` asks for. - Its ESM and CJS entries export the same 159 names at base, at commit 1 and at head. The `.d.ts` bar is ruling `6348891160` item 1 (on #25222, covering this card). Ruling `6364006498` is Sales's ruling A on this card, for bodies that read shared unexported helpers. This is the first cut it applies to. Cut 9 of the `auth-manager.ts` split: **the MCP access-token verifier.** Two `AuthManager` method bodies (94 lines) and the one module-level declaration only they read (11 lines) move unedited, re-indented one level, into one new module, `packages/plugins/plugin-auth/src/auth-manager-mcp-access-token.ts`. #25224 stays open for the later cuts. This follows the amended claim `6367866058`, which amends `6365318480` on the file cut 8 left (#25423, landed note `6366862591`). The shape is cuts 1 to 8's, which is #23050's. Branch: `claude/project-thread-lg3wrj`, the harness-pinned branch for this session, restarted from `origin/main` `2b7c331537` after #25423 merged. It names no issue, so the card relation is this body's first line. `auth-manager.ts` goes from 8,790 to 8,711 lines. The cut is +247 / −112 over 4 files: 359 changed lines, under the 3,000-line line. It is not a governed surface (`check-governed-merges.mjs --branch HEAD`: 0 of the 4 paths hit the register). There are two commits, so the `index.ts` change is measured by itself: - `b048a93241`: `index.ts` names `auth-manager.ts`'s published exports, and one comment sentence is reworded; - `619494870b`: the move. ## Commit 1: `index.ts` names what it publishes from `auth-manager.ts` Ruling `6364006498`, parameter 1. `index.ts:16` was `export * from './auth-manager.js';`, so any module-level helper exported from `auth-manager.ts` for a module split out of it would have been published. It is now an explicit `export { … } from './auth-manager.js';` of the 14 names `auth-manager.ts` exports at base, in declaration order: `DEFAULT_AUTH_BASE_PATH`, `readMcpServerEnabledEnv`, `resolveOidcProviderEnabled`, `resolveDcrEnabled`, `resolveScimEnabled`, `assertScimAdminCoherence`, `isOAuthEligibleBaseUrl`, `type InvitationPlacementServiceLike`, `readInvitationPlacementIntent`, `type AuthManagerOptions`, `ipMatchesRange`, `normalizeAuthEmailLocale`, `authEmailLocaleFromRequest`, `AuthManager`. A four-line comment above it says a new export of `auth-manager.ts` is published only by adding it there. The list was read off the base file with the TypeScript compiler API: 14 exported declarations, exactly these, in this order. At head the file has 15, and the fifteenth, `OAUTH_ORGANIZATION_CLAIM` (commit 2), is not listed. **One comment-only reword, agreed with the seat.** Cut 5's module header (`auth-manager-email-sms-delivery.ts:45`–`:49`) said "`index.ts` re-exports `auth-manager.ts` whole, so exporting them from there would add them to the published surface, and a value import of that file from here would be a cycle." This commit makes the first half false, and the PR would otherwise ship a statement it makes false. The sentence now reads: > The four module-level declarations move with the bodies rather than being imported back, because only these bodies read them. (`index.ts` lists the names it publishes from `auth-manager.ts` one by one, so a helper exported there in place for a module split out of it stays off the published surface.) Nothing re-exports this module, so the published surface does not move. - The cycle half is dropped. A value import of `auth-manager.ts` from a split module is a cycle, but cuts 8 and 9 take one by design, read only inside function bodies, so it is no longer a reason. "Only these bodies read them" is measured: none of the four names (`SMS_QUOTA_EXCEEDED_CODE`, `isSmsQuotaRefusal`, `smsQuotaExceededApiError`, `PHONE_OTP_SEND_REFUSAL_CODE`) has a reader in `auth-manager.ts`. - Neither the old nor the new wording cites an `ADR-NNNN`, with or without a decision letter: 0 and 0. - The file's token sequence is unchanged: 1,427 tokens before and after, compared with the template-aware scanner the move proof uses (trivia skipped), and a comment-free print of the file is identical. Negative control: one string token changed (`'TOO_MANY_REQUESTS'` → `'TOO_MANY_REQUEST'`) makes both comparisons fail, exit 1. **What commit 1 changes in the build**, measured at `b048a93241`: - `dist/index.d.ts` and `dist/index.d.mts` are raw byte-identical to base; - the ESM and CJS entries export the same 159 names; - `index.js` and `index.mjs` are byte-identical to base. Only `index.js.map` and `index.mjs.map` move, because `index.ts`'s lines moved. ## Commit 2: the move | moved | base line | lines | members it reads | |---|---:|---:|---| | `verifyMcpAccessToken` (public) | `:7228` | 74 | `getApi`, `getAuthIssuer`, `getMcpResourceUrl`, `isMcpOAuthEnabled`, `rereadKeyringForUnknownKey` | | `rereadKeyringForUnknownKey` (private) | `:7321` | 20 | `config`, and `unknownKeyReread` (read and written) | | `readPublishedJwks` (module-level, with its docblock) | `:1483`–`:1493` | 11 | — | Lines are base `2b7c331537`. The members come from a walk of every `this.X` in each body with the TypeScript compiler API. No body has a bare `this`, `this[...]`, `arguments` or `super`, or a `this` inside a `function` or a nested class. The one member a body assigns is `unknownKeyReread` (`rereadKeyringForUnknownKey`). - **`verifyMcpAccessToken`** is the local, fail-closed check of an OAuth 2.1 Bearer access token this deployment's authorization server minted: the signature against the published key set, `iss`, `aud`, the refusal of a token no human delegated, and the organization claim. - **`rereadKeyringForUnknownKey`** is the one forced keyring re-read a token signed by an unknown key earns, single-flight and floored at `UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS`. - **`readPublishedJwks` moves with them, unexported.** It reads the key set `/jwks` publishes. Its only readers are the two moved bodies (`:7238`, `:7329`). No anchor anywhere in the tree names it, and no test reads it. Its docblock cites `[#24186]` and no ADR. **`OAUTH_ORGANIZATION_CLAIM` is exported in place** (ruling `6364006498`, parameter 2): - `:1465` becomes `export const OAUTH_ORGANIZATION_CLAIM = 'organization_id';`. That is the only edit to text that stays. - It does not move. `buildPluginList` (`:4398`, which stays) mints the claim with it, and `verifyMcpAccessToken` (`:7288`) reads it back; its docblock calls it "one spelling for both ends". - The new module imports it by value from `./auth-manager.js`, beside `type AuthManager`. The one read is inside `verifyMcpAccessToken`'s body, when a token is verified. The module's top level is three imports, `readPublishedJwks`, the host interface and the two functions, so nothing reads the binding while the two modules are first evaluated. - `index.ts` (commit 1) does not list it, so it is not published. The head's ESM and CJS entries do not export it, and `dist/index.d.ts` does not name it. **Imports.** - `retireHeldKeyrings` and `UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS` (`./signing-key-read-cache.js`) leave `auth-manager.ts`, where their only reader was `rereadKeyringForUnknownKey`'s body, and that import line goes. The two docblock mentions of `UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS` that stay in the class are backticked text, not `{@link}`. - `type JSONWebKeySet` (`jose`) stays in `auth-manager.ts`, for the `unknownKeyReread` field and the `rereadKeyringForUnknownKey` signature. The new module imports it as well. - The `await import('jose')` inside `verifyMcpAccessToken` moves with the body. **Evaluation order.** `signing-key-read-cache.js` was first reached through `auth-manager.ts`'s own import line (`:146`). It is now first reached through `auth-manager-plugin-assembly.js`, which imports `createSigningKeyReader` from it (cut 6's import line, `:221` at head). Its top level is one type-only import, literal constants, a `Set`, a `WeakMap`, interfaces and function declarations. It imports no value and does nothing outside itself, so the change in order cannot be observed. ## The shape - **The class keeps both methods** at their places, with their names, signatures and docblocks: the published `.d.ts` lists each one. Each body is now a host literal of exactly the members that body reads, typed as a `Pick` of `McpAccessTokenHost`, plus `.call`. - **`McpAccessTokenHost`** lives in the new module. It names the 7 members the bodies read, each typed as the class member (`AuthManager['config']`, …). - **Every method goes as a late-bound closure** (`(...a) => this.m(...a)`), so a double or spy set on the manager is still the one a body reaches. The verifier suites replace `getApi` with a key-set double, and `verifyMcpAccessToken` asks `rereadKeyringForUnknownKey` through the manager rather than calling the function directly. - **`config` goes as a getter.** The class reassigns it after construction (`setRuntimeBaseUrl`, `applyConfigPatch`), so it is read off the manager when a body reads it, as before. - **`unknownKeyReread` goes as a get/set pair.** `rereadKeyringForUnknownKey` stores the re-read it starts on the manager, so the floor and the read in flight stay the manager's, shared by every token it verifies. - **`verifyMcpAccessToken` is no longer `async`.** It returns the moved async function's own promise, so a caller awaits the same promise the body settles. Building the host cannot throw, so it does not now throw synchronously where it used to reject. `rereadKeyringForUnknownKey` was not `async` and stays so. - **The cost:** one small host object per call, on MCP bearer-token verification only, not on every request. - **Nothing re-exports the new module**, so the published surface does not move. ## Path readers No path reader needs migrating: none reads a moved line. - **The tenant-audit census.** No census site moves: the moved text has no engine write, no `this.X.delete()` and no elevation read. `check-tenant-audit-census` exits 0 at head with no `--write`: 238 sites at base and at head, compared per site with line numbers ignored, 0 differences, and 2 of them in `auth-manager.ts` at both. So neither census document is edited. - **`check-dispatcher-error-vocabulary.mjs`** and **`api-error-code.pin.test.ts`'s `authManagerFiles` floor**: no `new APIError(` sits in the moved text. The gate reads 56 sites found and 56 classified at base and at head. - **The plugin-factory scans** in `better-auth-schema-parity.test.ts` and `managed-extension-fields.test.ts` match `await import('better-auth/plugins…')` and `await import('@better-auth/…')` only. The moved `await import('jose')` is neither, and the import lines that change are local modules. - **`check-auth-mount-ledger.mjs`** and **`auth-manager-base-path.test.ts`**: no base-path literal or declaration sits in the moved text. `getAuthIssuer` and `getMcpResourceUrl` stay on the class. - **The system-context census** reads 148 elevation read sites at base and at head. - **`scripts/adr-anchors/`**: the `auth-manager.ts` entry's invariant (ADR-0093, ADR-0108) is about the organization roles and the membership reconciler, not this text. - **Callers through the manager.** These suites drive the two methods through the class method, and all pass at head: - `auth-manager.mcp-oauth.test.ts` (local JWKS verification, fail-closed); - `auth-manager.mcp-verifier-keyring.test.ts` (the forced re-read, its floor and single flight; it reads `UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS` and `retireHeldKeyrings` from `signing-key-read-cache.js` itself); - `auth-manager.oauth-consent-organization.test.ts` (the organization claim end to end); - `auth-manager.mcp-oauth-resource.test.ts`. - **Every symbol anchor into the file.** Before the cut, every `auth-manager.ts#symbol` anchor in the tracked tree outside changelogs was mapped with `symbolSegmentResolution` from `scripts/symbol-anchors.mjs`. There are 29, and all 29 are `declaration`-class at base, with the moved text blanked, and at head. None names a moved method or `readPublishedJwks`. One names a token the moved text carries, and it resolves on the declaration that stays: `#isMcpOAuthEnabled` (`docs/qa/platform-checklist/areas/ai.json:806`; the moved verifier calls `this.isMcpOAuthEnabled()`). ## The decision-letter layer does not move The seat asks each cut's report for this count at both ends. `check-adr-anchors`' decision-letter layer reads 2,001 at base `2b7c331537` and 2,001 at head, and the gate exits 0 at both. Nothing this PR adds or moves cites an `ADR-NNNN`: not the moved text, not the new module's header, not the `index.ts` comment, and not the reworded sentence. So the number layer reads the same at both: 52,177 citations across 6,637 files. No anchor entry is added for the new module, as for cuts 1 to 8's modules. ## Proof Builds and suites ran through `os-verify-lock`, and every exit code was captured before any pipe. Base `2b7c331537`, commit 1 `b048a93241`, head `619494870b`. A full `turbo run build` ran first. Then `plugin-auth` was built and its suite run at base, built at commit 1, and built and its suite run at head, on the same dependency dists. **1. The moved text.** - For each of the 2 methods, the base class body and the new module's function body are the same TypeScript token sequence (scanner tokens, kind and text, trivia skipped, each template literal re-scanned at its substitutions as the parser does). - Every line is the base line minus exactly two leading spaces, and the parameters and return type are the same text. - Each class method keeps its docblock and signature, with `async` dropped from `verifyMcpAccessToken`. - `readPublishedJwks`'s 11 lines, docblock included, are verbatim in the module and absent from `auth-manager.ts`. Its 76 tokens are identical. - Result: 2 of 2 bodies and 1 of 1 declarations pass, 94 body lines plus 11. | moved | lines | tokens | |---|---:|---:| | `verifyMcpAccessToken` | 74 | 434 | | `rereadKeyringForUnknownKey` | 20 | 130 | | `readPublishedJwks` | 11 | 76 | - **Negative control:** one token changed in the module (`errors.JWKSNoMatchingKey` → `errors.JWKSNoMatchingKeys` in `verifyMcpAccessToken`) makes that method fail (tokens and lines DIFFER), and the run exits 1. The module was restored afterwards, and `cmp` against the saved copy was clean. - **An independent reading:** `git diff --color-moved=plain --color-moved-ws=allow-indentation-change b048a93 6194948` over the two source files marks 105 of the 107 deleted lines and 120 of the 222 added lines as moved. - The other 2 deleted lines are `const OAUTH_ORGANIZATION_CLAIM = …` (now `export const`) and the `async verifyMcpAccessToken(` signature line. - The other added lines are the stubs' host literals, `McpAccessTokenHost`, the module header, the per-function pointer docblocks and the import blocks. **2. `.d.ts` and exports (ruling `6348891160` items 1 and 2).** | file | base | commit 1 | head | raw diff lines vs base | |---|---|---|---|---:| | `dist/index.d.ts` raw | `12679e13f74973b0510551a65b50808e7c3936a2d273cf2f262a9196648c0737` | same | same | 0 and 0 | | `dist/index.d.ts` normalised | `c7a9fa25f118cfec858d770b979767744d27d9757a9ea4063faeea6238c33c01` | same | same | | | `dist/index.d.mts` raw | `df57562ff950159e738d790cb7ee82a655ba0fffddafe4153bdcc50c4537336f` | same | same | 0 and 0 | | `dist/index.d.mts` normalised | `68f824aea0947d9895b968ed46a7972eaae43ef0f2bda7e3ba9113002233ade6` | same | same | | - The normaliser, verbatim (the ruling's one-liner): `node -e 'const s=require("fs").readFileSync(process.argv[1],"utf8");process.stdout.write(s.replace(/"[^"\n]*"(?:\s*\|\s*"[^"\n]*")+/g,r=>r.split(/\s*\|\s*/).sort().join(" | ")))' dist/index.d.ts | sha256sum`. - The raw files are already byte-identical, so no union-order swap occurs in this cut: items 1(b) and 1(d) hold with nothing to account for. - **Positive control:** the head `dist/index.d.ts` with one private member made public (`private rereadKeyringForUnknownKey;` → `rereadKeyringForUnknownKey;`, line 3320, a one-line change) hashes raw `090e9f39924a2034bc496e4fded90d5f267d90ff88aa20a92619d04019ed471b` and normalised `dc9fc90b5a1699dcb01263f89e1c983da7367710ea2c0f6e17ec395a9d0da579`. Both differ from the readings above. - **Export lists:** ESM `dist/index.mjs` and CJS `dist/index.js` each have 159 names at base, at commit 1 and at head, the same list (the sha256 of the sorted names is `3abb126f5babb701…` on all six). `OAUTH_ORGANIZATION_CLAIM`, `readPublishedJwks` and the module's functions are not among them, and `dist/index.d.ts` names none of them. - **The bundle:** 18 files in `dist/` at all three, the same list. From base to head, only `index.js`, `index.mjs` and their two `.map` files differ, as a move of code inside the bundle must. From base to commit 1, only the two `.map` files differ. **3. Tests and lint.** - `pnpm exec vitest run --maxWorkers=3` in `packages/plugins/plugin-auth`: base `Test Files 189 passed (189)`, `Tests 3580 passed | 10 skipped (3590)`, exit 0. Head: the same counts, exit 0. - `pnpm --filter @objectstack/plugin-auth typecheck` (both `tsc` projects and `check:test-typecheck`): exit 0. `pnpm --filter @objectstack/runtime typecheck`: exit 0. - `eslint --no-inline-config` (the root `lint` script's binary and flags) over the 4 source files: exit 0. **4. The censuses and the anchors, at base and at head.** - The tenant-audit census, as above: 238 sites at both, 0 per-site differences, 0 unresolved and 0 unledgered at both, and every total the same. Only `scannedSources` (725 → 726, the new module) and `measuredAt` differ. - These checkers were run by hand at base and at head, because `dispatch-gates` does not derive the ADR one. All six exit 0 at both, with the same reading: - `check-adr-symbol-anchors`: 2,384 anchors across 142 records, 466 symbol (440 declaration, 26 literal); - `check-adr-anchors`: 80 anchored files; the decision-letter layer reads 2,001, as above; - `check-scripts-symbol-anchors`: 4,062 anchors; - `check-spec-docblock-symbol-anchors`: 5,075 anchors (both of these read in the same built tree); - `check-system-context-census`: 148 elevation read sites; - `check-platform-checklist`: 16 areas, 329 items. **5. Gates.** - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives 53 commands for the 4 paths. All 53 ran at `619494870b` after the full build, and all 53 exited 0. - `--ran` over the record with each exit code: 53 derived, 53 run, 0 NOT-MEASURED, a derived zero. - `check-governed-merges.mjs --branch HEAD`: NOT governed, 359 changed lines. - **Docs drift** (`node scripts/docs-audit/affected-docs.mjs --json 2b7c331`, exit 0): 1 page, `content/docs/kernel/contracts/auth-service.mdx`, which states the `verifyMcpAccessToken` contract signature. The class method keeps that signature, so no doc edit is owed. 0 release-owned pages. - **Serial constraints** (in the claim): 25 PRs are open, and none touches this cut's files or a reader above. `origin/main` moved to `b2c0752900` during the run with #25434, which touches `otp-send-guard.ts` and `rate-limit-storage.ts` and its test, none of them in this surface. `git merge-tree` of this branch onto `b2c0752900` is clean. ## Finding, outside this cut `scripts/measure-durability-swallow-family.mjs` is a measurement, not a gate. Its prose at `:625` and `:775` names `auth-manager.ts::verifyMcpAccessToken` as the one outstanding DARK member. The census does not list it on base: its one `auth-manager.ts` DARK member is `observeFirstMemberships`. So that prose was already stale before this cut. Its `--json` reading is the same at base and at head: 74 members at both, every member and quiet row the same (line numbers ignored), and every statistic the same except the scanned file count (3,449 → 3,450, the new module). Neither side names `verifyMcpAccessToken` or the new module. `scripts/` is not edited, and the finding is for the seat to judge. ## Changeset None: `skip-changeset`, per ruling `6348891160` item 4. The published declarations and export lists are unchanged, and the bundle's bytes move only because code moved inside it. --- _Generated by [Claude Code](https://claude.ai/code/session_01LSXBc2GCBwiAL7vV4kWkyr)_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LSXBc2GCBwiAL7vV4kWkyr --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent dd124e9 commit 05cc3ee

4 files changed

Lines changed: 247 additions & 112 deletions

File tree

‎packages/plugins/plugin-auth/src/auth-manager-email-sms-delivery.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,11 @@
4343
* manager holds.
4444
*
4545
* The four module-level declarations move with the bodies rather than being
46-
* imported back: `index.ts` re-exports `auth-manager.ts` whole, so exporting
47-
* them from there would add them to the published surface, and a value import
48-
* of that file from here would be a cycle. Nothing re-exports this module, so
49-
* the published surface does not move.
46+
* imported back, because only these bodies read them. (`index.ts` lists the
47+
* names it publishes from `auth-manager.ts` one by one, so a helper exported
48+
* there in place for a module split out of it stays off the published
49+
* surface.) Nothing re-exports this module, so the published surface does not
50+
* move.
5051
*/
5152

5253
import type { StandardErrorCode } from '@objectstack/spec/api';
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#25224 cut 9] The MCP access-token verifier of `AuthManager` — the local,
5+
* fail-closed check of an OAuth 2.1 Bearer access token this deployment's
6+
* authorization server minted, and the one forced keyring re-read a token
7+
* signed by an unknown key earns — moved out of `auth-manager.ts` whole, with
8+
* the one module-level declaration that only these bodies read: the reader of
9+
* the key set `/jwks` publishes.
10+
*
11+
* A MOVE, not a redesign. Each function below is the body of the
12+
* `AuthManager` method of the same name as it stood, re-indented one level
13+
* and otherwise unedited, and `readPublishedJwks` is its text as it stood in
14+
* `auth-manager.ts`: the same signature, issuer and audience checks, the same
15+
* refusal of a token no human delegated, the same organization claim, the
16+
* same single-flight re-read and its floor.
17+
*
18+
* ## How the moved bodies reach the class
19+
*
20+
* The shape #23050 set for `rest-server.ts` (its `data-action-routes.ts`
21+
* header carries the full reasoning), which `auth-manager-actor-guards.ts`,
22+
* `auth-manager-sign-in-lockout.ts`, `auth-manager-password-session-policy.ts`,
23+
* `auth-manager-database-hooks.ts`, `auth-manager-email-sms-delivery.ts`,
24+
* `auth-manager-plugin-assembly.ts`, `auth-manager-database-secret.ts` and
25+
* `auth-manager-membership-admission.ts` follow, applied per method. The class
26+
* keeps both methods — the published `dist/*.d.ts` lists each one — and each
27+
* body is one literal of exactly the {@link McpAccessTokenHost} members that
28+
* method reads, plus a `.call`. `verifyMcpAccessToken` is no longer `async`:
29+
* it returns the moved function's own promise, so a caller awaits the same
30+
* promise the body settles, with no extra wrapper.
31+
*
32+
* A method goes as a closure that calls it on the manager at call time, so a
33+
* test double or spy set on the manager is still the one a body reaches —
34+
* `getApi`, which the verifier suites replace with a key-set double, and
35+
* `rereadKeyringForUnknownKey` included, which `verifyMcpAccessToken` asks
36+
* through the manager rather than calling the function here directly.
37+
* `config` goes as a GETTER: the class reassigns it after construction
38+
* (`setRuntimeBaseUrl` and `applyConfigPatch`), so it is read off the manager
39+
* when the body reads it, as before. `unknownKeyReread` goes as a get/set
40+
* pair, because `rereadKeyringForUnknownKey` stores the re-read it starts on
41+
* the manager: the floor and the read in flight stay the ones the manager
42+
* holds, shared by every token it verifies.
43+
*
44+
* `readPublishedJwks` moves with the bodies, because only they read it.
45+
* `OAUTH_ORGANIZATION_CLAIM` stays: the plugin assembly that mints the claim
46+
* (`buildPluginList`, still in the class) reads it too, and it is one
47+
* spelling for both ends. So `auth-manager.ts` exports it and this module
48+
* imports it by VALUE, for one read inside `verifyMcpAccessToken`, when a
49+
* token is verified. `index.ts` lists `auth-manager.ts`'s published names one
50+
* by one, so that export is not published. The two modules import each
51+
* other, and that is safe because nothing reads the binding while they are
52+
* first evaluated: this module's top level is imports, an interface and
53+
* function declarations.
54+
*
55+
* Nothing re-exports this module, so the published surface does not move.
56+
*/
57+
58+
import type { JSONWebKeySet } from 'jose';
59+
import { OAUTH_ORGANIZATION_CLAIM, type AuthManager } from './auth-manager.js';
60+
import {
61+
retireHeldKeyrings,
62+
UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS,
63+
} from './signing-key-read-cache.js';
64+
65+
/**
66+
* [#24186] The key set the MCP access-token verifier checks signatures
67+
* against: the document better-auth's `getJwks` endpoint answers, which is
68+
* what `/jwks` publishes — public members only, read through the signer's
69+
* keyring reader (`signing-key-read-cache.ts`). `undefined` when the instance
70+
* carries no jwt plugin or the answer is not a key set.
71+
*/
72+
async function readPublishedJwks(api: unknown): Promise<JSONWebKeySet | undefined> {
73+
const jwks = await (api as { getJwks?: () => Promise<unknown> } | null | undefined)?.getJwks?.();
74+
return jwks && Array.isArray((jwks as { keys?: unknown }).keys) ? (jwks as JSONWebKeySet) : undefined;
75+
}
76+
77+
/**
78+
* The `AuthManager` members the functions below read — every one of them, and
79+
* no other; each function's `this` is the `Pick` of the members IT reads. Each
80+
* is typed AS the class member (an indexed access on the class), so a change
81+
* to a member's signature on the class is a change here too, and naming a
82+
* member the class does not have is a compile error. See the module header for
83+
* why `config` is a getter and `unknownKeyReread` an accessor.
84+
*/
85+
export interface McpAccessTokenHost {
86+
readonly config: AuthManager['config'];
87+
unknownKeyReread: AuthManager['unknownKeyReread'];
88+
getApi: AuthManager['getApi'];
89+
getAuthIssuer: AuthManager['getAuthIssuer'];
90+
getMcpResourceUrl: AuthManager['getMcpResourceUrl'];
91+
isMcpOAuthEnabled: AuthManager['isMcpOAuthEnabled'];
92+
rereadKeyringForUnknownKey: AuthManager['rereadKeyringForUnknownKey'];
93+
}
94+
95+
/** The body of `AuthManager#verifyMcpAccessToken`; the method's docblock there is its contract. */
96+
export async function verifyMcpAccessToken(
97+
this: Pick<McpAccessTokenHost, 'getApi' | 'getAuthIssuer' | 'getMcpResourceUrl' | 'isMcpOAuthEnabled' | 'rereadKeyringForUnknownKey'>,
98+
token: string,
99+
): Promise<{ userId: string; scopes: string[]; clientId?: string; organizationId?: string } | null> {
100+
try {
101+
if (!token || token.split('.').length !== 3) return null; // not a JWS compact token
102+
if (!this.isMcpOAuthEnabled()) return null;
103+
104+
const { createLocalJWKSet, jwtVerify, errors } = await import('jose');
105+
106+
const api = await this.getApi();
107+
const jwks = await readPublishedJwks(api);
108+
if (!jwks) return null;
109+
const expected = { issuer: this.getAuthIssuer(), audience: this.getMcpResourceUrl() };
110+
const verifyAgainst = async (keys: JSONWebKeySet) =>
111+
(await jwtVerify(token, createLocalJWKSet(keys), expected)).payload;
112+
113+
let payload: Awaited<ReturnType<typeof verifyAgainst>>;
114+
try {
115+
payload = await verifyAgainst(jwks);
116+
} catch (error) {
117+
// Only "no key in the set matches this token" earns the fresh read: a
118+
// key minted where this process could not see it. Every other failure
119+
// is the token's own, and a re-read could not change it.
120+
if (!(error instanceof errors.JWKSNoMatchingKey)) throw error;
121+
const fresh = await this.rereadKeyringForUnknownKey(api);
122+
if (!fresh) return null;
123+
payload = await verifyAgainst(fresh);
124+
}
125+
126+
const subject = typeof payload.sub === 'string' && payload.sub ? payload.sub : undefined;
127+
if (!subject) return null;
128+
129+
// The two spellings of "which client is presenting this", read
130+
// independently rather than through a `??` chain: a token that carries
131+
// both and disagrees with itself must be refused on EITHER match, and
132+
// collapsing them first would let the losing spelling smuggle the
133+
// client id past the comparison below.
134+
const clientIdClaim =
135+
typeof (payload as any).client_id === 'string' && (payload as any).client_id
136+
? ((payload as any).client_id as string)
137+
: undefined;
138+
const azp =
139+
typeof (payload as any).azp === 'string' && (payload as any).azp
140+
? ((payload as any).azp as string)
141+
: undefined;
142+
// No client identity at all → the human/machine discriminator has no
143+
// input. Fail closed rather than admit an unclassifiable token.
144+
if (!clientIdClaim && !azp) return null;
145+
// `sub` IS the client → RFC 9068 §2.2.3.1's "no resource owner was
146+
// involved" shape, i.e. a client_credentials grant. No principal.
147+
if (subject === clientIdClaim || subject === azp) return null;
148+
149+
const scopes =
150+
typeof payload.scope === 'string'
151+
? payload.scope.split(' ').filter(Boolean)
152+
: [];
153+
// The bound organization, when the token carries one. A claim that is
154+
// PRESENT but is not a non-empty string is no organization this
155+
// authorization server mints, so the token is refused rather than read
156+
// as "no organization": the check that cannot run must not pass.
157+
const organizationClaim = (payload as Record<string, unknown>)[OAUTH_ORGANIZATION_CLAIM];
158+
if (
159+
organizationClaim !== undefined
160+
&& (typeof organizationClaim !== 'string' || !organizationClaim)
161+
) return null;
162+
const organizationId = organizationClaim as string | undefined;
163+
return {
164+
userId: subject,
165+
scopes,
166+
...(azp ? { clientId: azp } : {}),
167+
...(organizationId ? { organizationId } : {}),
168+
};
169+
} catch {
170+
return null; // unknown/expired/wrong-audience/garbage → no principal
171+
}
172+
}
173+
174+
/** The body of `AuthManager#rereadKeyringForUnknownKey`; the method's docblock there is its contract. */
175+
export function rereadKeyringForUnknownKey(this: Pick<McpAccessTokenHost, 'config' | 'unknownKeyReread'>, api: unknown): Promise<JSONWebKeySet | undefined> {
176+
const last = this.unknownKeyReread;
177+
if (last?.pending) return last.pending;
178+
const now = Date.now();
179+
if (last && now - last.startedAtMs < UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS) {
180+
return Promise.resolve(undefined);
181+
}
182+
retireHeldKeyrings(this.config.dataEngine);
183+
const pending = readPublishedJwks(api);
184+
const reread: { readonly startedAtMs: number; pending?: Promise<JSONWebKeySet | undefined> } = {
185+
startedAtMs: now,
186+
pending,
187+
};
188+
this.unknownKeyReread = reread;
189+
const settle = () => {
190+
reread.pending = undefined;
191+
};
192+
pending.then(settle, settle);
193+
return pending;
194+
}

‎packages/plugins/plugin-auth/src/auth-manager.ts‎

Lines changed: 28 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,6 @@ import {
143143
buildAdminPluginSchema,
144144
buildPhoneNumberPluginSchema,
145145
} from './auth-schema-config.js';
146-
import {
147-
retireHeldKeyrings,
148-
UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS,
149-
} from './signing-key-read-cache.js';
150146
import {
151147
ORGANIZATION_DELETE_PATH,
152148
isDefaultOrganizationTarget,
@@ -242,6 +238,14 @@ import {
242238
validateAudienceAdmission,
243239
type MembershipAdmissionHost,
244240
} from './auth-manager-membership-admission.js';
241+
// [#25224 cut 9] The MCP access-token verifier and its forced keyring re-read;
242+
// the methods of the same names below hand each body the members it reads
243+
// (`McpAccessTokenHost`).
244+
import {
245+
rereadKeyringForUnknownKey,
246+
verifyMcpAccessToken,
247+
type McpAccessTokenHost,
248+
} from './auth-manager-mcp-access-token.js';
245249

246250
/**
247251
* Detect WebContainer (StackBlitz) environment.
@@ -1462,7 +1466,7 @@ async function invitationPlacementReadFaultApiError(cause: unknown): Promise<Err
14621466
* the embedded authorization server and read back by
14631467
* {@link AuthManager.verifyMcpAccessToken} — one spelling for both ends.
14641468
*/
1465-
const OAUTH_ORGANIZATION_CLAIM = 'organization_id';
1469+
export const OAUTH_ORGANIZATION_CLAIM = 'organization_id';
14661470

14671471
/**
14681472
* [#24166] The organization an OAuth consent binds: the consenting session's
@@ -1480,18 +1484,6 @@ function consentOrganizationOf(session: Record<string, unknown> | null | undefin
14801484
return typeof organizationId === 'string' && organizationId ? organizationId : undefined;
14811485
}
14821486

1483-
/**
1484-
* [#24186] The key set the MCP access-token verifier checks signatures
1485-
* against: the document better-auth's `getJwks` endpoint answers, which is
1486-
* what `/jwks` publishes — public members only, read through the signer's
1487-
* keyring reader (`signing-key-read-cache.ts`). `undefined` when the instance
1488-
* carries no jwt plugin or the answer is not a key set.
1489-
*/
1490-
async function readPublishedJwks(api: unknown): Promise<JSONWebKeySet | undefined> {
1491-
const jwks = await (api as { getJwks?: () => Promise<unknown> } | null | undefined)?.getJwks?.();
1492-
return jwks && Array.isArray((jwks as { keys?: unknown }).keys) ? (jwks as JSONWebKeySet) : undefined;
1493-
}
1494-
14951487
export class AuthManager {
14961488
private auth: Auth<any> | null = null;
14971489
/**
@@ -7225,81 +7217,18 @@ export class AuthManager {
72257217
* organization (`vetOrganizationClaim`, `@objectstack/core`), as the
72267218
* `IAuthService` contract states.
72277219
*/
7228-
async verifyMcpAccessToken(
7220+
verifyMcpAccessToken(
72297221
token: string,
72307222
): Promise<{ userId: string; scopes: string[]; clientId?: string; organizationId?: string } | null> {
7231-
try {
7232-
if (!token || token.split('.').length !== 3) return null; // not a JWS compact token
7233-
if (!this.isMcpOAuthEnabled()) return null;
7234-
7235-
const { createLocalJWKSet, jwtVerify, errors } = await import('jose');
7236-
7237-
const api = await this.getApi();
7238-
const jwks = await readPublishedJwks(api);
7239-
if (!jwks) return null;
7240-
const expected = { issuer: this.getAuthIssuer(), audience: this.getMcpResourceUrl() };
7241-
const verifyAgainst = async (keys: JSONWebKeySet) =>
7242-
(await jwtVerify(token, createLocalJWKSet(keys), expected)).payload;
7243-
7244-
let payload: Awaited<ReturnType<typeof verifyAgainst>>;
7245-
try {
7246-
payload = await verifyAgainst(jwks);
7247-
} catch (error) {
7248-
// Only "no key in the set matches this token" earns the fresh read: a
7249-
// key minted where this process could not see it. Every other failure
7250-
// is the token's own, and a re-read could not change it.
7251-
if (!(error instanceof errors.JWKSNoMatchingKey)) throw error;
7252-
const fresh = await this.rereadKeyringForUnknownKey(api);
7253-
if (!fresh) return null;
7254-
payload = await verifyAgainst(fresh);
7255-
}
7256-
7257-
const subject = typeof payload.sub === 'string' && payload.sub ? payload.sub : undefined;
7258-
if (!subject) return null;
7259-
7260-
// The two spellings of "which client is presenting this", read
7261-
// independently rather than through a `??` chain: a token that carries
7262-
// both and disagrees with itself must be refused on EITHER match, and
7263-
// collapsing them first would let the losing spelling smuggle the
7264-
// client id past the comparison below.
7265-
const clientIdClaim =
7266-
typeof (payload as any).client_id === 'string' && (payload as any).client_id
7267-
? ((payload as any).client_id as string)
7268-
: undefined;
7269-
const azp =
7270-
typeof (payload as any).azp === 'string' && (payload as any).azp
7271-
? ((payload as any).azp as string)
7272-
: undefined;
7273-
// No client identity at all → the human/machine discriminator has no
7274-
// input. Fail closed rather than admit an unclassifiable token.
7275-
if (!clientIdClaim && !azp) return null;
7276-
// `sub` IS the client → RFC 9068 §2.2.3.1's "no resource owner was
7277-
// involved" shape, i.e. a client_credentials grant. No principal.
7278-
if (subject === clientIdClaim || subject === azp) return null;
7279-
7280-
const scopes =
7281-
typeof payload.scope === 'string'
7282-
? payload.scope.split(' ').filter(Boolean)
7283-
: [];
7284-
// The bound organization, when the token carries one. A claim that is
7285-
// PRESENT but is not a non-empty string is no organization this
7286-
// authorization server mints, so the token is refused rather than read
7287-
// as "no organization": the check that cannot run must not pass.
7288-
const organizationClaim = (payload as Record<string, unknown>)[OAUTH_ORGANIZATION_CLAIM];
7289-
if (
7290-
organizationClaim !== undefined
7291-
&& (typeof organizationClaim !== 'string' || !organizationClaim)
7292-
) return null;
7293-
const organizationId = organizationClaim as string | undefined;
7294-
return {
7295-
userId: subject,
7296-
scopes,
7297-
...(azp ? { clientId: azp } : {}),
7298-
...(organizationId ? { organizationId } : {}),
7299-
};
7300-
} catch {
7301-
return null; // unknown/expired/wrong-audience/garbage → no principal
7302-
}
7223+
// [#25224 cut 9] The body moved to `auth-manager-mcp-access-token.ts` (see `McpAccessTokenHost`).
7224+
const host: Pick<McpAccessTokenHost, 'getApi' | 'getAuthIssuer' | 'getMcpResourceUrl' | 'isMcpOAuthEnabled' | 'rereadKeyringForUnknownKey'> = {
7225+
getApi: (...a) => this.getApi(...a),
7226+
getAuthIssuer: (...a) => this.getAuthIssuer(...a),
7227+
getMcpResourceUrl: (...a) => this.getMcpResourceUrl(...a),
7228+
isMcpOAuthEnabled: (...a) => this.isMcpOAuthEnabled(...a),
7229+
rereadKeyringForUnknownKey: (...a) => this.rereadKeyringForUnknownKey(...a),
7230+
};
7231+
return verifyMcpAccessToken.call(host, token);
73037232
}
73047233

73057234
/**
@@ -7319,24 +7248,16 @@ export class AuthManager {
73197248
* not read again on every token.
73207249
*/
73217250
private rereadKeyringForUnknownKey(api: unknown): Promise<JSONWebKeySet | undefined> {
7322-
const last = this.unknownKeyReread;
7323-
if (last?.pending) return last.pending;
7324-
const now = Date.now();
7325-
if (last && now - last.startedAtMs < UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS) {
7326-
return Promise.resolve(undefined);
7327-
}
7328-
retireHeldKeyrings(this.config.dataEngine);
7329-
const pending = readPublishedJwks(api);
7330-
const reread: { readonly startedAtMs: number; pending?: Promise<JSONWebKeySet | undefined> } = {
7331-
startedAtMs: now,
7332-
pending,
7333-
};
7334-
this.unknownKeyReread = reread;
7335-
const settle = () => {
7336-
reread.pending = undefined;
7251+
// [#25224 cut 9] The body moved to `auth-manager-mcp-access-token.ts` (see `McpAccessTokenHost`).
7252+
const readConfig = () => this.config;
7253+
const readUnknownKeyReread = () => this.unknownKeyReread;
7254+
const writeUnknownKeyReread = (v: AuthManager['unknownKeyReread']) => { this.unknownKeyReread = v; };
7255+
const host: Pick<McpAccessTokenHost, 'config' | 'unknownKeyReread'> = {
7256+
get config() { return readConfig(); },
7257+
get unknownKeyReread() { return readUnknownKeyReread(); },
7258+
set unknownKeyReread(v) { writeUnknownKeyReread(v); },
73377259
};
7338-
pending.then(settle, settle);
7339-
return pending;
7260+
return rereadKeyringForUnknownKey.call(host, api);
73407261
}
73417262

73427263
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)