diff --git a/apps/vscode-e2e/src/suite/mcp-oauth.test.ts b/apps/vscode-e2e/src/suite/mcp-oauth.test.ts index 1cca9aa76d..0a1219ae88 100644 --- a/apps/vscode-e2e/src/suite/mcp-oauth.test.ts +++ b/apps/vscode-e2e/src/suite/mcp-oauth.test.ts @@ -128,7 +128,13 @@ suite("Roo Code MCP OAuth", function () { // SDK constructs: new URL("/.well-known/oauth-authorization-server", "http://host/auth") // which resolves to http://host/.well-known/oauth-authorization-server (origin-relative) - if (url === "/.well-known/oauth-authorization-server") { + // Our custom fetchOAuthAuthServerMetadata constructs the RFC 8414 URL with issuer path: + // /.well-known/oauth-authorization-server/auth (with issuer path) + // Handle BOTH forms so our provider gets _authServerMeta. + if ( + url === "/.well-known/oauth-authorization-server" || + url === "/.well-known/oauth-authorization-server/auth" + ) { endpointsHit.add("auth-metadata") res.writeHead(200, { "Content-Type": "application/json" }) res.end( diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index b14b777a8f..aa1dc3abec 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -985,7 +985,7 @@ export class McpHub { // and hits the same broken URL for path-prefixed issuers (see // utils/oauth.ts for upstream issue links). await authProvider.exchangeCodeForTokens(code) - await authProvider.close() + authProvider.close().catch(console.error) // Recover the validated server config stored on the connection so we // can pass it directly to connectToServer without re-reading the file. diff --git a/src/services/mcp/McpOAuthClientProvider.ts b/src/services/mcp/McpOAuthClientProvider.ts index 9e2f318a1d..eb38b62f11 100644 --- a/src/services/mcp/McpOAuthClientProvider.ts +++ b/src/services/mcp/McpOAuthClientProvider.ts @@ -156,6 +156,21 @@ export class McpOAuthClientProvider implements OAuthClientProvider { */ async registerClientIfNeeded(): Promise { if (this._clientInfo) return // already registered + + // Check if we have a cached client_id from previous registration + const cachedData = await this._secretStorage.getOAuthData(this._serverUrl) + if (cachedData?.client_id && cachedData.redirect_uri === this.redirectUrl) { + this._clientInfo = { + client_id: cachedData.client_id, + redirect_uris: [this.redirectUrl], + client_name: this._clientName, + grant_types: this._grantTypes, + response_types: ["code"], + token_endpoint_auth_method: this._tokenEndpointAuthMethod, + } + return + } + if (!this._authServerMeta?.registration_endpoint) return // DCR not supported const response = await fetch(this._authServerMeta.registration_endpoint as string, { @@ -185,7 +200,12 @@ export class McpOAuthClientProvider implements OAuthClientProvider { async saveTokens(tokens: OAuthTokens): Promise { const expires_at = tokens.expires_in ? Date.now() + tokens.expires_in * 1000 : Date.now() + 3600 * 1000 // default 1 hour when server omits expires_in - await this._secretStorage.saveOAuthData(this._serverUrl, { tokens, expires_at }) + await this._secretStorage.saveOAuthData(this._serverUrl, { + tokens, + expires_at, + client_id: this._clientInfo?.client_id, + redirect_uri: this.redirectUrl, + }) } async redirectToAuthorization(authorizationUrl: URL): Promise { diff --git a/src/services/mcp/SecretStorageService.ts b/src/services/mcp/SecretStorageService.ts index d4d1684e54..e320b2ba6d 100644 --- a/src/services/mcp/SecretStorageService.ts +++ b/src/services/mcp/SecretStorageService.ts @@ -5,6 +5,10 @@ export interface StoredMcpOAuthData { tokens: OAuthTokens /** Unix ms timestamp after which the access token should be considered expired. */ expires_at: number + /** The client_id used to obtain these tokens (for token reuse without re-registration). */ + client_id?: string + /** The redirect_uri used during client registration (to detect port changes). */ + redirect_uri?: string } /** diff --git a/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts b/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts index 75f9b1486c..b5863b3dd0 100644 --- a/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts +++ b/src/services/mcp/__tests__/McpOAuthClientProvider.spec.ts @@ -39,6 +39,7 @@ mockFetch.mockResolvedValue({ issuer: "https://auth.example.com", authorization_endpoint: "https://auth.example.com/authorize", token_endpoint: "https://auth.example.com/token", + registration_endpoint: "https://auth.example.com/register", response_types_supported: ["code"], token_endpoint_auth_methods_supported: ["none"], grant_types_supported: ["authorization_code", "refresh_token"], @@ -543,4 +544,72 @@ describe("McpOAuthClientProvider", () => { expect(stopCallbackServer).toHaveBeenCalledTimes(1) }) }) + + describe("registerClientIfNeeded", () => { + it("should reuse cached client_id when redirect_uri matches", async () => { + setupCallbackServerMock() + const secretStorage = createMockSecretStorage() + + // Pre-populate storage with cached data + await secretStorage.saveOAuthData("https://example.com/mcp", { + tokens: { access_token: "cached-token", token_type: "Bearer" }, + expires_at: Date.now() + 3600000, + client_id: "cached-client-id", + redirect_uri: "http://localhost:12345/callback", + }) + + const provider = await McpOAuthClientProvider.create("https://example.com/mcp", secretStorage) + await provider.registerClientIfNeeded() + + expect((await provider.clientInformation())?.client_id).toBe("cached-client-id") + await provider.close() + }) + + it("should not reuse cached client_id when redirect_uri does not match", async () => { + setupCallbackServerMock() + const secretStorage = createMockSecretStorage() + + // Clear previous mocks and set up for this test + mockFetch.mockClear() + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + issuer: "https://auth.example.com", + authorization_endpoint: "https://auth.example.com/authorize", + token_endpoint: "https://auth.example.com/token", + registration_endpoint: "https://auth.example.com/register", + response_types_supported: ["code"], + token_endpoint_auth_methods_supported: ["none"], + grant_types_supported: ["authorization_code", "refresh_token"], + }), + }) + mockFetch.mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ + client_id: "new-client-id", + redirect_uris: ["http://localhost:12345/callback"], + client_name: "Roo Code", + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }), + }) + + // Pre-populate storage with cached data with different redirect_uri + await secretStorage.saveOAuthData("https://example.com/mcp", { + tokens: { access_token: "cached-token", token_type: "Bearer" }, + expires_at: Date.now() + 3600000, + client_id: "cached-client-id", + redirect_uri: "http://localhost:99999/callback", // different port + }) + + const provider = await McpOAuthClientProvider.create("https://example.com/mcp", secretStorage) + await provider.registerClientIfNeeded() + + expect((await provider.clientInformation())?.client_id).toBe("new-client-id") + await provider.close() + }) + }) })