diff --git a/.changeset/25118-dcr-infer-native.md b/.changeset/25118-dcr-infer-native.md new file mode 100644 index 00000000000..4cb0c7f33e8 --- /dev/null +++ b/.changeset/25118-dcr-infer-native.md @@ -0,0 +1,19 @@ +--- +'@objectstack/plugin-auth': patch +--- + +fix(plugin-auth): an MCP client that registers over OAuth dynamic client registration with a loopback or custom-scheme redirect, and no `application_type`, is registered as a native client instead of being refused + +Clause-②: no (a runtime security-boundary change the maintainer ruled; no published type, export or metadata key moves) +Metadata-protocol: none + +Local MCP clients (Claude Code, Cursor, VS Code, Codex CLI) register at `POST /api/v1/auth/oauth2/register` with a loopback `http` redirect (`http://localhost:PORT/...`, `http://127.0.0.1:PORT/...`) or a custom-scheme redirect, and send no `application_type`. The OAuth provider used to fill the missing type with `web` and refuse both redirect forms (`invalid_redirect_uri: web clients require https redirect URIs on non-loopback hosts`), so these clients could not connect to the MCP endpoint over OAuth and API keys were the only working path. + +plugin-auth now passes `clientRegistrationDefaultApplicationType: 'infer'` to `@better-auth/oauth-provider`. A dynamic registration that omits `application_type` is classified `native` when any redirect URI uses a non-http(s) scheme or `http` on exactly `localhost`, `127.0.0.1` or `[::1]`, and every redirect URI is then validated by the native rule. Otherwise it stays `web`. What this changes and what it keeps: + +- Accepted as `native`: the body a local MCP client sends (a public client with a loopback or custom-scheme redirect). +- Unchanged: an `application_type` the client sends is never overridden; `https://example.com/cb` alone stays `web`; `http://example.com/cb` and `http://localhost.example.com/cb` stay refused; with dynamic client registration off, every registration is still refused. +- Also accepted as `native`: a confidential registration (`client_secret_basic`, or no `token_endpoint_auth_method`, which the provider fills as `client_secret_basic`) with a loopback or custom-scheme redirect and no `application_type`, which is issued a `client_secret`. The same body was already accepted with `application_type: 'native'` spelled out, so no new client shape becomes registrable. +- PKCE stays required at `/oauth2/authorize` for every one of these clients, and every token still needs the user's sign-in and consent. + +The Setup form's own OAuth application door (`/sys-oauth-application/register`) is not affected. diff --git a/packages/plugins/plugin-auth/src/auth-manager.mcp-oauth-dcr-infer.test.ts b/packages/plugins/plugin-auth/src/auth-manager.mcp-oauth-dcr-infer.test.ts new file mode 100644 index 00000000000..d1980dcc9d8 --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-manager.mcp-oauth-dcr-infer.test.ts @@ -0,0 +1,345 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * MCP OAuth: anonymous Dynamic Client Registration (RFC 7591) by a client that + * sends NO `application_type` (#25118, ruling 6329352251, A1). Every reading + * is taken from the REAL `@better-auth/oauth-provider`, booted from the exact + * options AuthManager hands it, never from that options object. + * + * ## Why this file exists + * + * Local MCP clients (Claude Code, Cursor, VS Code, Codex CLI) register with a + * loopback `http` or a custom-scheme redirect and omit `application_type`. The + * provider's default filled the missing type with `web` and refused both + * redirect forms, so the client failed before its browser opened. + * `auth-manager.mcp-oauth-resource.test.ts` registers with an explicit + * `application_type: 'native'`, which is why it stayed green through that + * defect. Every body below omits the field the way those clients do, and the + * first cell sends the card's own request body byte for byte. + * + * ## The boundary as measured, not as it reads + * + * 1. A public client with a loopback (`localhost`, `127.0.0.1`) or a + * custom-scheme redirect registers as `native`. + * 2. `https://evil.example/cb` alone stays `web`; `http://evil.example/cb` and + * `http://localhost.evil.example` stay refused; an explicit `web` with a + * loopback redirect stays refused (a sent type is never overridden). + * 3. The provider classifies by redirect URIs and never reads + * `token_endpoint_auth_method`. So this is NOT "public clients only": a + * CONFIDENTIAL registration with a loopback or custom-scheme redirect and no + * type registers as `native` and is issued a `client_secret`. The same body + * with `application_type: 'native'` spelled out was already accepted. + * "Any redirect qualifies" also lets an https redirect ride beside a + * loopback one in the same body. + * 4. For EVERY client registered here, /oauth2/authorize without PKCE is + * refused, and the same request with S256 PKCE reaches consent. The pair + * differs only in the PKCE parameters, so the refusal cannot have another + * cause. + * 5. With DCR off the option opens nothing: the card's body is refused. + */ + +import { describe, it, expect, vi, beforeAll } from 'vitest'; + +import { AuthManager } from './auth-manager'; +import { buildJwtPluginSchema } from './auth-schema-config.js'; + +// Same pattern as auth-manager.mcp-oauth-resource.test.ts: better-auth is +// mocked so AuthManager's own instance build stays cheap, and the options it +// hands `oauthProvider()` are captured. The server every assertion runs +// against is a SEPARATE, REAL one booted from those options. +vi.mock('better-auth', () => ({ + betterAuth: vi.fn(() => ({ handler: vi.fn(), api: {} })), +})); +vi.mock('@better-auth/oauth-provider', () => ({ + oauthProvider: vi.fn((opts: any) => ({ id: 'oauth-provider', _opts: opts })), +})); + +import { oauthProvider } from '@better-auth/oauth-provider'; + +const BASE_URL = 'https://acme.example.com'; +const AUTH_BASE_PATH = '/api/v1/auth'; +const ISSUER = `${BASE_URL}${AUTH_BASE_PATH}`; +const MCP_RESOURCE = `${BASE_URL}/api/v1/mcp`; +// RFC 7636 Appendix B challenge. +const PKCE_CHALLENGE = 'E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM'; + +const LOOPBACK = 'http://localhost:63678/callback'; +const LOOPBACK_IPV4 = 'http://127.0.0.1:5000/callback'; +const CUSTOM_SCHEME = 'cursor://anysphere.cursor-retrieval/oauth/callback'; +const HTTPS_FOREIGN = 'https://evil.example/cb'; +const HTTP_FOREIGN = 'http://evil.example/cb'; +const LOOPBACK_LOOKALIKE = 'http://localhost.evil.example/cb'; + +/** + * The card's reproduction body, verbatim: a public client, a loopback `http` + * redirect, and no `application_type`. + */ +const CARD_BODY = { + client_name: 'probe', + redirect_uris: [LOOPBACK], + grant_types: ['authorization_code', 'refresh_token'], + response_types: ['code'], + token_endpoint_auth_method: 'none', +}; + +/** The card's body with other redirects and/or another auth method. */ +function cardBodyWith(overrides: Record, dropAuthMethod = false): Record { + const body: Record = { ...CARD_BODY, ...overrides }; + if (dropAuthMethod) delete body.token_endpoint_auth_method; + return body; +} + +const ENV_KEYS = ['OS_MCP_SERVER_ENABLED', 'OS_OIDC_PROVIDER_ENABLED', 'OS_OIDC_DCR_ENABLED'] as const; + +/** + * The options AuthManager actually hands `oauthProvider()`, under `env`. + * The process env is restored before this returns. + */ +async function captureProviderOptions(env: Partial>): Promise { + const saved: Record = {}; + for (const k of ENV_KEYS) { + saved[k] = process.env[k]; + delete process.env[k]; + } + Object.assign(process.env, env); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const manager = new AuthManager({ secret: 'test-secret-at-least-32-chars-long', baseUrl: BASE_URL }); + await manager.getAuthInstance(); + } finally { + warnSpy.mockRestore(); + for (const k of ENV_KEYS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } + } + const opts = (oauthProvider as any).mock.calls.at(-1)?.[0]; + expect(opts, 'AuthManager must register the oauthProvider plugin').toBeDefined(); + return opts; +} + +/** + * Boots a REAL better-auth server carrying the REAL provider configured with + * `opts`, on the in-memory adapter. The table list is derived from the + * plugin's own resolved schema, and `jwt()` gets the platform's schema; both + * reasons are recorded in auth-manager.mcp-oauth-resource.test.ts. + */ +async function bootRealAuthorizationServer(opts: any): Promise<{ auth: any; clientCount: () => number }> { + const [{ betterAuth }, { memoryAdapter }, { jwt }, { oauthProvider: realOauthProvider }] = await Promise.all([ + vi.importActual('better-auth'), + import('better-auth/adapters/memory'), + import('better-auth/plugins'), + vi.importActual('@better-auth/oauth-provider'), + ]); + const plugin = realOauthProvider(opts); + const pluginSchema = (plugin as any).schema as Record; + const db: Record = {}; + for (const m of ['user', 'session', 'account', 'verification', 'jwks']) db[m] = []; + for (const [model, def] of Object.entries(pluginSchema ?? {})) db[def.modelName ?? model] = []; + const jwtPlugin = jwt({ schema: buildJwtPluginSchema() as any }); + const jwksModel = (jwtPlugin as any).schema?.jwks?.modelName ?? 'jwks'; + db[jwksModel] = db[jwksModel] ?? []; + const auth = betterAuth({ + baseURL: BASE_URL, + basePath: AUTH_BASE_PATH, + secret: 'test-secret-at-least-32-chars-long', + database: memoryAdapter(db), + emailAndPassword: { enabled: true }, + plugins: [jwtPlugin, plugin as any], + }); + await auth.$context; + const clientModel = pluginSchema?.oauthClient?.modelName ?? 'oauthClient'; + return { auth, clientCount: () => db[clientModel]?.length ?? 0 }; +} + +async function register(auth: any, body: Record): Promise<{ status: number; body: any }> { + const res = await auth.handler( + new Request(`${ISSUER}/oauth2/register`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify(body), + }), + ); + return { status: res.status, body: (await res.json().catch(() => null)) as any }; +} + +async function signUp(auth: any): Promise { + const res = await auth.handler( + new Request(`${ISSUER}/sign-up/email`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ email: 'dev@acme.example.com', password: 'password-12345', name: 'Dev' }), + }), + ); + return String(res.headers.get('set-cookie') ?? '') + .split(',') + .map((c: string) => c.split(';')[0]!.trim()) + .join('; '); +} + +/** GET /oauth2/authorize for `clientId`, with or without S256 PKCE. */ +async function authorize( + auth: any, + cookie: string, + clientId: string, + redirectUri: string, + pkce: boolean, +): Promise<{ status: number; location: string }> { + const url = new URL(`${ISSUER}/oauth2/authorize`); + url.searchParams.set('client_id', clientId); + url.searchParams.set('redirect_uri', redirectUri); + url.searchParams.set('response_type', 'code'); + url.searchParams.set('scope', 'openid profile email data:read'); + url.searchParams.set('state', 'st'); + url.searchParams.set('resource', MCP_RESOURCE); + if (pkce) { + url.searchParams.set('code_challenge', PKCE_CHALLENGE); + url.searchParams.set('code_challenge_method', 'S256'); + } + const res = await auth.handler(new Request(url.toString(), { method: 'GET', headers: { cookie } })); + return { status: res.status, location: res.headers.get('location') ?? '' }; +} + +describe("DCR of a client that omits application_type, on the real provider (#25118, 'infer')", () => { + let server: { auth: any; clientCount: () => number }; + let cookie: string; + + beforeAll(async () => { + // MCP on ⇒ the OAuth provider is wired and DCR follows it (resolveDcrEnabled). + const opts = await captureProviderOptions({ OS_MCP_SERVER_ENABLED: 'true' }); + server = await bootRealAuthorizationServer(opts); + cookie = await signUp(server.auth); + }); + + /** + * The registration is accepted with `applicationType`, and the authorize + * step refuses it without PKCE while the same request with PKCE reaches + * consent. + */ + async function expectRegisteredAndPkceBound( + body: Record, + expected: { applicationType: 'native' | 'web'; authMethod: string; secret: boolean }, + ): Promise { + const reg = await register(server.auth, body); + expect(reg.status, JSON.stringify(reg.body)).toBe(201); + expect(reg.body.client_id, 'a registered client has an id').toBeTruthy(); + expect(reg.body.application_type).toBe(expected.applicationType); + expect(reg.body.token_endpoint_auth_method).toBe(expected.authMethod); + expect(typeof reg.body.client_secret === 'string', 'whether a client_secret is issued').toBe(expected.secret); + + const redirectUri = (body.redirect_uris as string[])[0]!; + + const withoutPkce = await authorize(server.auth, cookie, reg.body.client_id, redirectUri, false); + expect(withoutPkce.location, 'the refusal goes back to the client, never to consent').not.toContain('/oauth/consent'); + expect(withoutPkce.location.startsWith(redirectUri), withoutPkce.location).toBe(true); + const refusal = new URL(withoutPkce.location); + expect(refusal.searchParams.get('error'), withoutPkce.location).toBe('invalid_request'); + expect(refusal.searchParams.get('error_description') ?? '', 'the refusal names PKCE').toMatch(/pkce/i); + expect(refusal.searchParams.get('code'), 'no authorization code without PKCE').toBeNull(); + + // Differential: the same client, session, redirect and scopes, plus PKCE. + const withPkce = await authorize(server.auth, cookie, reg.body.client_id, redirectUri, true); + expect(withPkce.location, 'with PKCE the same request reaches consent').toContain('/oauth/consent'); + expect(withPkce.location).not.toContain('error='); + } + + async function expectRefusedRedirect(body: Record): Promise { + const before = server.clientCount(); + const reg = await register(server.auth, body); + expect(reg.status, JSON.stringify(reg.body)).toBe(400); + expect(reg.body?.error, JSON.stringify(reg.body)).toBe('invalid_redirect_uri'); + expect(reg.body?.client_id, 'a refused registration has no client id').toBeUndefined(); + expect(server.clientCount(), 'a refused registration stores no client').toBe(before); + } + + describe('accepted as native: the shapes native MCP clients send', () => { + it("the card's own body (public client, loopback http redirect) registers as native", async () => { + expect(Object.keys(CARD_BODY), 'the card body omits application_type, as MCP clients do').not.toContain( + 'application_type', + ); + await expectRegisteredAndPkceBound(CARD_BODY, { applicationType: 'native', authMethod: 'none', secret: false }); + }); + + it('a public client with an http://127.0.0.1 redirect registers as native', async () => { + await expectRegisteredAndPkceBound(cardBodyWith({ redirect_uris: [LOOPBACK_IPV4] }), { + applicationType: 'native', + authMethod: 'none', + secret: false, + }); + }); + + it('a public client with a custom-scheme redirect registers as native', async () => { + await expectRegisteredAndPkceBound(cardBodyWith({ redirect_uris: [CUSTOM_SCHEME] }), { + applicationType: 'native', + authMethod: 'none', + secret: false, + }); + }); + }); + + describe('unchanged: what a web client may and may not register', () => { + it('https://evil.example/cb alone stays web', async () => { + await expectRegisteredAndPkceBound(cardBodyWith({ redirect_uris: [HTTPS_FOREIGN] }), { + applicationType: 'web', + authMethod: 'none', + secret: false, + }); + }); + + it('http://evil.example/cb stays refused', async () => { + await expectRefusedRedirect(cardBodyWith({ redirect_uris: [HTTP_FOREIGN] })); + }); + + it('http://localhost.evil.example stays refused (the loopback match is exact)', async () => { + await expectRefusedRedirect(cardBodyWith({ redirect_uris: [LOOPBACK_LOOKALIKE] })); + }); + + it('an explicit application_type web with a loopback redirect stays refused (a sent type is never overridden)', async () => { + await expectRefusedRedirect(cardBodyWith({ application_type: 'web' })); + }); + }); + + describe('NOT "public clients only": the classifier reads redirect URIs, never the auth method', () => { + it('a confidential client (client_secret_basic) with a loopback redirect registers as native and is issued a client_secret', async () => { + await expectRegisteredAndPkceBound(cardBodyWith({ token_endpoint_auth_method: 'client_secret_basic' }), { + applicationType: 'native', + authMethod: 'client_secret_basic', + secret: true, + }); + }); + + it('a body that omits the auth method (filled as client_secret_basic) with a loopback redirect registers as native and is issued a client_secret', async () => { + await expectRegisteredAndPkceBound(cardBodyWith({}, true), { + applicationType: 'native', + authMethod: 'client_secret_basic', + secret: true, + }); + }); + + it('a confidential client with a custom-scheme redirect registers as native and is issued a client_secret', async () => { + await expectRegisteredAndPkceBound( + cardBodyWith({ token_endpoint_auth_method: 'client_secret_basic', redirect_uris: [CUSTOM_SCHEME] }), + { applicationType: 'native', authMethod: 'client_secret_basic', secret: true }, + ); + }); + + it('ANY qualifying redirect classifies the body: a loopback plus https://evil.example/cb registers as native', async () => { + await expectRegisteredAndPkceBound(cardBodyWith({ redirect_uris: [LOOPBACK, HTTPS_FOREIGN] }), { + applicationType: 'native', + authMethod: 'none', + secret: false, + }); + }); + }); +}); + +describe("DCR off: 'infer' opens nothing while the registration door is shut", () => { + it("refuses the card's own body with access_denied", async () => { + const opts = await captureProviderOptions({ OS_MCP_SERVER_ENABLED: 'true', OS_OIDC_DCR_ENABLED: 'false' }); + const server = await bootRealAuthorizationServer(opts); + const reg = await register(server.auth, CARD_BODY); + expect(reg.status, JSON.stringify(reg.body)).toBe(403); + expect(reg.body?.error, JSON.stringify(reg.body)).toBe('access_denied'); + expect(server.clientCount(), 'no client is stored').toBe(0); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 32d727067e6..9c00cadb68a 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -4371,6 +4371,47 @@ export class AuthManager { // an anonymous registration mints no authority by itself. allowDynamicClientRegistration: dcr, allowUnauthenticatedClientRegistration: dcr, + // ── Native MCP clients that omit `application_type` (#25118, ruling 6329352251, A1) ── + // Local MCP clients (Claude Code, Cursor, VS Code, Codex CLI) register + // over DCR with a loopback `http` or a custom-scheme redirect, as RFC + // 8252 §7.3 and the MCP authorization spec have native clients do, and + // send no `application_type`. The provider's default fills a missing + // type with `web` (OIDC Registration §2), whose redirect rule refuses + // both forms, so every such client was refused before its browser + // opened. + // + // `'infer'` is the provider's own classifier; there is no second rule + // here. A dynamic registration that omits the type becomes `native` + // when ANY redirect uses a non-http(s) scheme or `http` on exactly + // `localhost`, `127.0.0.1` or `[::1]`, and each redirect is then held to + // the native rule; otherwise it stays `web`. A type the client sends is + // never overridden. It acts on `/oauth2/register` only (the Setup + // form's managed `create-client` door is unaffected) and only while + // `dcr` is on: with DCR off the provider refuses every registration. + // + // The measured boundary, pinned on the real provider in + // auth-manager.mcp-oauth-dcr-infer.test.ts: + // • a public client with a loopback or custom-scheme redirect → `native`; + // • `https://evil.example/cb` alone stays `web`; `http://evil.example/cb`, + // `http://localhost.evil.example`, and an explicit `web` with a + // loopback redirect stay refused; + // • NOT "public clients only": the classifier reads redirect URIs and + // never `token_endpoint_auth_method`, so a CONFIDENTIAL registration + // (`client_secret_basic`, or the method omitted, which the provider + // fills as `client_secret_basic`) with a loopback or custom-scheme + // redirect and no type also registers as `native` and is issued a + // `client_secret`. That body was already accepted with + // `application_type: 'native'` spelled out, so no registrable client + // shape is added; only the meaning of an omitted field changes + // (RFC 8252 §8.4 allows a per-instance secret provisioned by DCR); + // • PKCE stays required at /oauth2/authorize for every one of these + // clients: unconditionally for a public client, and for a + // confidential DCR client because `clientRegistrationRequirePKCE` + // keeps its `true` default. ⛔ Do not pass it as `false`. + // + // ⛔ No hook on `/oauth2/register` and no second classification rule + // beside this one: ruling A1 declined both. + clientRegistrationDefaultApplicationType: 'infer', // ── The organization an OAuth consent binds (ruling 6232069920, A) ── // At consent the consenting session's active organization becomes the // consent's `reference_id`, and the installed provider carries that