feat(mcp-oauth): add full OAuth 2.1 authentication for MCP servers (#1169)
* feat(mcp-oauth): add oauth field to ClaudeCodeMcpServer schema Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * feat(mcp-oauth): add RFC 7591 Dynamic Client Registration * feat(mcp-oauth): add RFC 9728 PRM + RFC 8414 AS discovery * feat(mcp-oauth): add secure token storage with {host}/{resource} key format * feat(mcp-oauth): add dynamic port OAuth callback server * feat(mcp-oauth): add RFC 8707 Resource Indicators * feat(mcp-oauth): implement full-spec McpOAuthProvider * feat(mcp-oauth): add step-up authorization handler * feat(mcp-oauth): integrate authProvider into SkillMcpManager * feat(doctor): add MCP OAuth token status check * feat(cli): add mcp oauth subcommand structure * feat(cli): implement mcp oauth login command * fix(mcp-oauth): address cubic review — security, correctness, and test issues - Remove @ts-nocheck from provider.ts, storage.ts, provider.test.ts - Fix server resource leak on missing code/state (close + reject) - Fix command injection in openBrowser (spawn array args, cross-platform) - Mock McpOAuthProvider in login.test.ts for deterministic CI - Recreate auth provider with merged scopes in step-up flow - Add listAllTokens() for global status listing - Fix logout to accept --server-url for correct token deletion - Support both quoted and unquoted WWW-Authenticate params (RFC 2617) - Save/restore OPENCODE_CONFIG_DIR in storage.test.ts - Fix index.test.ts: vitest → bun:test * fix(mcp-oauth): use explorer instead of cmd /c start on Windows to prevent shell injection * fix(mcp-oauth): address remaining cubic review issues - Add 5-minute timeout to provider callback server to prevent indefinite hangs - Persist client registration from token storage across process restarts - Require --server-url for logout to match token storage key format - Use listTokensByHost for server-specific status lookups - Fix callback-server test to handle promise rejection ordering - Fix provider test port expectations (8912 → 19877) - Fix cli-guide.md duplicate Section 7 numbering - Fix manager test for login-on-missing-tokens behavior * fix(mcp-oauth): address final review issues - P1: Redact token values in status.ts output to prevent credential leakage - P2: Read OAuth error response body before throwing in token exchange - Test: Fix mcp-oauth doctor test to use epoch seconds (not milliseconds) --------- Co-authored-by: justsisyphus <justsisyphus@users.noreply.github.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -0,0 +1,129 @@
|
||||
import { afterEach, describe, expect, it } from "bun:test"
|
||||
import { findAvailablePort, startCallbackServer, type CallbackServer } from "./callback-server"
|
||||
|
||||
describe("findAvailablePort", () => {
|
||||
it("returns the start port when it is available", async () => {
|
||||
//#given
|
||||
const startPort = 19877
|
||||
|
||||
//#when
|
||||
const port = await findAvailablePort(startPort)
|
||||
|
||||
//#then
|
||||
expect(port).toBeGreaterThanOrEqual(startPort)
|
||||
expect(port).toBeLessThan(startPort + 20)
|
||||
})
|
||||
|
||||
it("skips busy ports and returns next available", async () => {
|
||||
//#given
|
||||
const blocker = Bun.serve({
|
||||
port: 19877,
|
||||
hostname: "127.0.0.1",
|
||||
fetch: () => new Response(),
|
||||
})
|
||||
|
||||
//#when
|
||||
const port = await findAvailablePort(19877)
|
||||
|
||||
//#then
|
||||
expect(port).toBeGreaterThan(19877)
|
||||
blocker.stop(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe("startCallbackServer", () => {
|
||||
let server: CallbackServer | null = null
|
||||
|
||||
afterEach(() => {
|
||||
server?.close()
|
||||
server = null
|
||||
})
|
||||
|
||||
it("starts server and returns port", async () => {
|
||||
//#given - no preconditions
|
||||
|
||||
//#when
|
||||
server = await startCallbackServer()
|
||||
|
||||
//#then
|
||||
expect(server.port).toBeGreaterThanOrEqual(19877)
|
||||
expect(typeof server.waitForCallback).toBe("function")
|
||||
expect(typeof server.close).toBe("function")
|
||||
})
|
||||
|
||||
it("resolves callback with code and state from query params", async () => {
|
||||
//#given
|
||||
server = await startCallbackServer()
|
||||
const callbackUrl = `http://127.0.0.1:${server.port}/oauth/callback?code=test-code&state=test-state`
|
||||
|
||||
//#when
|
||||
const fetchPromise = fetch(callbackUrl)
|
||||
const result = await server.waitForCallback()
|
||||
const response = await fetchPromise
|
||||
|
||||
//#then
|
||||
expect(result).toEqual({ code: "test-code", state: "test-state" })
|
||||
expect(response.status).toBe(200)
|
||||
const html = await response.text()
|
||||
expect(html).toContain("Authorization successful")
|
||||
})
|
||||
|
||||
it("returns 404 for non-callback routes", async () => {
|
||||
//#given
|
||||
server = await startCallbackServer()
|
||||
|
||||
//#when
|
||||
const response = await fetch(`http://127.0.0.1:${server.port}/other`)
|
||||
|
||||
//#then
|
||||
expect(response.status).toBe(404)
|
||||
})
|
||||
|
||||
it("returns 400 and rejects when code is missing", async () => {
|
||||
//#given
|
||||
server = await startCallbackServer()
|
||||
const callbackRejection = server.waitForCallback().catch((e: Error) => e)
|
||||
|
||||
//#when
|
||||
const response = await fetch(`http://127.0.0.1:${server.port}/oauth/callback?state=s`)
|
||||
|
||||
//#then
|
||||
expect(response.status).toBe(400)
|
||||
const error = await callbackRejection
|
||||
expect(error).toBeInstanceOf(Error)
|
||||
expect((error as Error).message).toContain("missing code or state")
|
||||
})
|
||||
|
||||
it("returns 400 and rejects when state is missing", async () => {
|
||||
//#given
|
||||
server = await startCallbackServer()
|
||||
const callbackRejection = server.waitForCallback().catch((e: Error) => e)
|
||||
|
||||
//#when
|
||||
const response = await fetch(`http://127.0.0.1:${server.port}/oauth/callback?code=c`)
|
||||
|
||||
//#then
|
||||
expect(response.status).toBe(400)
|
||||
const error = await callbackRejection
|
||||
expect(error).toBeInstanceOf(Error)
|
||||
expect((error as Error).message).toContain("missing code or state")
|
||||
})
|
||||
|
||||
it("close stops the server immediately", async () => {
|
||||
//#given
|
||||
server = await startCallbackServer()
|
||||
const port = server.port
|
||||
|
||||
//#when
|
||||
server.close()
|
||||
server = null
|
||||
|
||||
//#then
|
||||
try {
|
||||
await fetch(`http://127.0.0.1:${port}/oauth/callback?code=c&state=s`)
|
||||
expect(true).toBe(false)
|
||||
} catch (error) {
|
||||
expect(error).toBeDefined()
|
||||
}
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user