fix(opencode): preserve valid MCP OAuth state
This commit is contained in:
parent
ed486f0b49
commit
bb388efae6
6 changed files with 25 additions and 25 deletions
|
|
@ -147,11 +147,10 @@ export const layer = Layer.effect(
|
||||||
return yield* Effect.gen(function* () {
|
return yield* Effect.gen(function* () {
|
||||||
const data = yield* read()
|
const data = yield* read()
|
||||||
const entry = data[mcpName]
|
const entry = data[mcpName]
|
||||||
if (!entry?.oauthState) return false
|
if (entry?.oauthState !== oauthState) return false
|
||||||
const matches = entry.oauthState === oauthState
|
|
||||||
delete entry.oauthState
|
delete entry.oauthState
|
||||||
yield* fs.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600).pipe(Effect.orDie)
|
yield* fs.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600).pipe(Effect.orDie)
|
||||||
return matches
|
return true
|
||||||
}).pipe(flock.withLock(lockKey), Effect.orDie)
|
}).pipe(flock.withLock(lockKey), Effect.orDie)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -951,14 +951,16 @@ export const layer = Layer.effect(
|
||||||
oauthState: string,
|
oauthState: string,
|
||||||
) {
|
) {
|
||||||
yield* requireMcpConfig(mcpName)
|
yield* requireMcpConfig(mcpName)
|
||||||
const transport = pendingOAuthTransports.get(mcpName)
|
|
||||||
if (!transport) return yield* new OAuthError({ message: `No pending OAuth flow for MCP server: ${mcpName}` })
|
|
||||||
|
|
||||||
if (!(yield* auth.consumeOAuthState(mcpName, oauthState))) {
|
if (!(yield* auth.consumeOAuthState(mcpName, oauthState))) {
|
||||||
yield* cleanupAuth(mcpName)
|
|
||||||
return yield* new OAuthError({ message: "Invalid or expired OAuth state - potential CSRF attack" })
|
return yield* new OAuthError({ message: "Invalid or expired OAuth state - potential CSRF attack" })
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const transport = pendingOAuthTransports.get(mcpName)
|
||||||
|
if (!transport) {
|
||||||
|
yield* cleanupAuth(mcpName)
|
||||||
|
return yield* new OAuthError({ message: `No pending OAuth flow for MCP server: ${mcpName}` })
|
||||||
|
}
|
||||||
|
|
||||||
const result = yield* Effect.tryPromise({
|
const result = yield* Effect.tryPromise({
|
||||||
try: () => transport.finishAuth(authorizationCode).then(() => true as const),
|
try: () => transport.finishAuth(authorizationCode).then(() => true as const),
|
||||||
catch: (error) => {
|
catch: (error) => {
|
||||||
|
|
|
||||||
|
|
@ -88,7 +88,7 @@ export const McpApi = HttpApi.make("mcp")
|
||||||
identifier: "mcp.auth.callback",
|
identifier: "mcp.auth.callback",
|
||||||
summary: "Complete MCP OAuth",
|
summary: "Complete MCP OAuth",
|
||||||
description:
|
description:
|
||||||
"Complete OAuth authentication for a Model Context Protocol (MCP) server using the authorization code.",
|
"Complete OAuth authentication for a Model Context Protocol (MCP) server using the authorization code and state returned by the start endpoint.",
|
||||||
}),
|
}),
|
||||||
),
|
),
|
||||||
HttpApiEndpoint.post("authAuthenticate", McpPaths.authAuthenticate, {
|
HttpApiEndpoint.post("authAuthenticate", McpPaths.authAuthenticate, {
|
||||||
|
|
|
||||||
|
|
@ -2,7 +2,7 @@ import { NodeHttpServer, NodeServices } from "@effect/platform-node"
|
||||||
import Http from "node:http"
|
import Http from "node:http"
|
||||||
import path from "node:path"
|
import path from "node:path"
|
||||||
import { describe, expect } from "bun:test"
|
import { describe, expect } from "bun:test"
|
||||||
import { Config, Context, Effect, Layer } from "effect"
|
import { Config, Context, Effect, Layer, Ref } from "effect"
|
||||||
import {
|
import {
|
||||||
HttpClient,
|
HttpClient,
|
||||||
HttpClientRequest,
|
HttpClientRequest,
|
||||||
|
|
@ -32,7 +32,7 @@ const it = testEffect(
|
||||||
const callbackPath = McpPaths.authCallback.replace(":name", "secure-oauth")
|
const callbackPath = McpPaths.authCallback.replace(":name", "secure-oauth")
|
||||||
const authPath = McpPaths.auth.replace(":name", "secure-oauth")
|
const authPath = McpPaths.auth.replace(":name", "secure-oauth")
|
||||||
|
|
||||||
function listenOAuthServer(tokenCalls: { value: number }) {
|
function listenOAuthServer(tokenCalls: Ref.Ref<number>) {
|
||||||
return Effect.gen(function* () {
|
return Effect.gen(function* () {
|
||||||
const context = yield* Layer.build(NodeHttpServer.layer(Http.createServer, { host: "127.0.0.1", port: 0 }))
|
const context = yield* Layer.build(NodeHttpServer.layer(Http.createServer, { host: "127.0.0.1", port: 0 }))
|
||||||
const server = Context.get(context, HttpServer.HttpServer)
|
const server = Context.get(context, HttpServer.HttpServer)
|
||||||
|
|
@ -57,7 +57,7 @@ function listenOAuthServer(tokenCalls: { value: number }) {
|
||||||
if (url.pathname === "/token")
|
if (url.pathname === "/token")
|
||||||
return Effect.gen(function* () {
|
return Effect.gen(function* () {
|
||||||
yield* request.text
|
yield* request.text
|
||||||
tokenCalls.value++
|
yield* Ref.update(tokenCalls, (value) => value + 1)
|
||||||
return yield* HttpServerResponse.json({ access_token: "access-token", token_type: "Bearer" })
|
return yield* HttpServerResponse.json({ access_token: "access-token", token_type: "Bearer" })
|
||||||
})
|
})
|
||||||
if (url.pathname !== "/mcp") return Effect.succeed(HttpServerResponse.empty({ status: 404 }))
|
if (url.pathname !== "/mcp") return Effect.succeed(HttpServerResponse.empty({ status: 404 }))
|
||||||
|
|
@ -130,7 +130,7 @@ function assertPortAvailable(port: number) {
|
||||||
function setup() {
|
function setup() {
|
||||||
return Effect.gen(function* () {
|
return Effect.gen(function* () {
|
||||||
const directory = yield* tmpdirScoped({ git: true })
|
const directory = yield* tmpdirScoped({ git: true })
|
||||||
const tokenCalls = { value: 0 }
|
const tokenCalls = yield* Ref.make(0)
|
||||||
const upstream = yield* listenOAuthServer(tokenCalls)
|
const upstream = yield* listenOAuthServer(tokenCalls)
|
||||||
const callbackPort = yield* availablePort()
|
const callbackPort = yield* availablePort()
|
||||||
yield* Effect.promise(() =>
|
yield* Effect.promise(() =>
|
||||||
|
|
@ -169,24 +169,19 @@ describe("mcp HttpApi OAuth", () => {
|
||||||
|
|
||||||
const missing = yield* request(test.directory, callbackPath, { code: "missing-state" })
|
const missing = yield* request(test.directory, callbackPath, { code: "missing-state" })
|
||||||
expect(missing.status).toBe(400)
|
expect(missing.status).toBe(400)
|
||||||
expect(test.tokenCalls.value).toBe(0)
|
expect(yield* Ref.get(test.tokenCalls)).toBe(0)
|
||||||
|
|
||||||
const wrong = yield* request(test.directory, callbackPath, { code: "wrong-state", state: "wrong" })
|
const wrong = yield* request(test.directory, callbackPath, { code: "wrong-state", state: "wrong" })
|
||||||
expect(wrong.status).toBe(400)
|
expect(wrong.status).toBe(400)
|
||||||
expect(test.tokenCalls.value).toBe(0)
|
expect(yield* Ref.get(test.tokenCalls)).toBe(0)
|
||||||
|
|
||||||
const restarted = yield* request(test.directory, authPath)
|
const correct = yield* request(test.directory, callbackPath, { code: "valid-code", state: first.oauthState })
|
||||||
expect(restarted.status).toBe(200)
|
|
||||||
const second = (yield* restarted.json) as { oauthState: string }
|
|
||||||
expect(second.oauthState).not.toBe(first.oauthState)
|
|
||||||
|
|
||||||
const correct = yield* request(test.directory, callbackPath, { code: "valid-code", state: second.oauthState })
|
|
||||||
expect(correct.status).toBe(200)
|
expect(correct.status).toBe(200)
|
||||||
expect(test.tokenCalls.value).toBe(1)
|
expect(yield* Ref.get(test.tokenCalls)).toBe(1)
|
||||||
|
|
||||||
const replayed = yield* request(test.directory, callbackPath, { code: "replayed-code", state: second.oauthState })
|
const replayed = yield* request(test.directory, callbackPath, { code: "replayed-code", state: first.oauthState })
|
||||||
expect(replayed.status).toBe(400)
|
expect(replayed.status).toBe(400)
|
||||||
expect(test.tokenCalls.value).toBe(1)
|
expect(yield* Ref.get(test.tokenCalls)).toBe(1)
|
||||||
}),
|
}),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -199,7 +199,11 @@ describe("mcp HttpApi", () => {
|
||||||
for (const input of [
|
for (const input of [
|
||||||
{ method: "POST", route: "/mcp/missing/auth" },
|
{ method: "POST", route: "/mcp/missing/auth" },
|
||||||
{ method: "POST", route: "/mcp/missing/auth/authenticate" },
|
{ method: "POST", route: "/mcp/missing/auth/authenticate" },
|
||||||
{ method: "POST", route: "/mcp/missing/auth/callback", body: JSON.stringify({ code: "code" }) },
|
{
|
||||||
|
method: "POST",
|
||||||
|
route: "/mcp/missing/auth/callback",
|
||||||
|
body: JSON.stringify({ code: "code", state: "state" }),
|
||||||
|
},
|
||||||
{ method: "DELETE", route: "/mcp/missing/auth" },
|
{ method: "DELETE", route: "/mcp/missing/auth" },
|
||||||
{ method: "POST", route: "/mcp/missing/connect" },
|
{ method: "POST", route: "/mcp/missing/connect" },
|
||||||
{ method: "POST", route: "/mcp/missing/disconnect" },
|
{ method: "POST", route: "/mcp/missing/disconnect" },
|
||||||
|
|
|
||||||
|
|
@ -2296,7 +2296,7 @@ export class Auth2 extends HeyApiClient {
|
||||||
/**
|
/**
|
||||||
* Complete MCP OAuth
|
* Complete MCP OAuth
|
||||||
*
|
*
|
||||||
* Complete OAuth authentication for a Model Context Protocol (MCP) server using the authorization code.
|
* Complete OAuth authentication for a Model Context Protocol (MCP) server using the authorization code and state returned by the start endpoint.
|
||||||
*/
|
*/
|
||||||
public callback<ThrowOnError extends boolean = false>(
|
public callback<ThrowOnError extends boolean = false>(
|
||||||
parameters: {
|
parameters: {
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue