Browse Source

feat(mcp): add OAuth redirect URI configuration for MCP servers (#7379)

Christopher Tso 1 month ago
parent
commit
40b275d7e6

+ 6 - 0
packages/opencode/src/cli/cmd/mcp.ts

@@ -6,6 +6,7 @@ import * as prompts from "@clack/prompts"
 import { UI } from "../ui"
 import { MCP } from "../../mcp"
 import { McpAuth } from "../../mcp/auth"
+import { McpOAuthCallback } from "../../mcp/oauth-callback"
 import { McpOAuthProvider } from "../../mcp/oauth-provider"
 import { Config } from "../../config/config"
 import { Instance } from "../../project/instance"
@@ -682,6 +683,10 @@ export const McpDebugCommand = cmd({
 
             // Try to discover OAuth metadata
             const oauthConfig = typeof serverConfig.oauth === "object" ? serverConfig.oauth : undefined
+
+            // Start callback server
+            await McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri)
+
             const authProvider = new McpOAuthProvider(
               serverName,
               serverConfig.url,
@@ -689,6 +694,7 @@ export const McpDebugCommand = cmd({
                 clientId: oauthConfig?.clientId,
                 clientSecret: oauthConfig?.clientSecret,
                 scope: oauthConfig?.scope,
+                redirectUri: oauthConfig?.redirectUri,
               },
               {
                 onRedirect: async () => {},

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

@@ -433,6 +433,10 @@ export namespace Config {
         .describe("OAuth client ID. If not provided, dynamic client registration (RFC 7591) will be attempted."),
       clientSecret: z.string().optional().describe("OAuth client secret (if required by the authorization server)"),
       scope: z.string().optional().describe("OAuth scopes to request during authorization"),
+      redirectUri: z
+        .string()
+        .optional()
+        .describe("OAuth redirect URI (default: http://127.0.0.1:19876/mcp/oauth/callback)."),
     })
     .strict()
     .meta({

+ 17 - 10
packages/opencode/src/mcp/index.ts

@@ -308,6 +308,8 @@ export namespace MCP {
       let authProvider: McpOAuthProvider | undefined
 
       if (!oauthDisabled) {
+        await McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri)
+
         authProvider = new McpOAuthProvider(
           key,
           mcp.url,
@@ -315,6 +317,7 @@ export namespace MCP {
             clientId: oauthConfig?.clientId,
             clientSecret: oauthConfig?.clientSecret,
             scope: oauthConfig?.scope,
+            redirectUri: oauthConfig?.redirectUri,
           },
           {
             onRedirect: async (url) => {
@@ -344,6 +347,7 @@ export namespace MCP {
 
       let lastError: Error | undefined
       const connectTimeout = mcp.timeout ?? DEFAULT_TIMEOUT
+
       for (const { name, transport } of transports) {
         try {
           const client = new Client({
@@ -570,7 +574,8 @@ export namespace MCP {
 
     for (const [clientName, client] of Object.entries(clientsSnapshot)) {
       // Only include tools from connected MCPs (skip disabled ones)
-      if (s.status[clientName]?.status !== "connected") {
+      const clientStatus = s.status[clientName]?.status
+      if (clientStatus !== "connected") {
         continue
       }
 
@@ -720,8 +725,10 @@ export namespace MCP {
       throw new Error(`MCP server ${mcpName} has OAuth explicitly disabled`)
     }
 
-    // Start the callback server
-    await McpOAuthCallback.ensureRunning()
+    // OAuth config is optional - if not provided, we'll use auto-discovery
+    const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined
+
+    await McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri)
 
     // Generate and store a cryptographically secure state parameter BEFORE creating the provider
     // The SDK will call provider.state() to read this value
@@ -731,8 +738,6 @@ export namespace MCP {
     await McpAuth.updateOAuthState(mcpName, oauthState)
 
     // Create a new auth provider for this flow
-    // OAuth config is optional - if not provided, we'll use auto-discovery
-    const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined
     let capturedUrl: URL | undefined
     const authProvider = new McpOAuthProvider(
       mcpName,
@@ -741,6 +746,7 @@ export namespace MCP {
         clientId: oauthConfig?.clientId,
         clientSecret: oauthConfig?.clientSecret,
         scope: oauthConfig?.scope,
+        redirectUri: oauthConfig?.redirectUri,
       },
       {
         onRedirect: async (url) => {
@@ -769,6 +775,7 @@ export namespace MCP {
         pendingOAuthTransports.set(mcpName, transport)
         return { authorizationUrl: capturedUrl.toString() }
       }
+
       throw error
     }
   }
@@ -778,9 +785,9 @@ export namespace MCP {
    * Opens the browser and waits for callback.
    */
   export async function authenticate(mcpName: string): Promise<Status> {
-    const { authorizationUrl } = await startAuth(mcpName)
+    const result = await startAuth(mcpName)
 
-    if (!authorizationUrl) {
+    if (!result.authorizationUrl) {
       // Already authenticated
       const s = await state()
       return s.status[mcpName] ?? { status: "connected" }
@@ -794,9 +801,9 @@ export namespace MCP {
 
     // The SDK has already added the state parameter to the authorization URL
     // We just need to open the browser
-    log.info("opening browser for oauth", { mcpName, url: authorizationUrl, state: oauthState })
+    log.info("opening browser for oauth", { mcpName, url: result.authorizationUrl, state: oauthState })
     try {
-      const subprocess = await open(authorizationUrl)
+      const subprocess = await open(result.authorizationUrl)
       // The open package spawns a detached process and returns immediately.
       // We need to listen for errors which fire asynchronously:
       // - "error" event: command not found (ENOENT)
@@ -819,7 +826,7 @@ export namespace MCP {
       // Browser opening failed (e.g., in remote/headless sessions like SSH, devcontainers)
       // Emit event so CLI can display the URL for manual opening
       log.warn("failed to open browser, user must open URL manually", { mcpName, error })
-      Bus.publish(BrowserOpenFailed, { mcpName, url: authorizationUrl })
+      Bus.publish(BrowserOpenFailed, { mcpName, url: result.authorizationUrl })
     }
 
     // Wait for callback using the OAuth state parameter

+ 25 - 9
packages/opencode/src/mcp/oauth-callback.ts

@@ -1,8 +1,12 @@
 import { Log } from "../util/log"
-import { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH } from "./oauth-provider"
+import { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH, parseRedirectUri } from "./oauth-provider"
 
 const log = Log.create({ service: "mcp.oauth-callback" })
 
+// Current callback server configuration (may differ from defaults if custom redirectUri is used)
+let currentPort = OAUTH_CALLBACK_PORT
+let currentPath = OAUTH_CALLBACK_PATH
+
 const HTML_SUCCESS = `<!DOCTYPE html>
 <html>
 <head>
@@ -56,21 +60,33 @@ export namespace McpOAuthCallback {
 
   const CALLBACK_TIMEOUT_MS = 5 * 60 * 1000 // 5 minutes
 
-  export async function ensureRunning(): Promise<void> {
+  export async function ensureRunning(redirectUri?: string): Promise<void> {
+    // Parse the redirect URI to get port and path (uses defaults if not provided)
+    const { port, path } = parseRedirectUri(redirectUri)
+
+    // If server is running on a different port/path, stop it first
+    if (server && (currentPort !== port || currentPath !== path)) {
+      log.info("stopping oauth callback server to reconfigure", { oldPort: currentPort, newPort: port })
+      await stop()
+    }
+
     if (server) return
 
-    const running = await isPortInUse()
+    const running = await isPortInUse(port)
     if (running) {
-      log.info("oauth callback server already running on another instance", { port: OAUTH_CALLBACK_PORT })
+      log.info("oauth callback server already running on another instance", { port })
       return
     }
 
+    currentPort = port
+    currentPath = path
+
     server = Bun.serve({
-      port: OAUTH_CALLBACK_PORT,
+      port: currentPort,
       fetch(req) {
         const url = new URL(req.url)
 
-        if (url.pathname !== OAUTH_CALLBACK_PATH) {
+        if (url.pathname !== currentPath) {
           return new Response("Not found", { status: 404 })
         }
 
@@ -133,7 +149,7 @@ export namespace McpOAuthCallback {
       },
     })
 
-    log.info("oauth callback server started", { port: OAUTH_CALLBACK_PORT })
+    log.info("oauth callback server started", { port: currentPort, path: currentPath })
   }
 
   export function waitForCallback(oauthState: string): Promise<string> {
@@ -158,11 +174,11 @@ export namespace McpOAuthCallback {
     }
   }
 
-  export async function isPortInUse(): Promise<boolean> {
+  export async function isPortInUse(port: number = OAUTH_CALLBACK_PORT): Promise<boolean> {
     return new Promise((resolve) => {
       Bun.connect({
         hostname: "127.0.0.1",
-        port: OAUTH_CALLBACK_PORT,
+        port,
         socket: {
           open(socket) {
             socket.end()

+ 24 - 0
packages/opencode/src/mcp/oauth-provider.ts

@@ -17,6 +17,7 @@ export interface McpOAuthConfig {
   clientId?: string
   clientSecret?: string
   scope?: string
+  redirectUri?: string
 }
 
 export interface McpOAuthCallbacks {
@@ -32,6 +33,10 @@ export class McpOAuthProvider implements OAuthClientProvider {
   ) {}
 
   get redirectUrl(): string {
+    // Use configured redirectUri if provided, otherwise use OpenCode defaults
+    if (this.config.redirectUri) {
+      return this.config.redirectUri
+    }
     return `http://127.0.0.1:${OAUTH_CALLBACK_PORT}${OAUTH_CALLBACK_PATH}`
   }
 
@@ -152,3 +157,22 @@ export class McpOAuthProvider implements OAuthClientProvider {
 }
 
 export { OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH }
+
+/**
+ * Parse a redirect URI to extract port and path for the callback server.
+ * Returns defaults if the URI can't be parsed.
+ */
+export function parseRedirectUri(redirectUri?: string): { port: number; path: string } {
+  if (!redirectUri) {
+    return { port: OAUTH_CALLBACK_PORT, path: OAUTH_CALLBACK_PATH }
+  }
+
+  try {
+    const url = new URL(redirectUri)
+    const port = url.port ? parseInt(url.port, 10) : (url.protocol === "https:" ? 443 : 80)
+    const path = url.pathname || OAUTH_CALLBACK_PATH
+    return { port, path }
+  } catch {
+    return { port: OAUTH_CALLBACK_PORT, path: OAUTH_CALLBACK_PATH }
+  }
+}

+ 75 - 0
packages/opencode/test/mcp/oauth-callback.test.ts

@@ -0,0 +1,75 @@
+import { test, expect, describe, afterEach } from "bun:test"
+import { McpOAuthCallback } from "../../src/mcp/oauth-callback"
+import { parseRedirectUri } from "../../src/mcp/oauth-provider"
+
+describe("McpOAuthCallback.ensureRunning", () => {
+  afterEach(async () => {
+    await McpOAuthCallback.stop()
+  })
+
+  test("starts server with default config when no redirectUri provided", async () => {
+    await McpOAuthCallback.ensureRunning()
+    expect(McpOAuthCallback.isRunning()).toBe(true)
+  })
+
+  test("starts server with custom redirectUri", async () => {
+    await McpOAuthCallback.ensureRunning("http://127.0.0.1:18000/custom/callback")
+    expect(McpOAuthCallback.isRunning()).toBe(true)
+  })
+
+  test("is idempotent when called with same redirectUri", async () => {
+    await McpOAuthCallback.ensureRunning("http://127.0.0.1:18001/callback")
+    await McpOAuthCallback.ensureRunning("http://127.0.0.1:18001/callback")
+    expect(McpOAuthCallback.isRunning()).toBe(true)
+  })
+
+  test("restarts server when redirectUri changes", async () => {
+    await McpOAuthCallback.ensureRunning("http://127.0.0.1:18002/path1")
+    expect(McpOAuthCallback.isRunning()).toBe(true)
+
+    await McpOAuthCallback.ensureRunning("http://127.0.0.1:18003/path2")
+    expect(McpOAuthCallback.isRunning()).toBe(true)
+  })
+
+  test("isRunning returns false when not started", async () => {
+    expect(McpOAuthCallback.isRunning()).toBe(false)
+  })
+
+  test("isRunning returns false after stop", async () => {
+    await McpOAuthCallback.ensureRunning()
+    await McpOAuthCallback.stop()
+    expect(McpOAuthCallback.isRunning()).toBe(false)
+  })
+})
+
+describe("parseRedirectUri", () => {
+  test("returns defaults when no URI provided", () => {
+    const result = parseRedirectUri()
+    expect(result.port).toBe(19876)
+    expect(result.path).toBe("/mcp/oauth/callback")
+  })
+
+  test("parses port and path from URI", () => {
+    const result = parseRedirectUri("http://127.0.0.1:8080/oauth/callback")
+    expect(result.port).toBe(8080)
+    expect(result.path).toBe("/oauth/callback")
+  })
+
+  test("defaults to port 80 for http without explicit port", () => {
+    const result = parseRedirectUri("http://127.0.0.1/callback")
+    expect(result.port).toBe(80)
+    expect(result.path).toBe("/callback")
+  })
+
+  test("defaults to port 443 for https without explicit port", () => {
+    const result = parseRedirectUri("https://127.0.0.1/callback")
+    expect(result.port).toBe(443)
+    expect(result.path).toBe("/callback")
+  })
+
+  test("returns defaults for invalid URI", () => {
+    const result = parseRedirectUri("not-a-valid-url")
+    expect(result.port).toBe(19876)
+    expect(result.path).toBe("/mcp/oauth/callback")
+  })
+})