diff --git a/packages/plugins/plugin-auth/src/auth-manager-email-sms-delivery.ts b/packages/plugins/plugin-auth/src/auth-manager-email-sms-delivery.ts index 8e28cbd7ab..cca5fa3a6b 100644 --- a/packages/plugins/plugin-auth/src/auth-manager-email-sms-delivery.ts +++ b/packages/plugins/plugin-auth/src/auth-manager-email-sms-delivery.ts @@ -43,10 +43,11 @@ * manager holds. * * The four module-level declarations move with the bodies rather than being - * imported back: `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. Nothing re-exports this module, so - * the published surface does not move. + * 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. */ import type { StandardErrorCode } from '@objectstack/spec/api'; diff --git a/packages/plugins/plugin-auth/src/auth-manager-mcp-access-token.ts b/packages/plugins/plugin-auth/src/auth-manager-mcp-access-token.ts new file mode 100644 index 0000000000..b8e86b1cc1 --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-manager-mcp-access-token.ts @@ -0,0 +1,194 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25224 cut 9] The MCP access-token verifier of `AuthManager` — the local, + * fail-closed check of an OAuth 2.1 Bearer access token this deployment's + * authorization server minted, and the one forced keyring re-read a token + * signed by an unknown key earns — moved out of `auth-manager.ts` whole, with + * the one module-level declaration that only these bodies read: the reader of + * the key set `/jwks` publishes. + * + * A MOVE, not a redesign. Each function below is the body of the + * `AuthManager` method of the same name as it stood, re-indented one level + * and otherwise unedited, and `readPublishedJwks` is its text as it stood in + * `auth-manager.ts`: the same signature, issuer and audience checks, the same + * refusal of a token no human delegated, the same organization claim, the + * same single-flight re-read and its floor. + * + * ## How the moved bodies reach the class + * + * The shape #23050 set for `rest-server.ts` (its `data-action-routes.ts` + * header carries the full reasoning), which `auth-manager-actor-guards.ts`, + * `auth-manager-sign-in-lockout.ts`, `auth-manager-password-session-policy.ts`, + * `auth-manager-database-hooks.ts`, `auth-manager-email-sms-delivery.ts`, + * `auth-manager-plugin-assembly.ts`, `auth-manager-database-secret.ts` and + * `auth-manager-membership-admission.ts` follow, applied per method. The class + * keeps both methods — the published `dist/*.d.ts` lists each one — and each + * body is one literal of exactly the {@link McpAccessTokenHost} members that + * method reads, plus a `.call`. `verifyMcpAccessToken` is no longer `async`: + * it returns the moved function's own promise, so a caller awaits the same + * promise the body settles, with no extra wrapper. + * + * A method goes as a closure that calls it on the manager at call time, so a + * test double or spy set on the manager is still the one a body reaches — + * `getApi`, which the verifier suites replace with a key-set double, and + * `rereadKeyringForUnknownKey` included, which `verifyMcpAccessToken` asks + * through the manager rather than calling the function here directly. + * `config` goes as a GETTER: the class reassigns it after construction + * (`setRuntimeBaseUrl` and `applyConfigPatch`), so it is read off the manager + * when the body reads it, as before. `unknownKeyReread` goes as a get/set + * pair, because `rereadKeyringForUnknownKey` stores the re-read it starts on + * the manager: the floor and the read in flight stay the ones the manager + * holds, shared by every token it verifies. + * + * `readPublishedJwks` moves with the bodies, because only they read it. + * `OAUTH_ORGANIZATION_CLAIM` stays: the plugin assembly that mints the claim + * (`buildPluginList`, still in the class) reads it too, and it is one + * spelling for both ends. So `auth-manager.ts` exports it and this module + * imports it by VALUE, for one read inside `verifyMcpAccessToken`, when a + * token is verified. `index.ts` lists `auth-manager.ts`'s published names one + * by one, so that export is not published. The two modules import each + * other, and that is safe because nothing reads the binding while they are + * first evaluated: this module's top level is imports, an interface and + * function declarations. + * + * Nothing re-exports this module, so the published surface does not move. + */ + +import type { JSONWebKeySet } from 'jose'; +import { OAUTH_ORGANIZATION_CLAIM, type AuthManager } from './auth-manager.js'; +import { + retireHeldKeyrings, + UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS, +} from './signing-key-read-cache.js'; + +/** + * [#24186] The key set the MCP access-token verifier checks signatures + * against: the document better-auth's `getJwks` endpoint answers, which is + * what `/jwks` publishes — public members only, read through the signer's + * keyring reader (`signing-key-read-cache.ts`). `undefined` when the instance + * carries no jwt plugin or the answer is not a key set. + */ +async function readPublishedJwks(api: unknown): Promise { + const jwks = await (api as { getJwks?: () => Promise } | null | undefined)?.getJwks?.(); + return jwks && Array.isArray((jwks as { keys?: unknown }).keys) ? (jwks as JSONWebKeySet) : undefined; +} + +/** + * The `AuthManager` members the functions below read — every one of them, and + * no other; each function's `this` is the `Pick` of the members IT reads. Each + * is typed AS the class member (an indexed access on the class), so a change + * to a member's signature on the class is a change here too, and naming a + * member the class does not have is a compile error. See the module header for + * why `config` is a getter and `unknownKeyReread` an accessor. + */ +export interface McpAccessTokenHost { + readonly config: AuthManager['config']; + unknownKeyReread: AuthManager['unknownKeyReread']; + getApi: AuthManager['getApi']; + getAuthIssuer: AuthManager['getAuthIssuer']; + getMcpResourceUrl: AuthManager['getMcpResourceUrl']; + isMcpOAuthEnabled: AuthManager['isMcpOAuthEnabled']; + rereadKeyringForUnknownKey: AuthManager['rereadKeyringForUnknownKey']; +} + +/** The body of `AuthManager#verifyMcpAccessToken`; the method's docblock there is its contract. */ +export async function verifyMcpAccessToken( + this: Pick, + token: string, +): Promise<{ userId: string; scopes: string[]; clientId?: string; organizationId?: string } | null> { + try { + if (!token || token.split('.').length !== 3) return null; // not a JWS compact token + if (!this.isMcpOAuthEnabled()) return null; + + const { createLocalJWKSet, jwtVerify, errors } = await import('jose'); + + const api = await this.getApi(); + const jwks = await readPublishedJwks(api); + if (!jwks) return null; + const expected = { issuer: this.getAuthIssuer(), audience: this.getMcpResourceUrl() }; + const verifyAgainst = async (keys: JSONWebKeySet) => + (await jwtVerify(token, createLocalJWKSet(keys), expected)).payload; + + let payload: Awaited>; + try { + payload = await verifyAgainst(jwks); + } catch (error) { + // Only "no key in the set matches this token" earns the fresh read: a + // key minted where this process could not see it. Every other failure + // is the token's own, and a re-read could not change it. + if (!(error instanceof errors.JWKSNoMatchingKey)) throw error; + const fresh = await this.rereadKeyringForUnknownKey(api); + if (!fresh) return null; + payload = await verifyAgainst(fresh); + } + + const subject = typeof payload.sub === 'string' && payload.sub ? payload.sub : undefined; + if (!subject) return null; + + // The two spellings of "which client is presenting this", read + // independently rather than through a `??` chain: a token that carries + // both and disagrees with itself must be refused on EITHER match, and + // collapsing them first would let the losing spelling smuggle the + // client id past the comparison below. + const clientIdClaim = + typeof (payload as any).client_id === 'string' && (payload as any).client_id + ? ((payload as any).client_id as string) + : undefined; + const azp = + typeof (payload as any).azp === 'string' && (payload as any).azp + ? ((payload as any).azp as string) + : undefined; + // No client identity at all → the human/machine discriminator has no + // input. Fail closed rather than admit an unclassifiable token. + if (!clientIdClaim && !azp) return null; + // `sub` IS the client → RFC 9068 §2.2.3.1's "no resource owner was + // involved" shape, i.e. a client_credentials grant. No principal. + if (subject === clientIdClaim || subject === azp) return null; + + const scopes = + typeof payload.scope === 'string' + ? payload.scope.split(' ').filter(Boolean) + : []; + // The bound organization, when the token carries one. A claim that is + // PRESENT but is not a non-empty string is no organization this + // authorization server mints, so the token is refused rather than read + // as "no organization": the check that cannot run must not pass. + const organizationClaim = (payload as Record)[OAUTH_ORGANIZATION_CLAIM]; + if ( + organizationClaim !== undefined + && (typeof organizationClaim !== 'string' || !organizationClaim) + ) return null; + const organizationId = organizationClaim as string | undefined; + return { + userId: subject, + scopes, + ...(azp ? { clientId: azp } : {}), + ...(organizationId ? { organizationId } : {}), + }; + } catch { + return null; // unknown/expired/wrong-audience/garbage → no principal + } +} + +/** The body of `AuthManager#rereadKeyringForUnknownKey`; the method's docblock there is its contract. */ +export function rereadKeyringForUnknownKey(this: Pick, api: unknown): Promise { + const last = this.unknownKeyReread; + if (last?.pending) return last.pending; + const now = Date.now(); + if (last && now - last.startedAtMs < UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS) { + return Promise.resolve(undefined); + } + retireHeldKeyrings(this.config.dataEngine); + const pending = readPublishedJwks(api); + const reread: { readonly startedAtMs: number; pending?: Promise } = { + startedAtMs: now, + pending, + }; + this.unknownKeyReread = reread; + const settle = () => { + reread.pending = undefined; + }; + pending.then(settle, settle); + return pending; +} diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 12417a9fb2..989e7547af 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -143,10 +143,6 @@ import { buildAdminPluginSchema, buildPhoneNumberPluginSchema, } from './auth-schema-config.js'; -import { - retireHeldKeyrings, - UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS, -} from './signing-key-read-cache.js'; import { ORGANIZATION_DELETE_PATH, isDefaultOrganizationTarget, @@ -242,6 +238,14 @@ import { validateAudienceAdmission, type MembershipAdmissionHost, } from './auth-manager-membership-admission.js'; +// [#25224 cut 9] The MCP access-token verifier and its forced keyring re-read; +// the methods of the same names below hand each body the members it reads +// (`McpAccessTokenHost`). +import { + rereadKeyringForUnknownKey, + verifyMcpAccessToken, + type McpAccessTokenHost, +} from './auth-manager-mcp-access-token.js'; /** * Detect WebContainer (StackBlitz) environment. @@ -1462,7 +1466,7 @@ async function invitationPlacementReadFaultApiError(cause: unknown): Promise | null | undefin return typeof organizationId === 'string' && organizationId ? organizationId : undefined; } -/** - * [#24186] The key set the MCP access-token verifier checks signatures - * against: the document better-auth's `getJwks` endpoint answers, which is - * what `/jwks` publishes — public members only, read through the signer's - * keyring reader (`signing-key-read-cache.ts`). `undefined` when the instance - * carries no jwt plugin or the answer is not a key set. - */ -async function readPublishedJwks(api: unknown): Promise { - const jwks = await (api as { getJwks?: () => Promise } | null | undefined)?.getJwks?.(); - return jwks && Array.isArray((jwks as { keys?: unknown }).keys) ? (jwks as JSONWebKeySet) : undefined; -} - export class AuthManager { private auth: Auth | null = null; /** @@ -7225,81 +7217,18 @@ export class AuthManager { * organization (`vetOrganizationClaim`, `@objectstack/core`), as the * `IAuthService` contract states. */ - async verifyMcpAccessToken( + verifyMcpAccessToken( token: string, ): Promise<{ userId: string; scopes: string[]; clientId?: string; organizationId?: string } | null> { - try { - if (!token || token.split('.').length !== 3) return null; // not a JWS compact token - if (!this.isMcpOAuthEnabled()) return null; - - const { createLocalJWKSet, jwtVerify, errors } = await import('jose'); - - const api = await this.getApi(); - const jwks = await readPublishedJwks(api); - if (!jwks) return null; - const expected = { issuer: this.getAuthIssuer(), audience: this.getMcpResourceUrl() }; - const verifyAgainst = async (keys: JSONWebKeySet) => - (await jwtVerify(token, createLocalJWKSet(keys), expected)).payload; - - let payload: Awaited>; - try { - payload = await verifyAgainst(jwks); - } catch (error) { - // Only "no key in the set matches this token" earns the fresh read: a - // key minted where this process could not see it. Every other failure - // is the token's own, and a re-read could not change it. - if (!(error instanceof errors.JWKSNoMatchingKey)) throw error; - const fresh = await this.rereadKeyringForUnknownKey(api); - if (!fresh) return null; - payload = await verifyAgainst(fresh); - } - - const subject = typeof payload.sub === 'string' && payload.sub ? payload.sub : undefined; - if (!subject) return null; - - // The two spellings of "which client is presenting this", read - // independently rather than through a `??` chain: a token that carries - // both and disagrees with itself must be refused on EITHER match, and - // collapsing them first would let the losing spelling smuggle the - // client id past the comparison below. - const clientIdClaim = - typeof (payload as any).client_id === 'string' && (payload as any).client_id - ? ((payload as any).client_id as string) - : undefined; - const azp = - typeof (payload as any).azp === 'string' && (payload as any).azp - ? ((payload as any).azp as string) - : undefined; - // No client identity at all → the human/machine discriminator has no - // input. Fail closed rather than admit an unclassifiable token. - if (!clientIdClaim && !azp) return null; - // `sub` IS the client → RFC 9068 §2.2.3.1's "no resource owner was - // involved" shape, i.e. a client_credentials grant. No principal. - if (subject === clientIdClaim || subject === azp) return null; - - const scopes = - typeof payload.scope === 'string' - ? payload.scope.split(' ').filter(Boolean) - : []; - // The bound organization, when the token carries one. A claim that is - // PRESENT but is not a non-empty string is no organization this - // authorization server mints, so the token is refused rather than read - // as "no organization": the check that cannot run must not pass. - const organizationClaim = (payload as Record)[OAUTH_ORGANIZATION_CLAIM]; - if ( - organizationClaim !== undefined - && (typeof organizationClaim !== 'string' || !organizationClaim) - ) return null; - const organizationId = organizationClaim as string | undefined; - return { - userId: subject, - scopes, - ...(azp ? { clientId: azp } : {}), - ...(organizationId ? { organizationId } : {}), - }; - } catch { - return null; // unknown/expired/wrong-audience/garbage → no principal - } + // [#25224 cut 9] The body moved to `auth-manager-mcp-access-token.ts` (see `McpAccessTokenHost`). + const host: Pick = { + getApi: (...a) => this.getApi(...a), + getAuthIssuer: (...a) => this.getAuthIssuer(...a), + getMcpResourceUrl: (...a) => this.getMcpResourceUrl(...a), + isMcpOAuthEnabled: (...a) => this.isMcpOAuthEnabled(...a), + rereadKeyringForUnknownKey: (...a) => this.rereadKeyringForUnknownKey(...a), + }; + return verifyMcpAccessToken.call(host, token); } /** @@ -7319,24 +7248,16 @@ export class AuthManager { * not read again on every token. */ private rereadKeyringForUnknownKey(api: unknown): Promise { - const last = this.unknownKeyReread; - if (last?.pending) return last.pending; - const now = Date.now(); - if (last && now - last.startedAtMs < UNKNOWN_SIGNING_KEY_REREAD_FLOOR_MS) { - return Promise.resolve(undefined); - } - retireHeldKeyrings(this.config.dataEngine); - const pending = readPublishedJwks(api); - const reread: { readonly startedAtMs: number; pending?: Promise } = { - startedAtMs: now, - pending, - }; - this.unknownKeyReread = reread; - const settle = () => { - reread.pending = undefined; + // [#25224 cut 9] The body moved to `auth-manager-mcp-access-token.ts` (see `McpAccessTokenHost`). + const readConfig = () => this.config; + const readUnknownKeyReread = () => this.unknownKeyReread; + const writeUnknownKeyReread = (v: AuthManager['unknownKeyReread']) => { this.unknownKeyReread = v; }; + const host: Pick = { + get config() { return readConfig(); }, + get unknownKeyReread() { return readUnknownKeyReread(); }, + set unknownKeyReread(v) { writeUnknownKeyReread(v); }, }; - pending.then(settle, settle); - return pending; + return rereadKeyringForUnknownKey.call(host, api); } // --------------------------------------------------------------------------- diff --git a/packages/plugins/plugin-auth/src/index.ts b/packages/plugins/plugin-auth/src/index.ts index d5c015f034..3aadf41d89 100644 --- a/packages/plugins/plugin-auth/src/index.ts +++ b/packages/plugins/plugin-auth/src/index.ts @@ -13,7 +13,26 @@ export * from './auth-plugin.js'; // mounts ObjectQL without AuthPlugin (an app's or a plugin's test kit). // AuthPlugin registers the same manifest from the same builder. export * from './identity-objects-plugin.js'; -export * from './auth-manager.js'; +// [#25224] auth-manager.ts's published names, listed by name so that a +// module-level helper it exports for the modules split out of it is not +// published with them. A new export of auth-manager.ts is published only by +// adding it here. +export { + DEFAULT_AUTH_BASE_PATH, + readMcpServerEnabledEnv, + resolveOidcProviderEnabled, + resolveDcrEnabled, + resolveScimEnabled, + assertScimAdminCoherence, + isOAuthEligibleBaseUrl, + type InvitationPlacementServiceLike, + readInvitationPlacementIntent, + type AuthManagerOptions, + ipMatchesRange, + normalizeAuthEmailLocale, + authEmailLocaleFromRequest, + AuthManager, +} from './auth-manager.js'; export * from './ensure-default-organization.js'; // [ADR-0131 D3 / D10 fate 3] The Default Organization's boot invariant, for the // v18 organization-ownership ceremony (`os migrate organization-ownership