From f6d2d021696c93e2d4461b2eb8b96d748039c08d Mon Sep 17 00:00:00 2001 From: Sanjay Doppalapudi <80777235+Sanjay-doppalapudi@users.noreply.github.com> Date: Thu, 7 May 2026 14:10:28 -0500 Subject: [PATCH] fix: force OAuth flow when server accepts unauthenticated connections Some MCP servers (like Google Drive) accept the initial connection without authentication but enforce it at the tool-call level. OpenCode would skip the OAuth flow entirely and report 'Authentication successful!' without obtaining any tokens, leaving the server unusable. When startAuth() detects that the server accepted the connection without requiring OAuth but the user has configured OAuth credentials and no tokens exist, we now force the OAuth discovery/redirect flow via the SDK's auth() function. As a safety net, authenticate() also checks whether tokens were actually obtained before reporting success when OAuth was configured. --- packages/opencode/src/mcp/index.ts | 75 ++++++++- .../test/mcp/oauth-auto-connect.test.ts | 153 +++++++++++++++++- .../opencode/test/mcp/oauth-browser.test.ts | 1 + 3 files changed, 217 insertions(+), 12 deletions(-) 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(() => {