diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 493bd5332..d65a9edc1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed MCP reauthentication continuing to an authorization URL without `client_id` after dynamic client registration fails; the registration error now blocks the flow with the provider response details ([#5852](https://github.com/can1357/oh-my-pi/issues/5852)). + ## [17.0.2] - 2026-07-17 ### Added diff --git a/packages/coding-agent/src/mcp/oauth-flow.ts b/packages/coding-agent/src/mcp/oauth-flow.ts index d0a35e08d..7655de77b 100644 --- a/packages/coding-agent/src/mcp/oauth-flow.ts +++ b/packages/coding-agent/src/mcp/oauth-flow.ts @@ -385,6 +385,9 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { async generateAuthUrl(state: string, redirectUri: string): Promise<{ url: string; instructions?: string }> { if (!this.#resolvedClientId) { await this.#tryRegisterClient(redirectUri); + if (!this.#resolvedClientId && this.#registrationFailure) { + throw this.#missingClientIdError(); + } } const authUrl = new URL(this.config.authorizationUrl); diff --git a/packages/coding-agent/test/oauth-flow.test.ts b/packages/coding-agent/test/oauth-flow.test.ts index 8824e2a2e..556aeee34 100644 --- a/packages/coding-agent/test/oauth-flow.test.ts +++ b/packages/coding-agent/test/oauth-flow.test.ts @@ -660,41 +660,41 @@ describe("mcp oauth flow", () => { expect(registrationCalled).toBe(false); }); - // Issue #4307: Figma's DCR endpoint 403s every request (only catalog-approved - // clients may connect). The old flow swallowed the 403 and threw a bare - // "OAuth provider requires client_id" with no way for the user to see that - // DCR was tried and rejected. The rewritten error must name the endpoint and - // status and point at the `oauth.clientId` workaround. - it("surfaces DCR endpoint and status when registration is rejected", async () => { + // Issue #5852: a rejected DCR request must stop reauthentication before an + // authorization URL without client_id reaches the browser. + it("blocks authorization when dynamic client registration is rejected", async () => { + let authorizationRequests = 0; const fetchImpl: FetchImpl = async input => { const url = String(input); - if (url === "https://www.figma.com/.well-known/oauth-authorization-server") { + if (url === "https://cropwise.example/oauth/register") { return new Response( - JSON.stringify({ registration_endpoint: "https://api.figma.com/v1/oauth/mcp/register" }), - { status: 200, headers: { "Content-Type": "application/json" } }, + JSON.stringify({ + error: "unapproved_client", + error_description: "client_name 'oh-my-pi' is not on the approved list.", + }), + { status: 403, headers: { "Content-Type": "application/json" } }, ); } - if (url === "https://api.figma.com/v1/oauth/mcp/register") { - return new Response("Forbidden", { status: 403 }); - } - if (url.startsWith("https://www.figma.com/oauth/mcp?")) { - return new Response("Parameter client_id is required", { status: 400 }); + if (url.startsWith("https://cropwise.example/authorize?")) { + authorizationRequests += 1; + return new Response("Missing required parameter: client_id", { status: 400 }); } throw new Error(`Unexpected fetch: ${url}`); }; const flow = new MCPOAuthFlow( { - authorizationUrl: "https://www.figma.com/oauth/mcp", - tokenUrl: "https://api.figma.com/v1/oauth/token", - registrationUrl: "https://api.figma.com/v1/oauth/mcp/register", + authorizationUrl: "https://cropwise.example/authorize", + tokenUrl: "https://cropwise.example/token", + registrationUrl: "https://cropwise.example/oauth/register", fetch: fetchImpl, }, {}, ); await expect(flow.generateAuthUrl("state", "http://127.0.0.1:53190/callback")).rejects.toThrow( - /dynamic client registration was rejected \(POST https:\/\/api\.figma\.com\/v1\/oauth\/mcp\/register → HTTP 403 — Forbidden\).*oauth\.clientId/s, + /HTTP 403.*unapproved_client.*approved list.*oauth\.clientId/s, ); + expect(authorizationRequests).toBe(0); }); it("names the missing-DCR case when no registration endpoint is advertised", async () => {