mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-06 02:47:56 +00:00
fix(McpHub): not blocking on authprovider flow
This commit is contained in:
parent
0afa23ce99
commit
3121d36e55
5 changed files with 102 additions and 3 deletions
|
|
@ -128,7 +128,13 @@ suite("Roo Code MCP OAuth", function () {
|
||||||
|
|
||||||
// SDK constructs: new URL("/.well-known/oauth-authorization-server", "http://host/auth")
|
// SDK constructs: new URL("/.well-known/oauth-authorization-server", "http://host/auth")
|
||||||
// which resolves to http://host/.well-known/oauth-authorization-server (origin-relative)
|
// 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")
|
endpointsHit.add("auth-metadata")
|
||||||
res.writeHead(200, { "Content-Type": "application/json" })
|
res.writeHead(200, { "Content-Type": "application/json" })
|
||||||
res.end(
|
res.end(
|
||||||
|
|
|
||||||
|
|
@ -985,7 +985,7 @@ export class McpHub {
|
||||||
// and hits the same broken URL for path-prefixed issuers (see
|
// and hits the same broken URL for path-prefixed issuers (see
|
||||||
// utils/oauth.ts for upstream issue links).
|
// utils/oauth.ts for upstream issue links).
|
||||||
await authProvider.exchangeCodeForTokens(code)
|
await authProvider.exchangeCodeForTokens(code)
|
||||||
await authProvider.close()
|
authProvider.close().catch(console.error)
|
||||||
|
|
||||||
// Recover the validated server config stored on the connection so we
|
// Recover the validated server config stored on the connection so we
|
||||||
// can pass it directly to connectToServer without re-reading the file.
|
// can pass it directly to connectToServer without re-reading the file.
|
||||||
|
|
|
||||||
|
|
@ -156,6 +156,21 @@ export class McpOAuthClientProvider implements OAuthClientProvider {
|
||||||
*/
|
*/
|
||||||
async registerClientIfNeeded(): Promise<void> {
|
async registerClientIfNeeded(): Promise<void> {
|
||||||
if (this._clientInfo) return // already registered
|
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
|
if (!this._authServerMeta?.registration_endpoint) return // DCR not supported
|
||||||
|
|
||||||
const response = await fetch(this._authServerMeta.registration_endpoint as string, {
|
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> {
|
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
|
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> {
|
async redirectToAuthorization(authorizationUrl: URL): Promise<void> {
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,10 @@ export interface StoredMcpOAuthData {
|
||||||
tokens: OAuthTokens
|
tokens: OAuthTokens
|
||||||
/** Unix ms timestamp after which the access token should be considered expired. */
|
/** Unix ms timestamp after which the access token should be considered expired. */
|
||||||
expires_at: number
|
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
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
||||||
|
|
@ -39,6 +39,7 @@ mockFetch.mockResolvedValue({
|
||||||
issuer: "https://auth.example.com",
|
issuer: "https://auth.example.com",
|
||||||
authorization_endpoint: "https://auth.example.com/authorize",
|
authorization_endpoint: "https://auth.example.com/authorize",
|
||||||
token_endpoint: "https://auth.example.com/token",
|
token_endpoint: "https://auth.example.com/token",
|
||||||
|
registration_endpoint: "https://auth.example.com/register",
|
||||||
response_types_supported: ["code"],
|
response_types_supported: ["code"],
|
||||||
token_endpoint_auth_methods_supported: ["none"],
|
token_endpoint_auth_methods_supported: ["none"],
|
||||||
grant_types_supported: ["authorization_code", "refresh_token"],
|
grant_types_supported: ["authorization_code", "refresh_token"],
|
||||||
|
|
@ -543,4 +544,72 @@ describe("McpOAuthClientProvider", () => {
|
||||||
expect(stopCallbackServer).toHaveBeenCalledTimes(1)
|
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()
|
||||||
|
})
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue