From 7f27627a903e0ea5b764fbd9f9571b30de4b926e Mon Sep 17 00:00:00 2001 From: can1357 Date: Tue, 26 May 2026 20:19:24 +0200 Subject: [PATCH] feat(mcp/oauth): added RFC 8414 path-ful issuer form to OAuth discovery - Extended `buildWellKnownUrls` and `#resolveRegistrationEndpoint` to try `/.well-known//` as a third candidate after origin-root and path-prefixed forms. - Fixed single-segment path handling so `/my-service` is treated as the gateway prefix rather than dropped. - Fixed missing `await` on `#tryWellKnownForRegistration` that caused path-prefixed fallback to return an unresolved Promise. - Added tests for single-segment prefix discovery and RFC 8414 path-ful issuer fallback. --- packages/coding-agent/CHANGELOG.md | 6 +- .../coding-agent/src/mcp/oauth-discovery.ts | 29 ++++++++- packages/coding-agent/src/mcp/oauth-flow.ts | 18 +++++- .../coding-agent/test/oauth-discovery.test.ts | 60 +++++++++++++++++++ 4 files changed, 102 insertions(+), 11 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1f2af5620..fdeafff26 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -29,8 +29,6 @@ ## [15.4.1] - 2026-05-26 -### Breaking Changes - ### Breaking Changes - The `vim` edit mode option is no longer available; configurations using `edit.mode: vim` will be automatically mapped to `hashline` mode - Hashline payload semantics are now strictly inline-first: the first payload line is whatever follows the sigil on the op line itself, and subsequent lines append after it. A newline immediately after `↑`/`↓`/`:` is no longer a free separator — it produces a blank first payload line. Use `LINE↓content` for a one-line insert, `LINE↓firstline\nsecondline` for two lines; bare `LINE↓` / `LINE↑` / `LINE:` (no inline payload) still insert/replace with one blank line as before. @@ -82,9 +80,6 @@ - Updated `parseMcpAuthServerUrl` and `extractMcpAuthServerUrl` to accept optional `serverUrl` for relative URL resolution ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) - Updated `MCPOAuthFlow.#resolveRegistrationEndpoint` to try origin-root well-known first, then fall back to path-prefixed well-known ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) -### Fixed -- Fixed missing `await` on `#tryWellKnownForRegistration` call in `#resolveRegistrationEndpoint` that caused path-prefixed well-known fallback to never actually execute, returning the unresolved Promise object instead of the registration endpoint ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) - ### Removed - Removed the `installH2Fetch()` activation from CLI startup; HTTPS fetches now use Bun's default transport - Removed the `vim` edit mode along with the `VimTool` module, prompt, and supporting buffer/engine/renderer stack @@ -97,6 +92,7 @@ ### Fixed +- Fixed missing `await` on `#tryWellKnownForRegistration` call in `#resolveRegistrationEndpoint` that caused path-prefixed well-known fallback to never actually execute, returning the unresolved Promise object instead of the registration endpoint ([1407](https://github.com/can1357/oh-my-pi/pull/1407) by [@faizhasim](https://github.com/faizhasim)) - Fixed JavaScript module reloading to refresh local re-exports when transitive dependency files are edited - Fixed Python tool calls in warm kernels to initialize once bridge environment variables appear after startup and to return a clear `tool bridge is unavailable` error when missing - Fixed IRC `send` handling to preserve recipient incoming messages when auto-reply timeouts instead of dropping them diff --git a/packages/coding-agent/src/mcp/oauth-discovery.ts b/packages/coding-agent/src/mcp/oauth-discovery.ts index 1224d1abd..1a1db88e3 100644 --- a/packages/coding-agent/src/mcp/oauth-discovery.ts +++ b/packages/coding-agent/src/mcp/oauth-discovery.ts @@ -407,8 +407,31 @@ function buildWellKnownUrls(wellKnownPath: string, baseUrl: string): URL[] { const normalizedPath = parsed.pathname.replace(/\/$/, ""); const lastSlash = normalizedPath.lastIndexOf("/"); - if (lastSlash <= 0) return [absUrl]; + // Bare origin (no path beyond "/") — only the origin-root candidate applies. + if (lastSlash < 0) return [absUrl]; - const relUrl = new URL(wellKnownPath.slice(1), `${parsed.origin}${normalizedPath.slice(0, lastSlash)}/`); - return relUrl.href !== absUrl.href ? [absUrl, relUrl] : [absUrl]; + // Path-prefixed well-known (common for gateways with sub-path routing). + // Multi-segment paths drop the trailing segment (typically the MCP endpoint); + // single-segment paths (lastSlash === 0) are themselves the gateway prefix. + const prefixPath = lastSlash === 0 ? normalizedPath : normalizedPath.slice(0, lastSlash); + const relUrl = new URL(wellKnownPath.slice(1), `${parsed.origin}${prefixPath}/`); + + const candidates: URL[] = [absUrl]; + const seen = new Set([absUrl.href]); + const push = (u: URL): void => { + if (!seen.has(u.href)) { + candidates.push(u); + seen.add(u.href); + } + }; + push(relUrl); + + // RFC 8414 §3.1 path-ful issuer form: /.well-known//. + // Only meaningful for well-known metadata documents. + if (wellKnownPath.startsWith("/.well-known/")) { + const pathfulUrl = new URL(`${wellKnownPath}${normalizedPath}`, parsed.origin); + push(pathfulUrl); + } + + return candidates; } diff --git a/packages/coding-agent/src/mcp/oauth-flow.ts b/packages/coding-agent/src/mcp/oauth-flow.ts index 72d8c44bc..4de143cbc 100644 --- a/packages/coding-agent/src/mcp/oauth-flow.ts +++ b/packages/coding-agent/src/mcp/oauth-flow.ts @@ -334,13 +334,25 @@ export class MCPOAuthFlow extends OAuthCallbackFlow { // path-prefixed well-known for gateways (e.g. https://gateway.example.com/my-service/). const normalizedPath = authorizationUrl.pathname.replace(/\/$/, ""); const lastSlash = normalizedPath.lastIndexOf("/"); - if (lastSlash <= 0) return null; + // Bare-origin authorization URL — nothing further to try. + if (lastSlash < 0) return null; + // Single-segment paths are the gateway prefix itself; multi-segment paths + // drop the trailing segment (typically a service endpoint). + const prefixPath = lastSlash === 0 ? normalizedPath : normalizedPath.slice(0, lastSlash); const prefixedUrl = new URL( ".well-known/oauth-authorization-server", - `${authorizationUrl.origin}${normalizedPath.slice(0, lastSlash)}/`, + `${authorizationUrl.origin}${prefixPath}/`, ).toString(); - return await this.#tryWellKnownForRegistration(prefixedUrl); + const prefixedEndpoint = await this.#tryWellKnownForRegistration(prefixedUrl); + if (prefixedEndpoint) return prefixedEndpoint; + + // RFC 8414 §3.1 path-ful issuer form: /.well-known/oauth-authorization-server/. + const pathfulUrl = new URL( + `/.well-known/oauth-authorization-server${normalizedPath}`, + authorizationUrl.origin, + ).toString(); + return await this.#tryWellKnownForRegistration(pathfulUrl); } async #tryWellKnownForRegistration(wellKnownUrl: string): Promise { diff --git a/packages/coding-agent/test/oauth-discovery.test.ts b/packages/coding-agent/test/oauth-discovery.test.ts index a9c7de08f..4748234d7 100644 --- a/packages/coding-agent/test/oauth-discovery.test.ts +++ b/packages/coding-agent/test/oauth-discovery.test.ts @@ -88,6 +88,66 @@ describe("path-prefixed auth servers", () => { expect(calls).toContain("https://gateway.example.com/my-service/.well-known/oauth-authorization-server"); }); + it("discovers endpoints via single-segment path prefix (no trailing endpoint segment)", async () => { + const calls: string[] = []; + using _hook = hookFetch(input => { + const url = String(input); + calls.push(url); + + if (url === "https://gateway.example.com/.well-known/oauth-authorization-server") { + return new Response("not found", { status: 404 }); + } + if (url === "https://gateway.example.com/my-service/.well-known/oauth-authorization-server") { + return new Response( + JSON.stringify({ + authorization_endpoint: "https://gateway.example.com/my-service/oauth/authorize", + token_endpoint: "https://gateway.example.com/my-service/oauth/token", + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + + return new Response("not found", { status: 404 }); + }); + + const oauth = await discoverOAuthEndpoints("https://gateway.example.com/my-service"); + + expect(oauth).toEqual({ + authorizationUrl: "https://gateway.example.com/my-service/oauth/authorize", + tokenUrl: "https://gateway.example.com/my-service/oauth/token", + }); + expect(calls[0]).toBe("https://gateway.example.com/.well-known/oauth-authorization-server"); + expect(calls).toContain("https://gateway.example.com/my-service/.well-known/oauth-authorization-server"); + }); + + it("falls back to RFC 8414 path-ful issuer form (/.well-known/oauth-authorization-server/)", async () => { + const calls: string[] = []; + using _hook = hookFetch(input => { + const url = String(input); + calls.push(url); + + if (url === "https://gateway.example.com/.well-known/oauth-authorization-server/my-service") { + return new Response( + JSON.stringify({ + authorization_endpoint: "https://gateway.example.com/my-service/oauth", + token_endpoint: "https://gateway.example.com/my-service/token", + }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + } + + return new Response("not found", { status: 404 }); + }); + + const oauth = await discoverOAuthEndpoints("https://gateway.example.com/my-service"); + + expect(oauth).toEqual({ + authorizationUrl: "https://gateway.example.com/my-service/oauth", + tokenUrl: "https://gateway.example.com/my-service/token", + }); + expect(calls).toContain("https://gateway.example.com/.well-known/oauth-authorization-server/my-service"); + }); + it("prefers absolute well-known when it succeeds (origin-root servers still work)", async () => { const calls: string[] = []; using _hook = hookFetch(input => {