Skip to content

Commit 6194948

Browse files
committed
refactor(plugin-auth): move AuthManager's MCP access-token verifier into auth-manager-mcp-access-token.ts
Two method bodies move unedited: verifyMcpAccessToken, the local, fail-closed check of an OAuth Bearer access token this deployment minted, and rereadKeyringForUnknownKey, the one forced keyring re-read a token signed by an unknown key earns. readPublishedJwks, which only they read, moves with them unedited. The class keeps both methods, their docblocks and signatures, and hands each body the members it reads. OAUTH_ORGANIZATION_CLAIM stays in auth-manager.ts, where the plugin assembly also reads it, and is exported there for the new module; index.ts does not publish it. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LSXBc2GCBwiAL7vV4kWkyr
1 parent b048a93 commit 6194948

2 files changed

Lines changed: 222 additions & 107 deletions

File tree

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)