Compare commits

...

3 Commits

Author SHA1 Message Date
Shoubhit Dash b4344098de fix(mcp): validate oauth callback port range 2026-05-22 16:47:36 +05:30
Sebin Thomas bca26b0094 test(mcp): add McpOAuthProvider unit tests for scope and callbackPort
Covers:
- redirectUrl uses callbackPort when set, falls back to default 19876
- redirectUri takes precedence over callbackPort when both are set
- clientMetadata includes scope when set in config
- clientMetadata omits scope when not set (backward compat)
- token_endpoint_auth_method based on clientSecret presence

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
2026-05-22 12:08:51 +02:00
Sebin Thomas e9ca7d292b fix(mcp): include scope in clientMetadata and add callbackPort option
Two related OAuth issues with remote MCP servers:

1. `scope` config field was accepted but never propagated to
   `clientMetadata`. The MCP SDK uses `clientMetadata.scope` as its
   last-resort fallback when neither the WWW-Authenticate header nor
   the Protected Resource Metadata (scopes_supported) advertise scopes.
   For servers whose metadata returns no scopes (e.g. AWS Bedrock
   AgentCore), the authorization request was sent with no scope
   parameter, causing IdPs such as Okta to reject with
   "No scopes were requested."

2. The callback port was hardcoded to 19876 with no way to override it
   short of providing a full `redirectUri`. Added `callbackPort` as a
   shorthand in the OAuth config — when set it constructs the redirect
   URI as `http://127.0.0.1:<callbackPort>/mcp/oauth/callback`.
   `redirectUri` still takes precedence if both are provided.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
2026-05-22 11:58:19 +02:00
4 changed files with 80 additions and 4 deletions
+4
View File
@@ -26,6 +26,10 @@ export const OAuth = Schema.Struct({
description: "OAuth client secret (if required by the authorization server)",
}),
scope: Schema.optional(Schema.String).annotate({ description: "OAuth scopes to request during authorization" }),
callbackPort: Schema.optional(Schema.Int.check(Schema.isBetween({ minimum: 1, maximum: 65535 }))).annotate({
description:
"Port for the local OAuth callback server (default: 19876). Shorthand for redirectUri when only the port needs changing. Ignored if redirectUri is set.",
}),
redirectUri: Schema.optional(Schema.String).annotate({
description: "OAuth redirect URI (default: http://127.0.0.1:19876/mcp/oauth/callback).",
}),
+11 -3
View File
@@ -19,7 +19,7 @@ import { NamedError } from "@opencode-ai/core/util/error"
import { InstallationVersion } from "@opencode-ai/core/installation/version"
import { withTimeout } from "@/util/timeout"
import { AppFileSystem } from "@opencode-ai/core/filesystem"
import { McpOAuthProvider } from "./oauth-provider"
import { McpOAuthProvider, OAUTH_CALLBACK_PATH } from "./oauth-provider"
import { McpOAuthCallback } from "./oauth-callback"
import { McpAuth } from "./auth"
import { BusEvent } from "../bus/bus-event"
@@ -318,6 +318,7 @@ export const layer = Layer.effect(
clientId: oauthConfig?.clientId,
clientSecret: oauthConfig?.clientSecret,
scope: oauthConfig?.scope,
callbackPort: oauthConfig?.callbackPort,
redirectUri: oauthConfig?.redirectUri,
},
{
@@ -769,8 +770,15 @@ export const layer = Layer.effect(
// OAuth config is optional - if not provided, we'll use auto-discovery
const oauthConfig = typeof mcpConfig.oauth === "object" ? mcpConfig.oauth : undefined
// Resolve effective redirect URI: explicit redirectUri > callbackPort shorthand > default
const effectiveRedirectUri =
oauthConfig?.redirectUri ??
(oauthConfig?.callbackPort
? `http://127.0.0.1:${oauthConfig.callbackPort}${OAUTH_CALLBACK_PATH}`
: undefined)
// Start the callback server with custom redirectUri if configured
yield* Effect.promise(() => McpOAuthCallback.ensureRunning(oauthConfig?.redirectUri))
yield* Effect.promise(() => McpOAuthCallback.ensureRunning(effectiveRedirectUri))
const oauthState = Array.from(crypto.getRandomValues(new Uint8Array(32)))
.map((b) => b.toString(16).padStart(2, "0"))
@@ -784,7 +792,7 @@ export const layer = Layer.effect(
clientId: oauthConfig?.clientId,
clientSecret: oauthConfig?.clientSecret,
scope: oauthConfig?.scope,
redirectUri: oauthConfig?.redirectUri,
redirectUri: effectiveRedirectUri,
},
{
onRedirect: async (url) => {
+4 -1
View File
@@ -18,6 +18,7 @@ export interface McpOAuthConfig {
clientId?: string
clientSecret?: string
scope?: string
callbackPort?: number
redirectUri?: string
}
@@ -38,7 +39,8 @@ export class McpOAuthProvider implements OAuthClientProvider {
if (this.config.redirectUri) {
return this.config.redirectUri
}
return `http://127.0.0.1:${OAUTH_CALLBACK_PORT}${OAUTH_CALLBACK_PATH}`
const port = this.config.callbackPort ?? OAUTH_CALLBACK_PORT
return `http://127.0.0.1:${port}${OAUTH_CALLBACK_PATH}`
}
get clientMetadata(): OAuthClientMetadata {
@@ -49,6 +51,7 @@ export class McpOAuthProvider implements OAuthClientProvider {
grant_types: ["authorization_code", "refresh_token"],
response_types: ["code"],
token_endpoint_auth_method: this.config.clientSecret ? "client_secret_post" : "none",
...(this.config.scope ? { scope: this.config.scope } : {}),
}
}
@@ -0,0 +1,61 @@
import { test, expect, describe } from "bun:test"
import { McpOAuthProvider, OAUTH_CALLBACK_PORT, OAUTH_CALLBACK_PATH } from "../../src/mcp/oauth-provider"
import type { McpAuth } from "../../src/mcp/auth"
// Stub auth — only synchronous getters are exercised in these tests
const stubAuth = {} as McpAuth.Interface
const makeProvider = (config: ConstructorParameters<typeof McpOAuthProvider>[2]) =>
new McpOAuthProvider("test-server", "https://mcp.example.com/mcp", config, { onRedirect: async () => {} }, stubAuth)
describe("McpOAuthProvider.redirectUrl", () => {
test("defaults to 127.0.0.1:19876/mcp/oauth/callback", () => {
const provider = makeProvider({})
expect(provider.redirectUrl).toBe(`http://127.0.0.1:${OAUTH_CALLBACK_PORT}${OAUTH_CALLBACK_PATH}`)
})
test("uses callbackPort when set", () => {
const provider = makeProvider({ callbackPort: 6620 })
expect(provider.redirectUrl).toBe(`http://127.0.0.1:6620${OAUTH_CALLBACK_PATH}`)
})
test("redirectUri takes precedence over callbackPort", () => {
const provider = makeProvider({
callbackPort: 6620,
redirectUri: "http://127.0.0.1:9999/custom/callback",
})
expect(provider.redirectUrl).toBe("http://127.0.0.1:9999/custom/callback")
})
test("uses explicit redirectUri when set without callbackPort", () => {
const provider = makeProvider({ redirectUri: "http://127.0.0.1:8080/oauth/callback" })
expect(provider.redirectUrl).toBe("http://127.0.0.1:8080/oauth/callback")
})
})
describe("McpOAuthProvider.clientMetadata", () => {
test("includes redirect_uris from redirectUrl", () => {
const provider = makeProvider({ callbackPort: 6620 })
expect(provider.clientMetadata.redirect_uris).toEqual([`http://127.0.0.1:6620${OAUTH_CALLBACK_PATH}`])
})
test("includes scope when set in config", () => {
const provider = makeProvider({ scope: "openid offline_access" })
expect(provider.clientMetadata.scope).toBe("openid offline_access")
})
test("omits scope when not set in config", () => {
const provider = makeProvider({})
expect(provider.clientMetadata.scope).toBeUndefined()
})
test("sets token_endpoint_auth_method to client_secret_post when clientSecret provided", () => {
const provider = makeProvider({ clientSecret: "secret" })
expect(provider.clientMetadata.token_endpoint_auth_method).toBe("client_secret_post")
})
test("sets token_endpoint_auth_method to none when no clientSecret", () => {
const provider = makeProvider({})
expect(provider.clientMetadata.token_endpoint_auth_method).toBe("none")
})
})