fix(McpHub): not blocking on authprovider flow

This commit is contained in:
Elliott de Launay 2026-03-07 01:56:54 +00:00 committed by Elliott de Launay
parent 20f2b11b1e
commit 6e8b9cfb5a
No known key found for this signature in database
GPG key ID: BB899BED766D1806
5 changed files with 102 additions and 3 deletions

View file

@ -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(

View file

@ -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.

View file

@ -156,6 +156,21 @@ export class McpOAuthClientProvider implements OAuthClientProvider {
*/
async registerClientIfNeeded(): Promise<void> {
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<void> {
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<void> {

View file

@ -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
}
/**

View file

@ -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()
})
})
})