From 59f9e1941bc61186ff83df4fe2f6aae100bd0775 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 20:37:44 +0000 Subject: [PATCH] Explain OAuth client-registration failures during login Logging in to a server that refuses Dynamic Client Registration (e.g. Figma's remote MCP server, which 403s any client not on its allow-list with a plain-text body) surfaced the SDK's raw JSON parse error: HTTP 403: Invalid OAuth error response: SyntaxError: Unexpected token 'F', "Forbidden" is not valid JSON. Raw body: Forbidden performOAuthFlow now rewrites registration-phase failures (detected via the SDK's typed OAuthError plus a reachedAuthorization flag set at the authorization redirect) into actionable guidance pointing at --client-id / --client-metadata-url. Allow-list wording is used only for 401/403 or OAuth-coded client rejections; other registration failures (5xx outages, rejected metadata) are reported honestly, and the server's underlying response stays visible in the message. Post-redirect errors pass through unchanged. Supersedes #297. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01AP2pmUcWvR8rNypXqNEqtk --- CHANGELOG.md | 4 + src/lib/auth/oauth-flow.ts | 130 +++++++++++++++++++++++++- test/unit/lib/auth/oauth-flow.test.ts | 124 ++++++++++++++++++++++++ 3 files changed, 256 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d827eda4..eed1aeb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `mcpc login` now explains OAuth client-registration failures with actionable guidance (use `--client-id` or `--client-metadata-url`) instead of surfacing a raw SDK JSON parse error. This affects servers that reject Dynamic Client Registration, such as Figma's remote MCP server, which only accepts an allow-list of approved clients. + ## [0.4.0] - 2026-06-25 ### Added diff --git a/src/lib/auth/oauth-flow.ts b/src/lib/auth/oauth-flow.ts index d1478ad5..a237dd36 100644 --- a/src/lib/auth/oauth-flow.ts +++ b/src/lib/auth/oauth-flow.ts @@ -10,10 +10,12 @@ import { URL } from 'url'; import { createInterface } from 'readline'; import { randomBytes } from 'crypto'; import { auth as sdkAuth } from '@modelcontextprotocol/sdk/client/auth.js'; +import { OAuthError as SdkOAuthError } from '@modelcontextprotocol/sdk/server/auth/errors.js'; import { OAuthProvider, type OAuthProviderOptions } from './oauth-provider.js'; import { getServerHost } from '../utils.js'; -import { ClientError } from '../errors.js'; +import { AuthError, ClientError } from '../errors.js'; import { createLogger } from '../logger.js'; +import { describeAuthError } from './client-credentials.js'; import { removeKeychainOAuthClientInfo, storeKeychainOAuthClientInfo } from './keychain.js'; import type { AuthProfile } from '../types.js'; import { @@ -25,6 +27,118 @@ import { renderAuthPage } from './auth-page.js'; const logger = createLogger('oauth-flow'); +/** Matches the MCP SDK error thrown when a server exposes no `registration_endpoint`. */ +const DCR_UNSUPPORTED_PATTERN = /does not support dynamic client registration/i; + +/** + * OAuth `error` codes that mean the authorization server rejected mcpc as a + * client, as opposed to a malformed request or a server-side fault. + */ +const CLIENT_REJECTED_CODES = new Set(['invalid_client', 'unauthorized_client', 'access_denied']); + +/** + * Cap on how much of the server's error response is echoed back to the user. + * The SDK embeds the full raw response body in its message, which can be an + * entire HTML error page. + */ +const MAX_DETAIL_LENGTH = 400; + +/** + * Rewrite a raw MCP SDK OAuth error into an actionable one when the failure + * happened during dynamic client registration (DCR) — that is, before the user + * was ever redirected to the authorization endpoint. + * + * Many hosted MCP servers do not allow open DCR: they either expose no + * `registration_endpoint`, or the endpoint rejects unknown clients. Figma's + * remote MCP server is a concrete example — its registration endpoint returns + * a bare `403 Forbidden` for any client not on its approved allow-list. + * Because that body is not valid OAuth-error JSON, the SDK surfaces it as the + * opaque `HTTP 403: Invalid OAuth error response: ... Raw body: Forbidden`, + * which gives the user no idea what to do next. Retrying cannot help; the user + * must supply pre-registered client credentials (or a custom CIMD document). + * + * Phase detection: before the authorization redirect the only SDK request that + * surfaces {@link SdkOAuthError} is client registration — discovery failures + * are swallowed by the SDK, and the token exchange (plus its refresh variant, + * disabled here by `forceReauth`) only runs after the redirect. Errors thrown + * after the redirect, and pre-redirect errors unrelated to registration + * (network faults, user cancellation), are returned unchanged so their own + * messaging is preserved. + * + * @param error - The error thrown by the SDK auth flow. + * @param context - The normalized server URL and whether the flow reached the + * authorization redirect. + * @returns An {@link AuthError} with remediation guidance when the failure is + * registration-related, otherwise the original error unchanged. + */ +export function explainOAuthRegistrationFailure( + error: unknown, + context: { serverUrl: string; reachedAuthorization: boolean } +): unknown { + // A failure after the authorization redirect (token exchange, user + // cancellation, callback problems) is not a registration problem. + if (context.reachedAuthorization) { + return error; + } + + const detail = describeAuthError(error); + const dcrUnsupported = DCR_UNSUPPORTED_PATTERN.test(detail); + const isRegistrationError = error instanceof SdkOAuthError; + // The HTTP status only appears in the message for non-JSON error bodies + // (the SDK's `Invalid OAuth error response` case); OAuth-coded rejections + // carry a machine-readable errorCode instead. Keeping the status fallback + // independent of the instanceof check also preserves the rewrite if the SDK + // ever stops throwing typed errors from the registration path. + const status = /\bHTTP (\d{3})\b/.exec(detail)?.[1]; + const rejectedByServer = + status === '401' || + status === '403' || + (error instanceof SdkOAuthError && CLIENT_REJECTED_CODES.has(error.errorCode)); + + if (!dcrUnsupported && !isRegistrationError && !rejectedByServer) { + return error; + } + + const host = getServerHost(context.serverUrl); + const truncatedDetail = + detail.length > MAX_DETAIL_LENGTH ? `${detail.slice(0, MAX_DETAIL_LENGTH)}…` : detail; + + const lines: string[] = []; + if (dcrUnsupported) { + lines.push( + `${host} does not support Dynamic Client Registration, and no pre-registered ` + + `client credentials were provided.` + ); + } else if (rejectedByServer) { + lines.push( + `${host} refused to register mcpc as an OAuth client${status ? ` (HTTP ${status})` : ''}.`, + `Some MCP servers only accept a fixed allow-list of approved clients and reject ` + + `Dynamic Client Registration from everyone else (Figma's remote MCP server behaves ` + + `this way). When that happens there is no self-service client for mcpc to register.` + ); + } else { + // Any other registration failure (e.g. a 5xx outage or a rejected client + // metadata field): report it honestly without claiming an allow-list. + lines.push(`Client registration with ${host} failed: ${truncatedDetail}`); + } + + lines.push( + '', + 'To authenticate, supply a client the server already recognizes:', + ` • pre-registered client: mcpc login ${host} --client-id [--client-secret ]`, + ` • custom CIMD document: mcpc login ${host} --client-metadata-url `, + '', + `If the server restricts MCP access to specific approved clients, check with the ` + + `provider whether third-party clients such as mcpc are supported.` + ); + + if (rejectedByServer && !dcrUnsupported) { + lines.push('', `Server response: ${truncatedDetail}`); + } + + return new AuthError(lines.join('\n'), { originalError: detail }); +} + // Special key codes const ESCAPE_KEY = '\x1b'; const CTRL_C = '\x03'; @@ -498,6 +612,11 @@ export async function performOAuthFlow( // Track whether browser failed so we can fall back to URL paste let browserFailed = false; + // Track whether the flow reached the authorization redirect. Used to + // distinguish discovery/registration failures (before this point) from + // failures during user interaction or code exchange (after this point). + let reachedAuthorization = false; + // Handler refs for cleanup const pasteHandlerRef: { current: { cleanup: () => void } | null } = { current: null }; @@ -518,6 +637,10 @@ export async function performOAuthFlow( // Override redirectToAuthorization to open browser provider.redirectToAuthorization = async (authorizationUrl: URL) => { + // Reaching this point means discovery and client registration both + // succeeded — any later failure is not a registration problem. + reachedAuthorization = true; + // Inject a random `state` parameter if the SDK didn't add one. OAuth 2.1 treats // `state` as RECOMMENDED rather than REQUIRED, but RFC 6749 §4.1.1 allows servers // to require it, and some production MCP servers do (e.g. Ubersuggest). PKCE @@ -614,7 +737,10 @@ export async function performOAuthFlow( }; } catch (error) { logger.debug(`OAuth flow failed: ${(error as Error).message}`); - throw error; + throw explainOAuthRegistrationFailure(error, { + serverUrl: normalizedServerUrl, + reachedAuthorization, + }); } finally { // Clean up paste handler in case of error pasteHandlerRef.current?.cleanup(); diff --git a/test/unit/lib/auth/oauth-flow.test.ts b/test/unit/lib/auth/oauth-flow.test.ts index 78efb711..3506c244 100644 --- a/test/unit/lib/auth/oauth-flow.test.ts +++ b/test/unit/lib/auth/oauth-flow.test.ts @@ -2,7 +2,131 @@ * Unit tests for OAuth flow utility functions */ +import { + InvalidClientError, + InvalidClientMetadataError, + ServerError as SdkServerError, +} from '@modelcontextprotocol/sdk/server/auth/errors.js'; import { validateClientMetadataUrl } from '../../../../src/lib/auth/oauth-utils.js'; +import { explainOAuthRegistrationFailure } from '../../../../src/lib/auth/oauth-flow.js'; +import { AuthError } from '../../../../src/lib/errors.js'; + +describe('explainOAuthRegistrationFailure', () => { + const serverUrl = 'https://mcp.figma.com/mcp'; + const preAuth = { serverUrl, reachedAuthorization: false }; + + it('rewrites the SDK non-JSON 403 error (Figma) into allow-list guidance', () => { + // The exact error the MCP SDK surfaces for Figma's plain-text "Forbidden" body. + const raw = new SdkServerError( + "HTTP 403: Invalid OAuth error response: SyntaxError: Unexpected token 'F', " + + '"Forbidden" is not valid JSON. Raw body: Forbidden' + ); + + const result = explainOAuthRegistrationFailure(raw, preAuth); + + expect(result).toBeInstanceOf(AuthError); + const message = (result as AuthError).message; + expect(message).toContain('mcp.figma.com refused to register mcpc'); + expect(message).toContain('(HTTP 403)'); + expect(message).toContain('--client-id'); + expect(message).toContain('--client-metadata-url'); + // The underlying server response stays visible (login prints only error.message). + expect(message).toContain('Server response:'); + expect(message).toContain('Raw body: Forbidden'); + expect((result as AuthError).details).toEqual({ originalError: raw.message }); + }); + + it('recognizes a 401/403 registration rejection even when the error is untyped', () => { + const raw = new Error('HTTP 401: Invalid OAuth error response: foo. Raw body: Unauthorized'); + + const result = explainOAuthRegistrationFailure(raw, preAuth); + + expect(result).toBeInstanceOf(AuthError); + expect((result as AuthError).message).toContain('refused to register mcpc'); + }); + + it('recognizes OAuth-coded client rejections without an HTTP status', () => { + const raw = new InvalidClientError('mcpc is not an approved client'); + + const result = explainOAuthRegistrationFailure(raw, preAuth); + + expect(result).toBeInstanceOf(AuthError); + const message = (result as AuthError).message; + expect(message).toContain('refused to register mcpc as an OAuth client.'); + expect(message).toContain('Server response: mcpc is not an approved client'); + }); + + it('explains servers that expose no registration endpoint', () => { + const raw = new Error('Incompatible auth server: does not support dynamic client registration'); + + const result = explainOAuthRegistrationFailure(raw, preAuth); + + expect(result).toBeInstanceOf(AuthError); + const message = (result as AuthError).message; + expect(message).toContain('does not support Dynamic Client Registration'); + expect(message).toContain('--client-id'); + }); + + it('does not claim an allow-list for a registration 5xx', () => { + const raw = new SdkServerError( + 'HTTP 500: Invalid OAuth error response: SyntaxError: bad. Raw body: outage' + ); + + const result = explainOAuthRegistrationFailure(raw, preAuth); + + expect(result).toBeInstanceOf(AuthError); + const message = (result as AuthError).message; + expect(message).toContain('Client registration with mcp.figma.com failed:'); + expect(message).not.toContain('refused to register'); + expect(message).not.toContain('allow-list'); + expect(message).toContain('--client-id'); + }); + + it('reports rejected client metadata without claiming an allow-list', () => { + const raw = new InvalidClientMetadataError('redirect_uri is not allowed'); + + const result = explainOAuthRegistrationFailure(raw, preAuth); + + expect(result).toBeInstanceOf(AuthError); + const message = (result as AuthError).message; + expect(message).toContain('Client registration with mcp.figma.com failed:'); + expect(message).toContain('redirect_uri is not allowed'); + expect(message).not.toContain('refused to register'); + }); + + it('truncates oversized response bodies in the message but keeps details intact', () => { + const body = 'x'.repeat(2000); + const raw = new SdkServerError( + `HTTP 403: Invalid OAuth error response: bad. Raw body: ${body}` + ); + + const result = explainOAuthRegistrationFailure(raw, preAuth); + + const message = (result as AuthError).message; + expect(message).toContain('…'); + expect(message).not.toContain(body); + expect((result as AuthError).details).toEqual({ originalError: raw.message }); + }); + + it('leaves the error unchanged once authorization has been reached', () => { + // A 403 after the redirect is a token-exchange failure, not registration. + const raw = new SdkServerError('HTTP 403: Invalid OAuth error response. Raw body: Forbidden'); + + const result = explainOAuthRegistrationFailure(raw, { serverUrl, reachedAuthorization: true }); + + expect(result).toBe(raw); + }); + + it('leaves unrelated pre-authorization errors unchanged', () => { + for (const raw of [ + new Error('connect ECONNREFUSED 127.0.0.1:3845'), + new Error('Forbidden'), // no HTTP status, not an SDK OAuth error + new Error('Authentication cancelled by user'), + ]) { + expect(explainOAuthRegistrationFailure(raw, preAuth)).toBe(raw); + } + }); +}); describe('validateClientMetadataUrl', () => { it('accepts a valid HTTPS URL with path', () => {