Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/opencode/src/cli/cmd/mcp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,9 @@ export const McpAuthCommand = effectCmd({
Effect.sync(() => {
if (status.status === "connected") {
spinner.stop("Authentication successful!")
} else if (status.status === "needs_auth") {
spinner.stop("Authentication did not complete", 1)
prompts.log.error("Server connected without storing OAuth credentials")
} else if (status.status === "needs_client_registration") {
spinner.stop("Authentication failed", 1)
prompts.log.error(status.error)
Expand Down
21 changes: 18 additions & 3 deletions packages/opencode/src/mcp/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ type AuthData = Record<string, Entry>

const filepath = path.join(Global.Path.data, "mcp-auth.json")

function normalizeEntry(entry: Entry | undefined) {
if (!entry) return
Comment thread
ImDevinC marked this conversation as resolved.
if (entry.tokens || entry.clientInfo || entry.codeVerifier || entry.oauthState) return entry
}

export interface Interface {
readonly all: () => Effect.Effect<Record<string, Entry>>
readonly get: (mcpName: string) => Effect.Effect<Entry | undefined>
Expand All @@ -58,7 +63,12 @@ export const layer = Layer.effect(

const all = Effect.fn("McpAuth.all")(function* () {
return yield* fs.readJson(filepath).pipe(
Effect.map((data): AuthData => Option.getOrElse(decodeAuthData(data), () => ({}) as AuthData) as AuthData),
Effect.map((data): AuthData => {
const decoded = Option.getOrElse(decodeAuthData(data), () => ({}) as AuthData) as AuthData
return Object.fromEntries(
Object.entries(decoded).flatMap(([mcpName, entry]) => (normalizeEntry(entry) ? [[mcpName, entry]] : [])),
)
}),
Effect.catch(() => Effect.succeed({} as AuthData)),
)
})
Expand All @@ -78,8 +88,13 @@ export const layer = Layer.effect(

const set = Effect.fn("McpAuth.set")(function* (mcpName: string, entry: Entry, serverUrl?: string) {
const data = yield* all()
if (serverUrl) entry.serverUrl = serverUrl
yield* fs.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600).pipe(Effect.orDie)
const next = normalizeEntry(serverUrl ? { ...entry, serverUrl } : entry)
if (!next) {
delete data[mcpName]
yield* fs.writeJson(filepath, data, 0o600).pipe(Effect.orDie)
return
}
yield* fs.writeJson(filepath, { ...data, [mcpName]: next }, 0o600).pipe(Effect.orDie)
})

const remove = Effect.fn("McpAuth.remove")(function* (mcpName: string) {
Expand Down
59 changes: 44 additions & 15 deletions packages/opencode/src/mcp/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ 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 { auth as performOAuth, UnauthorizedError } from "@modelcontextprotocol/sdk/client/auth.js"
import { createFetchWithInit } from "@modelcontextprotocol/sdk/shared/transport.js"
import {
CallToolResultSchema,
ListToolsResultSchema,
Expand Down Expand Up @@ -756,7 +757,7 @@ export const layer = Layer.effect(
return mcpConfig
})

const startAuth = Effect.fn("MCP.startAuth")(function* (mcpName: string) {
const beginAuth = Effect.fn("MCP.beginAuth")(function* (mcpName: string) {
const mcpConfig = yield* getMcpConfig(mcpName)
if (!mcpConfig) throw new Error(`MCP server ${mcpName} not found or disabled`)
if (mcpConfig.type !== "remote") throw new Error(`MCP server ${mcpName} is not a remote server`)
Expand Down Expand Up @@ -792,7 +793,24 @@ export const layer = Layer.effect(
auth,
)

const transport = new StreamableHTTPClientTransport(url, { authProvider })
const requestInit = mcpConfig.headers ? { headers: mcpConfig.headers } : undefined
const transport = new StreamableHTTPClientTransport(url, { authProvider, requestInit })

const authorization = yield* Effect.tryPromise({
try: () =>
performOAuth(authProvider, {
serverUrl: url,
scope: oauthConfig?.scope,
fetchFn: createFetchWithInit(undefined, requestInit),
}),
catch: (error) => (error instanceof Error ? error : new Error(String(error))),
})

if (authorization === "REDIRECT") {
if (!capturedUrl) throw new Error(`OAuth redirect requested without authorization URL for MCP server: ${mcpName}`)
pendingOAuthTransports.set(mcpName, transport)
return { authorizationUrl: capturedUrl.toString(), oauthState } satisfies AuthResult
}

return yield* Effect.tryPromise({
try: () => {
Expand All @@ -802,19 +820,11 @@ export const layer = Layer.effect(
.then(() => ({ authorizationUrl: "", oauthState, client }) satisfies AuthResult)
},
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.die(error)
}),
)
})
})

const authenticate = Effect.fn("MCP.authenticate")(function* (mcpName: string) {
const result = yield* startAuth(mcpName)
const authenticateImpl = Effect.fn("MCP.authenticate")(function* (mcpName: string) {
const result = yield* beginAuth(mcpName)
if (!result.authorizationUrl) {
const client = "client" in result ? result.client : undefined
const mcpConfig = yield* getMcpConfig(mcpName)
Expand All @@ -829,8 +839,19 @@ export const layer = Layer.effect(
return { status: "failed", error: "Failed to get tools" } as Status
}

const s = yield* InstanceState.get(state)
yield* auth.clearOAuthState(mcpName)
yield* auth.clearCodeVerifier(mcpName)
const authStatus = yield* getAuthStatus(mcpName)
if (authStatus !== "authenticated") {
log.warn("oauth authenticate completed without stored tokens", { mcpName, authStatus })
yield* Effect.tryPromise(() => client.close()).pipe(Effect.ignore)
return {
status: "failed",
error: "Server connected without requesting OAuth. This MCP endpoint may not support interactive OAuth authentication.",
} as Status
}

const s = yield* InstanceState.get(state)
return yield* storeClient(s, mcpName, client, listed, mcpConfig.timeout)
}

Expand Down Expand Up @@ -871,6 +892,14 @@ export const layer = Layer.effect(
return yield* finishAuth(mcpName, code)
})

const startAuth = (mcpName: string) =>
beginAuth(mcpName).pipe(
Effect.map((result) => ({ authorizationUrl: result.authorizationUrl, oauthState: result.oauthState })),
Effect.orDie,
)

const authenticate = (mcpName: string) => authenticateImpl(mcpName).pipe(Effect.orDie)

const finishAuth = Effect.fn("MCP.finishAuth")(function* (mcpName: string, authorizationCode: string) {
const transport = pendingOAuthTransports.get(mcpName)
if (!transport) throw new Error(`No pending OAuth flow for MCP server: ${mcpName}`)
Expand Down
1 change: 1 addition & 0 deletions packages/opencode/test/mcp/lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ void mock.module("@modelcontextprotocol/sdk/client/auth.js", () => ({
super("Unauthorized")
}
},
auth: async () => "AUTHORIZED",
}))

// Mock Client that delegates to per-name MockClientState
Expand Down
73 changes: 70 additions & 3 deletions packages/opencode/test/mcp/oauth-auto-connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const transportCalls: Array<{
// auth flow (which calls provider.state()) or a simple UnauthorizedError.
let simulateAuthFlow = true
let connectSucceedsImmediately = false
let directAuthResult: "REDIRECT" | "AUTHORIZED" = "REDIRECT"

// Mock the transport constructors to simulate OAuth auto-auth on 401
void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
Expand Down Expand Up @@ -102,12 +103,26 @@ 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<string>
saveCodeVerifier?: (v: string) => Promise<void>
redirectToAuthorization?: (url: URL) => Promise<void>
},
) => {
if (directAuthResult === "AUTHORIZED") return "AUTHORIZED"
await provider.state?.()
await provider.saveCodeVerifier?.("test-verifier")
await provider.redirectToAuthorization?.(new URL("https://auth.example.com/authorize?state=test"))
return "REDIRECT"
},
}))

beforeEach(() => {
transportCalls.length = 0
simulateAuthFlow = true
connectSucceedsImmediately = false
directAuthResult = "REDIRECT"
})

// Import modules after mocking
Expand Down Expand Up @@ -210,27 +225,79 @@ mcpTest.instance("state() returns existing state when one is saved", () =>
}),
)

mcpTest.instance("invalidateCredentials() removes entries that only retain serverUrl", () =>
Effect.gen(function* () {
const auth = yield* McpAuth.Service
const provider = new McpOAuthProvider(
"test-stale-server-url",
"https://example.com/mcp",
{},
{ onRedirect: async () => {} },
auth,
)

yield* Effect.promise(() =>
provider.saveTokens({
access_token: "token",
token_type: "Bearer",
}),
)

expect((yield* auth.get("test-stale-server-url"))?.serverUrl).toBe("https://example.com/mcp")

yield* Effect.promise(() => provider.invalidateCredentials("tokens"))

expect(yield* auth.get("test-stale-server-url")).toBeUndefined()
expect((yield* auth.all())["test-stale-server-url"]).toBeUndefined()
}),
)

mcpTest.instance(
"authenticate() stores a connected client when auth completes without redirect",
"authenticate() fails clearly when transport reconnects without stored tokens",
() =>
MCP.Service.use((mcp) =>
Effect.gen(function* () {
const auth = yield* McpAuth.Service
const added = yield* mcp.add("test-oauth-connect", {
type: "remote",
url: "https://example.com/mcp",
})
const before = added.status as Record<string, { status: string; error?: string }>
expect(before["test-oauth-connect"]?.status).toBe("needs_auth")

directAuthResult = "AUTHORIZED"
simulateAuthFlow = false
connectSucceedsImmediately = true

const result = yield* mcp.authenticate("test-oauth-connect")
expect(result.status).toBe("connected")
expect(result.status).toBe("failed")
if (result.status === "failed") {
expect(result.error).toContain("may not support interactive OAuth authentication")
}

const entry = yield* auth.get("test-oauth-connect")
expect(entry).toBeUndefined()

const after = yield* mcp.status()
expect(after["test-oauth-connect"]?.status).toBe("connected")
expect(after["test-oauth-connect"]?.status).toBe("needs_auth")
}),
),
{ config: config("test-oauth-connect") },
)

mcpTest.instance(
"startAuth() returns an authorization URL even when transport can connect anonymously",
() =>
MCP.Service.use((mcp) =>
Effect.gen(function* () {
simulateAuthFlow = false
connectSucceedsImmediately = true
directAuthResult = "REDIRECT"

const result = yield* mcp.startAuth("test-oauth-direct")
expect(result.authorizationUrl).toContain("https://auth.example.com/authorize")
expect(result.oauthState.length).toBeGreaterThan(0)
}),
),
{ config: config("test-oauth-direct") },
)
12 changes: 12 additions & 0 deletions packages/opencode/test/mcp/oauth-browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,18 @@ 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 (
provider: {
state?: () => Promise<string>
saveCodeVerifier?: (v: string) => Promise<void>
redirectToAuthorization?: (url: URL) => Promise<void>
},
) => {
await provider.state?.()
await provider.saveCodeVerifier?.("test-verifier")
await provider.redirectToAuthorization?.(new URL("https://auth.example.com/authorize?client_id=test"))
return "REDIRECT"
},
}))

beforeEach(() => {
Expand Down
Loading