Bläddra i källkod

fix(mcp): verify explicit OAuth authentication

Aiden Cline 1 månad sedan
förälder
incheckning
5ad089fc01

+ 12 - 0
packages/opencode/src/mcp/index.ts

@@ -859,6 +859,7 @@ export const layer = Layer.effect(
     })
 
     const authenticate = Effect.fn("MCP.authenticate")(function* (mcpName: string) {
+      const previousTokens = (yield* auth.get(mcpName))?.tokens
       const result = yield* startAuth(mcpName)
       if (!result.authorizationUrl) {
         const client = "client" in result ? result.client : undefined
@@ -876,6 +877,17 @@ export const layer = Layer.effect(
           return { status: "failed", error: "Failed to get tools" } satisfies Status
         }
 
+        const currentTokens = (yield* auth.get(mcpName))?.tokens
+        if (!currentTokens || JSON.stringify(currentTokens) === JSON.stringify(previousTokens)) {
+          yield* Effect.tryPromise(() => client.close()).pipe(Effect.ignore)
+          yield* auth.clearOAuthState(mcpName)
+          return {
+            status: "failed",
+            error:
+              "The server did not issue a standard OAuth challenge. Anonymous MCP access remains available, but authentication was not completed. Verify the server's OAuth configuration or use credentials supported by the server.",
+          } satisfies Status
+        }
+
         const s = yield* InstanceState.get(state)
         yield* auth.clearOAuthState(mcpName)
         return yield* storeClient(s, mcpName, client, listed, client.getInstructions()?.trim(), mcpConfig.timeout)

+ 82 - 0
packages/opencode/test/mcp/oauth-anonymous.test.ts

@@ -0,0 +1,82 @@
+import { afterAll, expect } from "bun:test"
+import { LATEST_PROTOCOL_VERSION } from "@modelcontextprotocol/sdk/types.js"
+import { Effect } from "effect"
+import { MCP } from "../../src/mcp/index"
+import { testEffect } from "../lib/effect"
+
+const server = Bun.serve({
+  port: 0,
+  async fetch(request) {
+    if (request.method !== "POST") return new Response(null, { status: 405 })
+
+    const message = (await request.json()) as { id?: number; method: string }
+    if (message.method === "initialize") {
+      return Response.json({
+        jsonrpc: "2.0",
+        id: message.id,
+        result: {
+          protocolVersion: LATEST_PROTOCOL_VERSION,
+          capabilities: { tools: {} },
+          serverInfo: { name: "anonymous-oauth-test", version: "1" },
+        },
+      })
+    }
+    if (message.method === "notifications/initialized") return new Response(null, { status: 202 })
+    if (message.method === "tools/list") {
+      return Response.json({
+        jsonrpc: "2.0",
+        id: message.id,
+        result: {
+          tools: [{ name: "protected", inputSchema: { type: "object", properties: {} } }],
+        },
+      })
+    }
+    if (message.method === "tools/call") {
+      return new Response("Authentication required", {
+        status: 401,
+        headers: { "WWW-Authenticate": `Bearer resource_metadata="${server.url}.well-known/oauth-protected-resource"` },
+      })
+    }
+    return Response.json({ jsonrpc: "2.0", id: message.id, error: { code: -32601, message: "Method not found" } })
+  },
+})
+
+afterAll(() => server.stop(true))
+
+const it = testEffect(MCP.defaultLayer)
+
+it.instance(
+  "explicit auth fails when anonymous initialize and catalog emit no OAuth challenge",
+  () =>
+    MCP.Service.use((mcp) =>
+      Effect.gen(function* () {
+        const added = yield* mcp.add("anonymous-oauth", { type: "remote", url: server.url.toString() })
+        expect(added.status).toEqual({ "anonymous-oauth": { status: "connected" } })
+        expect(Object.keys(yield* mcp.tools())).toEqual(["anonymous-oauth_protected"])
+
+        const protectedResponse = yield* Effect.promise(() =>
+          fetch(server.url, {
+            method: "POST",
+            headers: { "content-type": "application/json" },
+            body: JSON.stringify({
+              jsonrpc: "2.0",
+              id: 1,
+              method: "tools/call",
+              params: { name: "protected", arguments: {} },
+            }),
+          }),
+        )
+        expect(protectedResponse.status).toBe(401)
+
+        const result = yield* mcp.authenticate("anonymous-oauth")
+        expect(result).toEqual({
+          status: "failed",
+          error:
+            "The server did not issue a standard OAuth challenge. Anonymous MCP access remains available, but authentication was not completed. Verify the server's OAuth configuration or use credentials supported by the server.",
+        })
+        expect(yield* mcp.hasStoredTokens("anonymous-oauth")).toBe(false)
+        expect(yield* mcp.status()).toEqual({ "anonymous-oauth": { status: "connected" } })
+      }),
+    ),
+  { config: { mcp: { "anonymous-oauth": { type: "remote", url: server.url.toString() } } } },
+)

+ 11 - 2
packages/opencode/test/mcp/oauth-auto-connect.test.ts

@@ -21,6 +21,7 @@ const transportCalls: Array<{
 // auth flow (which calls provider.state()) or a simple UnauthorizedError.
 let simulateAuthFlow = true
 let connectSucceedsImmediately = false
+let saveTokensOnConnect = false
 let serverCapabilities: { tools?: object; resources?: object } = { tools: {} }
 let listToolsCalls = 0
 
@@ -32,6 +33,7 @@ void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
           state?: () => Promise<string>
           redirectToAuthorization?: (url: URL) => Promise<void>
           saveCodeVerifier?: (v: string) => Promise<void>
+          saveTokens?: (tokens: { access_token: string; token_type: string }) => Promise<void>
         }
       | undefined
     constructor(url: URL, options?: { authProvider?: unknown }) {
@@ -43,7 +45,11 @@ void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
       })
     }
     async start() {
-      if (connectSucceedsImmediately) return
+      if (connectSucceedsImmediately) {
+        if (saveTokensOnConnect)
+          await this.authProvider?.saveTokens?.({ access_token: "new-token", token_type: "bearer" })
+        return
+      }
 
       // Simulate what the real SDK transport does on 401:
       // It calls auth() which eventually calls provider.state(), then
@@ -123,6 +129,7 @@ beforeEach(() => {
   transportCalls.length = 0
   simulateAuthFlow = true
   connectSucceedsImmediately = false
+  saveTokensOnConnect = false
   serverCapabilities = { tools: {} }
   listToolsCalls = 0
 })
@@ -228,7 +235,7 @@ mcpTest.instance("state() returns existing state when one is saved", () =>
 )
 
 mcpTest.instance(
-  "authenticate() stores a connected client when auth completes without redirect",
+  "authenticate() stores a connected client when stored credentials connect without redirect",
   () =>
     MCP.Service.use((mcp) =>
       Effect.gen(function* () {
@@ -241,6 +248,7 @@ mcpTest.instance(
 
         simulateAuthFlow = false
         connectSucceedsImmediately = true
+        saveTokensOnConnect = true
 
         const result = yield* mcp.authenticate("test-oauth-connect")
         expect(result.status).toBe("connected")
@@ -266,6 +274,7 @@ mcpTest.instance(
 
         simulateAuthFlow = false
         connectSucceedsImmediately = true
+        saveTokensOnConnect = true
         serverCapabilities = { resources: {} }
 
         const result = yield* mcp.authenticate("test-oauth-resources")