Просмотр исходного кода

fix(mcp): include scope in clientMetadata and add callbackPort option (#28810)

Co-authored-by: Shoubhit Dash <shoubhit2005@gmail.com>
Sebin 3 месяцев назад
Родитель
Сommit
7a769dab3a

+ 4 - 0
packages/opencode/src/config/mcp.ts

@@ -26,6 +26,10 @@ export const OAuth = Schema.Struct({
     description: "OAuth client secret (if required by the authorization server)",
   }),
   scope: Schema.optional(Schema.String).annotate({ description: "OAuth scopes to request during authorization" }),
+  callbackPort: Schema.optional(Schema.Int.check(Schema.isBetween({ minimum: 1, maximum: 65535 }))).annotate({
+    description:
+      "Port for the local OAuth callback server (default: 19876). Shorthand for redirectUri when only the port needs changing. Ignored if redirectUri is set.",
+  }),
   redirectUri: Schema.optional(Schema.String).annotate({
     description: "OAuth redirect URI (default: http://127.0.0.1:19876/mcp/oauth/callback).",
   }),

+ 11 - 3
packages/opencode/src/mcp/index.ts

@@ -19,7 +19,7 @@ import { NamedError } from "@opencode-ai/core/util/error"
 import { InstallationVersion } from "@opencode-ai/core/installation/version"
 import { withTimeout } from "@/util/timeout"
 import { AppFileSystem } from "@opencode-ai/core/filesystem"
-import { McpOAuthProvider } from "./oauth-provider"
+import { McpOAuthProvider, OAUTH_CALLBACK_PATH } from "./oauth-provider"
 import { McpOAuthCallback } from "./oauth-callback"
 import { McpAuth } from "./auth"
 import { BusEvent } from "../bus/bus-event"
@@ -318,6 +318,7 @@ export const layer = Layer.effect(
             clientId: oauthConfig?.clientId,
             clientSecret: oauthConfig?.clientSecret,
             scope: oauthConfig?.scope,
+            callbackPort: oauthConfig?.callbackPort,
             redirectUri: oauthConfig?.redirectUri,
           },
           {
@@ -769,8 +770,15 @@ export const layer = Layer.effect(
       // OAuth config is optional - if not provided, we'll use auto-discovery
       const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined
 
+      // Resolve effective redirect URI: explicit redirectUri > callbackPort shorthand > default
+      const effectiveRedirectUri =
+        oauthConfig?.redirectUri ??
+        (oauthConfig?.callbackPort
+          ? `http://127.0.0.1:${oauthConfig.callbackPort}${OAUTH_CALLBACK_PATH}`
+          : undefined)
+
       // Start the callback server with custom redirectUri if configured
-      yield* Effect.promise(() => McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri))
+      yield* Effect.promise(() => McpOAuthCallback.ensureRunning(effectiveRedirectUri))
 
       const oauthState = Array.from(crypto.getRandomValues(new Uint8Array(32)))
         .map((b) => b.toString(16).padStart(2, "0"))
@@ -784,7 +792,7 @@ export const layer = Layer.effect(
           clientId: oauthConfig?.clientId,
           clientSecret: oauthConfig?.clientSecret,
           scope: oauthConfig?.scope,
-          redirectUri: oauthConfig?.redirectUri,
+          redirectUri: effectiveRedirectUri,
         },
         {
           onRedirect: async (url) => {

+ 4 - 1
packages/opencode/src/mcp/oauth-provider.ts

@@ -18,6 +18,7 @@ export interface McpOAuthConfig {
   clientId?: string
   clientSecret?: string
   scope?: string
+  callbackPort?: number
   redirectUri?: string
 }
 
@@ -38,7 +39,8 @@ export class McpOAuthProvider implements OAuthClientProvider {
     if (this.config.redirectUri) {
       return this.config.redirectUri
     }
-    return `http://127.0.0.1:${OAUTH_CALLBACK_PORT}${OAUTH_CALLBACK_PATH}`
+    const port = this.config.callbackPort ?? OAUTH_CALLBACK_PORT
+    return `http://127.0.0.1:${port}${OAUTH_CALLBACK_PATH}`
   }
 
   get clientMetadata(): OAuthClientMetadata {
@@ -49,6 +51,7 @@ export class McpOAuthProvider implements OAuthClientProvider {
       grant_types: ["authorization_code", "refresh_token"],
       response_types: ["code"],
       token_endpoint_auth_method: this.config.clientSecret ? "client_secret_post" : "none",
+      ...(this.config.scope ? { scope: this.config.scope } : {}),
     }
   }
 

+ 61 - 0
packages/opencode/test/mcp/oauth-provider.test.ts

@@ -0,0 +1,61 @@
+import { test, expect, describe } from "bun:test"
+import { McpOAuthProvider, OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH } from "../../src/mcp/oauth-provider"
+import type { McpAuth } from "../../src/mcp/auth"
+
+// Stub auth — only synchronous getters are exercised in these tests
+const stubAuth = {} as McpAuth.Interface
+
+const makeProvider = (config: ConstructorParameters<typeof McpOAuthProvider>[2]) =>
+  new McpOAuthProvider("test-server", "https://mcp.example.com/mcp", config, { onRedirect: async () => {} }, stubAuth)
+
+describe("McpOAuthProvider.redirectUrl", () => {
+  test("defaults to 127.0.0.1:19876/mcp/oauth/callback", () => {
+    const provider = makeProvider({})
+    expect(provider.redirectUrl).toBe(`http://127.0.0.1:${OAUTH_CALLBACK_PORT}${OAUTH_CALLBACK_PATH}`)
+  })
+
+  test("uses callbackPort when set", () => {
+    const provider = makeProvider({ callbackPort: 6620 })
+    expect(provider.redirectUrl).toBe(`http://127.0.0.1:6620${OAUTH_CALLBACK_PATH}`)
+  })
+
+  test("redirectUri takes precedence over callbackPort", () => {
+    const provider = makeProvider({
+      callbackPort: 6620,
+      redirectUri: "http://127.0.0.1:9999/custom/callback",
+    })
+    expect(provider.redirectUrl).toBe("http://127.0.0.1:9999/custom/callback")
+  })
+
+  test("uses explicit redirectUri when set without callbackPort", () => {
+    const provider = makeProvider({ redirectUri: "http://127.0.0.1:8080/oauth/callback" })
+    expect(provider.redirectUrl).toBe("http://127.0.0.1:8080/oauth/callback")
+  })
+})
+
+describe("McpOAuthProvider.clientMetadata", () => {
+  test("includes redirect_uris from redirectUrl", () => {
+    const provider = makeProvider({ callbackPort: 6620 })
+    expect(provider.clientMetadata.redirect_uris).toEqual([`http://127.0.0.1:6620${OAUTH_CALLBACK_PATH}`])
+  })
+
+  test("includes scope when set in config", () => {
+    const provider = makeProvider({ scope: "openid offline_access" })
+    expect(provider.clientMetadata.scope).toBe("openid offline_access")
+  })
+
+  test("omits scope when not set in config", () => {
+    const provider = makeProvider({})
+    expect(provider.clientMetadata.scope).toBeUndefined()
+  })
+
+  test("sets token_endpoint_auth_method to client_secret_post when clientSecret provided", () => {
+    const provider = makeProvider({ clientSecret: "secret" })
+    expect(provider.clientMetadata.token_endpoint_auth_method).toBe("client_secret_post")
+  })
+
+  test("sets token_endpoint_auth_method to none when no clientSecret", () => {
+    const provider = makeProvider({})
+    expect(provider.clientMetadata.token_endpoint_auth_method).toBe("none")
+  })
+})