mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix(@modelcontextprotocol): overwrites
This commit is contained in:
Elliott de Launay
Elliott de Launay
parent
ae2aab2aa4
commit
29fa99d649
2 changed files with 226 additions and 0 deletions
148
src/services/mcp/utils/__tests__/oauth.spec.ts
Normal file
148
src/services/mcp/utils/__tests__/oauth.spec.ts
Normal file
|
|
@ -0,0 +1,148 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest"
|
||||
|
||||
// Mock the SDK's discoverOAuthProtectedResourceMetadata
|
||||
vi.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
|
||||
discoverOAuthProtectedResourceMetadata: vi.fn(),
|
||||
}))
|
||||
|
||||
import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/client/auth.js"
|
||||
import { fetchOAuthAuthServerMetadata } from "../oauth"
|
||||
|
||||
const mockFetch = vi.fn()
|
||||
global.fetch = mockFetch
|
||||
|
||||
describe("fetchOAuthAuthServerMetadata", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it("returns null when resource metadata has no authorization_servers", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({
|
||||
resource: "https://example.com/",
|
||||
authorization_servers: [],
|
||||
})
|
||||
|
||||
const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("returns null when discoverOAuthProtectedResourceMetadata throws", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockRejectedValue(new Error("network error"))
|
||||
|
||||
const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("constructs the RFC 8414 discovery URL correctly for an issuer with a path", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({
|
||||
resource: "https://mcp.kapa.ai/",
|
||||
authorization_servers: ["https://mcp.kapa.ai/auth/public"],
|
||||
})
|
||||
|
||||
const mockMeta = {
|
||||
issuer: "https://mcp.kapa.ai/auth/public",
|
||||
registration_endpoint: "https://mcp.kapa.ai/auth/public/register",
|
||||
}
|
||||
mockFetch.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(mockMeta) })
|
||||
|
||||
const result = await fetchOAuthAuthServerMetadata("https://mcp.kapa.ai/mcp")
|
||||
|
||||
// Verify the RFC 8414 §3.1 URL: well-known inserted between host and path
|
||||
expect(mockFetch).toHaveBeenCalledWith(
|
||||
"https://mcp.kapa.ai/.well-known/oauth-authorization-server/auth/public",
|
||||
expect.objectContaining({ headers: { Accept: "application/json" } }),
|
||||
)
|
||||
expect(result).toEqual({ authServerMeta: mockMeta, resourceIndicator: "https://mcp.kapa.ai/" })
|
||||
})
|
||||
|
||||
it("constructs the RFC 8414 discovery URL correctly for an issuer without a path", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({
|
||||
resource: "https://auth.example.com/",
|
||||
authorization_servers: ["https://auth.example.com"],
|
||||
})
|
||||
|
||||
const mockMeta = { issuer: "https://auth.example.com" }
|
||||
mockFetch.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(mockMeta) })
|
||||
|
||||
await fetchOAuthAuthServerMetadata("https://auth.example.com/mcp")
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledWith(
|
||||
"https://auth.example.com/.well-known/oauth-authorization-server",
|
||||
expect.any(Object),
|
||||
)
|
||||
})
|
||||
|
||||
it("strips trailing slash from issuer path before inserting well-known", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({
|
||||
resource: "https://example.com/",
|
||||
authorization_servers: ["https://example.com/issuer/"],
|
||||
})
|
||||
|
||||
mockFetch.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({}) })
|
||||
|
||||
await fetchOAuthAuthServerMetadata("https://example.com/mcp")
|
||||
|
||||
expect(mockFetch).toHaveBeenCalledWith(
|
||||
"https://example.com/.well-known/oauth-authorization-server/issuer",
|
||||
expect.any(Object),
|
||||
)
|
||||
})
|
||||
|
||||
it("returns null when the discovery endpoint returns a non-OK response", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({
|
||||
resource: "https://example.com/",
|
||||
authorization_servers: ["https://auth.example.com"],
|
||||
})
|
||||
|
||||
mockFetch.mockResolvedValueOnce({ ok: false, status: 404 })
|
||||
|
||||
const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("returns null when fetch throws", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({
|
||||
resource: "https://example.com/",
|
||||
authorization_servers: ["https://auth.example.com"],
|
||||
})
|
||||
|
||||
mockFetch.mockRejectedValueOnce(new Error("connection refused"))
|
||||
|
||||
const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp")
|
||||
expect(result).toBeNull()
|
||||
})
|
||||
|
||||
it("returns the parsed metadata and resource indicator on success", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({
|
||||
resource: "https://example.com/",
|
||||
authorization_servers: ["https://auth.example.com/oauth2"],
|
||||
})
|
||||
|
||||
const meta = {
|
||||
issuer: "https://auth.example.com/oauth2",
|
||||
authorization_endpoint: "https://auth.example.com/oauth2/authorize",
|
||||
token_endpoint: "https://auth.example.com/oauth2/token",
|
||||
registration_endpoint: "https://auth.example.com/oauth2/register",
|
||||
token_endpoint_auth_methods_supported: ["client_secret_post", "client_secret_basic"],
|
||||
grant_types_supported: ["authorization_code", "refresh_token"],
|
||||
scopes_supported: ["openid"],
|
||||
}
|
||||
mockFetch.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(meta) })
|
||||
|
||||
const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp")
|
||||
expect(result).toEqual({ authServerMeta: meta, resourceIndicator: "https://example.com/" })
|
||||
})
|
||||
|
||||
it("returns null resourceIndicator when protected resource metadata has no resource field", async () => {
|
||||
;(discoverOAuthProtectedResourceMetadata as any).mockResolvedValue({
|
||||
authorization_servers: ["https://auth.example.com"],
|
||||
// no `resource` field
|
||||
})
|
||||
|
||||
const meta = { issuer: "https://auth.example.com" }
|
||||
mockFetch.mockResolvedValueOnce({ ok: true, json: () => Promise.resolve(meta) })
|
||||
|
||||
const result = await fetchOAuthAuthServerMetadata("https://example.com/mcp")
|
||||
expect(result).toEqual({ authServerMeta: meta, resourceIndicator: null })
|
||||
})
|
||||
})
|
||||
78
src/services/mcp/utils/oauth.ts
Normal file
78
src/services/mcp/utils/oauth.ts
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
import { discoverOAuthProtectedResourceMetadata } from "@modelcontextprotocol/sdk/client/auth.js"
|
||||
|
||||
/**
|
||||
* Result of a successful OAuth discovery for an MCP server.
|
||||
*/
|
||||
export interface OAuthDiscoveryResult {
|
||||
/** The raw OAuth Authorization Server metadata (RFC 8414). */
|
||||
authServerMeta: Record<string, any>
|
||||
/**
|
||||
* The RFC 8707 resource indicator — the `resource` field from the Protected
|
||||
* Resource Metadata (RFC 9728). `null` when the server didn't advertise one.
|
||||
*
|
||||
* Must be sent as the `resource` query parameter in authorization requests so
|
||||
* the auth server can scope the issued tokens to this specific resource server.
|
||||
*/
|
||||
resourceIndicator: string | null
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetches the raw OAuth Authorization Server metadata for an MCP server URL.
|
||||
*
|
||||
* This replaces the SDK's built-in `discoverOAuthMetadata()` because it
|
||||
* constructs the RFC 8414 well-known URL incorrectly for auth servers with
|
||||
* path components — a known bug tracked in multiple upstream issues:
|
||||
*
|
||||
* - https://github.com/modelcontextprotocol/typescript-sdk/issues/545
|
||||
* (URL constructor discards base path with leading-slash well-known)
|
||||
* - https://github.com/modelcontextprotocol/typescript-sdk/issues/762
|
||||
* (uses MCP server URL instead of authorization server URL)
|
||||
* - https://github.com/modelcontextprotocol/typescript-sdk/issues/744
|
||||
* (doesn't respect provided authorization server URL)
|
||||
* - https://github.com/modelcontextprotocol/typescript-sdk/issues/822
|
||||
* (general RFC 8414 compliance — affects Keycloak, Okta, Azure Entra)
|
||||
*
|
||||
* Performs two discovery steps:
|
||||
* 1. RFC 9728 – fetches the Protected Resource Metadata to find the issuer URL
|
||||
* and the RFC 8707 resource indicator.
|
||||
* 2. RFC 8414 §3.1 – constructs the well-known discovery URL by inserting
|
||||
* `/.well-known/oauth-authorization-server` *between* the host and the issuer
|
||||
* path (not appended after the path).
|
||||
*
|
||||
* Correct: https://example.com/.well-known/oauth-authorization-server/auth/public
|
||||
* SDK wrong: https://example.com/auth/public/.well-known/oauth-authorization-server
|
||||
*
|
||||
* Returns an {@link OAuthDiscoveryResult} on success, or `null` if any step fails.
|
||||
*/
|
||||
export async function fetchOAuthAuthServerMetadata(serverUrl: string): Promise<OAuthDiscoveryResult | null> {
|
||||
try {
|
||||
// Step 1 – RFC 9728: resolve the authorization server issuer URL and
|
||||
// capture the resource indicator for RFC 8707.
|
||||
const resourceMeta = await discoverOAuthProtectedResourceMetadata(serverUrl)
|
||||
const authServers = resourceMeta.authorization_servers
|
||||
if (!authServers?.length) return null
|
||||
|
||||
// RFC 8707: the `resource` field from the protected resource metadata is
|
||||
// used as the `resource` parameter in the authorization request so the auth
|
||||
// server can issue tokens scoped to this specific resource server.
|
||||
const resourceIndicator: string | null =
|
||||
typeof resourceMeta.resource === "string" ? resourceMeta.resource : null
|
||||
|
||||
// Step 2 – RFC 8414 §3.1: build the well-known URL.
|
||||
// For issuer "https://example.com/auth/public"
|
||||
// → "https://example.com/.well-known/oauth-authorization-server/auth/public"
|
||||
const parsed = new URL(authServers[0])
|
||||
const base = `${parsed.protocol}//${parsed.host}`
|
||||
const issuePath = parsed.pathname.replace(/\/$/, "") || ""
|
||||
const discoveryUrl = `${base}/.well-known/oauth-authorization-server${issuePath}`
|
||||
|
||||
const response = await fetch(discoveryUrl, {
|
||||
headers: { Accept: "application/json" },
|
||||
})
|
||||
if (!response.ok) return null
|
||||
const authServerMeta = (await response.json()) as Record<string, any>
|
||||
return { authServerMeta, resourceIndicator }
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue