diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index fe7180238851..8fc604e1e51a 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -3,7 +3,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js" import { StreamableHTTPClientTransport } from "@modelcontextprotocol/sdk/client/streamableHttp.js" import { SSEClientTransport } from "@modelcontextprotocol/sdk/client/sse.js" import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js" -import { UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js" +import { UnauthorizedError, auth as sdkAuth } from "@modelcontextprotocol/sdk/client/auth.js" import { CallToolResultSchema, type Tool as MCPToolDef, @@ -25,7 +25,7 @@ import { BusEvent } from "../bus/bus-event" import { Bus } from "@/bus" import { TuiEvent } from "@/cli/cmd/tui/event" import open from "open" -import { Effect, Exit, Layer, Option, Context, Schema, Stream } from "effect" +import { Effect, Exit, Layer, Option, Context, Schema, Stream, Cause } from "effect" import { EffectBridge } from "@/effect/bridge" import { InstanceState } from "@/effect/instance-state" import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process" @@ -767,29 +767,81 @@ export const layer = Layer.effect( const transport = new StreamableHTTPClientTransport(url, { authProvider }) - return yield* Effect.tryPromise({ + const result: AuthResult = yield* Effect.tryPromise({ try: () => { const client = new Client({ name: "opencode", version: InstallationVersion }) return client .connect(transport) - .then(() => ({ authorizationUrl: "", oauthState, client }) satisfies AuthResult) + .then((): AuthResult => ({ authorizationUrl: "", oauthState, client })) }, catch: (error) => error, }).pipe( Effect.catch((error) => { if (error instanceof UnauthorizedError && capturedUrl) { pendingOAuthTransports.set(mcpName, transport) - return Effect.succeed({ authorizationUrl: capturedUrl.toString(), oauthState } satisfies AuthResult) + return Effect.succeed({ authorizationUrl: capturedUrl.toString(), oauthState } as AuthResult) } return Effect.die(error) }), ) + + if ( + !result.authorizationUrl && + oauthConfig && + !(yield* auth.get(mcpName))?.tokens + ) { + log.info("server accepted connection without auth but oauth was configured, forcing oauth flow", { mcpName }) + yield* Effect.tryPromise(() => result.client?.close() ?? Promise.resolve()).pipe(Effect.ignore) + + const forceCapturedUrl: URL[] = [] + const forceAuthProvider = new McpOAuthProvider( + mcpName, + mcpConfig.url, + { + clientId: oauthConfig.clientId, + clientSecret: oauthConfig.clientSecret, + scope: oauthConfig.scope, + redirectUri: oauthConfig.redirectUri, + }, + { + onRedirect: async (url) => { + forceCapturedUrl.push(url) + }, + }, + auth, + ) + + const forceTransport = new StreamableHTTPClientTransport(url, { authProvider: forceAuthProvider }) + + const forceAuthResult = yield* Effect.tryPromise(() => sdkAuth(forceAuthProvider, { serverUrl: url })).pipe( + Effect.catchCause((cause) => { + const error = Cause.squash(cause) + if (error instanceof UnauthorizedError && forceCapturedUrl.length > 0) { + pendingOAuthTransports.set(mcpName, forceTransport) + return Effect.succeed("REDIRECT" as const) + } + log.error("forced oauth flow failed", { mcpName, error }) + return Effect.succeed("AUTHORIZED" as const) + }), + ) + + if (forceAuthResult === "REDIRECT" && forceCapturedUrl.length > 0) { + return { + authorizationUrl: forceCapturedUrl[0].toString(), + oauthState, + } as AuthResult + } + + return { authorizationUrl: "", oauthState } as AuthResult + } + + return result }) const authenticate = Effect.fn("MCP.authenticate")(function* (mcpName: string) { const result = yield* startAuth(mcpName) if (!result.authorizationUrl) { - const client = "client" in result ? result.client : undefined + const client = result.client const mcpConfig = yield* getMcpConfig(mcpName) if (!mcpConfig) { yield* Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore) @@ -802,6 +854,17 @@ export const layer = Layer.effect( return { status: "failed", error: "Failed to get tools" } as Status } + const hasOAuthConfig = mcpConfig.type === "remote" && typeof mcpConfig.oauth === "object" + const savedEntry = yield* auth.get(mcpName) + if (hasOAuthConfig && !savedEntry?.tokens) { + yield* Effect.tryPromise(() => client.close()).pipe(Effect.ignore) + return { + status: "failed", + error: + "Server accepted the connection without OAuth, but no tokens were obtained. The server may require authentication for operations.", + } as Status + } + const s = yield* InstanceState.get(state) yield* auth.clearOAuthState(mcpName) return yield* storeClient(s, mcpName, client, listed, mcpConfig.timeout) diff --git a/packages/opencode/test/mcp/oauth-auto-connect.test.ts b/packages/opencode/test/mcp/oauth-auto-connect.test.ts index 3cf67742156d..91cacd136044 100644 --- a/packages/opencode/test/mcp/oauth-auto-connect.test.ts +++ b/packages/opencode/test/mcp/oauth-auto-connect.test.ts @@ -101,19 +101,41 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({ // Mock UnauthorizedError in the auth module so instanceof checks work void mock.module("@modelcontextprotocol/sdk/client/auth.js", () => ({ UnauthorizedError: MockUnauthorizedError, + auth: async ( + provider: { + state?: () => Promise + redirectToAuthorization?: (url: URL) => Promise + saveCodeVerifier?: (v: string) => Promise + tokens?: () => Promise + clientInformation?: () => Promise + clientMetadata?: unknown + redirectUrl?: string + }, + _options?: { serverUrl?: URL }, + ) => { + if (simulateAuthFlow && provider.redirectToAuthorization) { + if (provider.state) await provider.state() + if (provider.saveCodeVerifier) await provider.saveCodeVerifier("test-verifier") + await provider.redirectToAuthorization(new URL("https://auth.example.com/authorize?state=test")) + return "REDIRECT" + } + throw new MockUnauthorizedError() + }, })) -beforeEach(() => { - transportCalls.length = 0 - simulateAuthFlow = true - connectSucceedsImmediately = false -}) - // Import modules after mocking const { MCP } = await import("../../src/mcp/index") const { Instance } = await import("../../src/project/instance") const { WithInstance } = await import("../../src/project/with-instance") const { tmpdir } = await import("../fixture/fixture") +const { McpOAuthCallback } = await import("../../src/mcp/oauth-callback") + +beforeEach(async () => { + transportCalls.length = 0 + simulateAuthFlow = true + connectSucceedsImmediately = false + await McpOAuthCallback.stop() +}) test("first connect to OAuth server shows needs_auth instead of failed", async () => { await using tmp = await tmpdir({ @@ -280,3 +302,122 @@ test("authenticate() stores a connected client when auth completes without redir }, }) }) + +test("startAuth() forces OAuth redirect when server accepts connection without auth and OAuth is configured", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write( + `${dir}/opencode.json`, + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + mcp: { + gdrive: { + type: "remote", + url: "https://drivemcp.googleapis.com/mcp/v1", + oauth: { + clientId: "test-client-id", + clientSecret: "test-secret", + scope: "https://www.googleapis.com/auth/drive.readonly", + }, + }, + }, + }), + ) + }, + }) + + await WithInstance.provide({ + directory: tmp.path, + fn: async () => { + await Effect.runPromise( + MCP.Service.use((mcp) => + Effect.gen(function* () { + yield* mcp.add("gdrive", { + type: "remote", + url: "https://drivemcp.googleapis.com/mcp/v1", + oauth: { + clientId: "test-client-id", + clientSecret: "test-secret", + scope: "https://www.googleapis.com/auth/drive.readonly", + }, + }) + + simulateAuthFlow = true + connectSucceedsImmediately = true + + const result = yield* mcp.startAuth("gdrive") + expect(result.authorizationUrl).toBeTruthy() + expect(result.authorizationUrl).toContain("https://auth.example.com/authorize") + }), + ).pipe(Effect.provide(MCP.defaultLayer)), + ) + }, + }) +}) + +test("authenticate() succeeds when server accepts connection without OAuth and tokens already exist", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write( + `${dir}/opencode.json`, + JSON.stringify({ + $schema: "https://opencode.ai/config.json", + mcp: { + "with-tokens": { + type: "remote", + url: "https://example.com/mcp", + oauth: { + clientId: "test-client-id", + scope: "read", + }, + }, + }, + }), + ) + }, + }) + + await WithInstance.provide({ + directory: tmp.path, + fn: async () => { + const { McpAuth } = await import("../../src/mcp/auth") + await Effect.runPromise( + McpAuth.Service.use((auth) => + auth.updateTokens( + "with-tokens", + { + accessToken: "existing-access-token", + refreshToken: "existing-refresh-token", + expiresAt: Date.now() / 1000 + 3600, + scope: "read", + }, + "https://example.com/mcp", + ), + ).pipe(Effect.provide(McpAuth.defaultLayer)), + ) + + await Effect.runPromise( + MCP.Service.use((mcp) => + Effect.gen(function* () { + const added = yield* mcp.add("with-tokens", { + type: "remote", + url: "https://example.com/mcp", + oauth: { + clientId: "test-client-id", + scope: "read", + }, + }) + const before = added.status as Record + expect(before["with-tokens"]?.status).toBe("needs_auth") + + simulateAuthFlow = false + connectSucceedsImmediately = true + + const result = yield* mcp.authenticate("with-tokens") + expect(result.status).toBe("connected") + }), + ).pipe(Effect.provide(MCP.defaultLayer)), + ) + }, + }) +}) diff --git a/packages/opencode/test/mcp/oauth-browser.test.ts b/packages/opencode/test/mcp/oauth-browser.test.ts index 20cb90a18e0a..b97de6eb4001 100644 --- a/packages/opencode/test/mcp/oauth-browser.test.ts +++ b/packages/opencode/test/mcp/oauth-browser.test.ts @@ -92,6 +92,7 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({ // Mock UnauthorizedError in the auth module void mock.module("@modelcontextprotocol/sdk/client/auth.js", () => ({ UnauthorizedError: MockUnauthorizedError, + auth: async () => "REDIRECT", })) beforeEach(() => {