diff --git a/clients/cli/__tests__/stored-auth.test.ts b/clients/cli/__tests__/stored-auth.test.ts index 3fb5c0b1..bea1ad49 100644 --- a/clients/cli/__tests__/stored-auth.test.ts +++ b/clients/cli/__tests__/stored-auth.test.ts @@ -873,3 +873,86 @@ describe("--wait-for-auth", () => { expect(result.stderr).toContain("positive number of seconds"); }); }); + +/** + * The stored-token refresh path calls SDK discovery directly rather than + * through `InspectorClient.effectiveAuthFetch`, so it carries its own copy of + * the #2172 compatibility wrapper. This test injects no `discover`, because + * the wrapper lives in the *default* — injecting one would bypass exactly what + * is under test. + */ +describe("refreshStoredAuthToken discovery compatibility (#2172)", () => { + const REFRESHED = { + access_token: "refreshed-access-token", + token_type: "Bearer", + refresh_token: "rotated-refresh-token", + expires_in: 3600, + }; + + it("refreshes against an AS publishing RFC 8414 metadata at the OIDC path", async () => { + let base = ""; + let rfc8414Probes = 0; + const server: Server = createServer((req, res) => { + const path = req.url ?? ""; + if (path.startsWith("/.well-known/oauth-authorization-server")) { + rfc8414Probes += 1; + res.writeHead(404).end(); + return; + } + // Plain OAuth 2.0: no jwks_uri, no subject_types_supported, no + // id_token_signing_alg_values_supported — the shape the SDK rejects when + // it finds it under this filename. + if (path === "/mcp/.well-known/openid-configuration") { + res.writeHead(200, { "content-type": "application/json" }); + res.end( + JSON.stringify({ + issuer: `${base}/mcp`, + authorization_endpoint: `${base}/oauth/authorize`, + token_endpoint: `${base}/oauth/token`, + response_types_supported: ["code"], + grant_types_supported: ["authorization_code", "refresh_token"], + token_endpoint_auth_methods_supported: ["client_secret_post"], + }), + ); + return; + } + if (path === "/oauth/token") { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify(REFRESHED)); + return; + } + res.writeHead(404).end(); + }); + await new Promise((resolve) => { + server.listen(0, "127.0.0.1", resolve); + }); + const addr = server.address(); + base = + typeof addr === "object" && addr ? `http://127.0.0.1:${addr.port}` : ""; + const serverUrl = `${base}/mcp`; + const fixture = writeOAuthFixture({ + [serverUrl]: { + tokens: { refresh_token: "old-refresh", token_type: "Bearer" }, + clientInformation: { client_id: "cid", client_secret: "sec" }, + }, + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshStoredAuthToken(serverUrl, fixture)).resolves.toBe( + "refreshed-access-token", + ); + // The RFC 8414 location really was tried and really did 404, so the + // document could only have come from the OIDC path. + expect(rfc8414Probes).toBeGreaterThan(0); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining("/mcp/.well-known/openid-configuration"), + ); + } finally { + warn.mockRestore(); + rmSync(dirname(fixture), { recursive: true, force: true }); + await new Promise((resolve) => { + server.close(() => resolve()); + }); + } + }); +}); diff --git a/clients/cli/src/cli.ts b/clients/cli/src/cli.ts index af4bed16..6561e67b 100644 --- a/clients/cli/src/cli.ts +++ b/clients/cli/src/cli.ts @@ -52,6 +52,7 @@ import { getAuthorizationServerUrl, getAuthorizationServerUrlCandidates, } from "@inspector/core/auth/discovery.js"; +import { withRfc8414OidcCompat } from "@inspector/core/auth/oidcDiscoveryCompat.js"; import { writeStoreFile } from "@inspector/core/storage/store-io.js"; import { refreshAuthorization, @@ -329,7 +330,21 @@ export async function refreshStoredAuthToken( deps: RefreshStoredAuthDeps = {}, ): Promise { const refresh = deps.refresh ?? refreshAuthorization; - const discover = deps.discover ?? discoverAuthorizationServerMetadata; + // #2172: this path calls SDK discovery directly rather than through + // `InspectorClient.effectiveAuthFetch`, so it needs the same compatibility + // wrapper — otherwise a stored refresh token with no persisted + // `serverMetadata` still cannot refresh against an authorization server that + // publishes RFC 8414 metadata at the OIDC well-known path (Copilot). + const compatFetch = withRfc8414OidcCompat(fetch); + const discover: typeof discoverAuthorizationServerMetadata = + deps.discover ?? + ((authorizationServerUrl, options) => + discoverAuthorizationServerMetadata(authorizationServerUrl, { + ...options, + // A caller-supplied fetch is left alone — it is theirs to compose. The + // walker below passes no options, so in practice this wraps the global. + fetchFn: options?.fetchFn ?? compatFetch, + })); const snapshot = await readOAuthSnapshot(statePath); const servers = snapshot.servers as StoredServers; diff --git a/clients/web/src/test/core/auth/oidcDiscoveryCompat.test.ts b/clients/web/src/test/core/auth/oidcDiscoveryCompat.test.ts index 88ef8d20..87500e0c 100644 --- a/clients/web/src/test/core/auth/oidcDiscoveryCompat.test.ts +++ b/clients/web/src/test/core/auth/oidcDiscoveryCompat.test.ts @@ -75,6 +75,16 @@ describe("oidcDiscoveryCandidates", () => { it("returns nothing for an unparseable URL", () => { expect(oidcDiscoveryCandidates("not a url")).toEqual([]); }); + + it("does not claim a path that merely shares the well-known prefix", () => { + // A bare prefix match would treat this as a discovery request and replace + // its failed response with a document from a derived URL (Copilot). + expect( + oidcDiscoveryCandidates( + "https://as.example.com/.well-known/oauth-authorization-server-backup", + ), + ).toEqual([]); + }); }); describe("isRfc8414OnlyMetadata", () => { diff --git a/core/auth/oidcDiscoveryCompat.ts b/core/auth/oidcDiscoveryCompat.ts index 498749cc..b45d8afc 100644 --- a/core/auth/oidcDiscoveryCompat.ts +++ b/core/auth/oidcDiscoveryCompat.ts @@ -115,6 +115,13 @@ export function oidcDiscoveryCandidates(rfc8414Url: string): string[] { } if (!url.pathname.startsWith(RFC8414_WELL_KNOWN)) return []; const path = url.pathname.slice(RFC8414_WELL_KNOWN.length); + // A bare prefix match would also claim `/.well-known/oauth-authorization-server-backup` + // — a path this wrapper has no business rewriting, and one whose failed + // response it would replace with a document fetched from somewhere else + // entirely (Copilot). The SDK appends the authorization server's pathname, + // which always begins with `/`, so requiring the exact path or a `/` + // boundary is precisely the set it can emit. + if (path !== "" && !path.startsWith("/")) return []; // The authorization server had no path, so the SDK emitted a single OIDC // candidate at the origin. if (path === "") return [new URL(OIDC_WELL_KNOWN, url.origin).href];