Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
37d0bde
feat(SecretStorageService): service for mcp hub to store access and r…
edelauna Mar 6, 2026
5128372
feat(callbackServer): used for OAuth 2.1 callbacks
edelauna Mar 6, 2026
288f147
fix(@modelcontextprotocol): overwrites
edelauna Mar 6, 2026
1aa1184
feat(McpOAuthClientProvider): implementing provider to allow OAuth fo…
edelauna Mar 6, 2026
0afa23c
fix(callbackServer): removing reflected xss and updating template
edelauna Mar 7, 2026
3121d36
fix(McpHub): not blocking on authprovider flow
edelauna Mar 7, 2026
8f314c4
refactor(McpOAuthClientProvider): do not default to openid scope
edelauna Mar 11, 2026
240cc81
feat(McpHub): handle OAuth refresh mid tool call
edelauna Mar 11, 2026
c9f981e
fix(McpOAuthClientProvider): refresh token race condition between cli…
edelauna Mar 12, 2026
441deac
feat(McpHub): update OAuth Flow to be opt in and handle concurrent fl…
edelauna Mar 14, 2026
faffb4b
refactor(McpHub): prompt for authentication even mid tool call
edelauna Mar 25, 2026
ab8d12d
refactor(callbackServer): added in i18n translations
edelauna Mar 25, 2026
7c0d0e3
feat(McpHub): cache whether server required OAuth or not
edelauna Mar 27, 2026
b1d44ce
refactor(McpOAuthClientProvider): add TOKEN_EXPIRY_BUFFER
edelauna Mar 27, 2026
f2a3cfb
refactor(SecretStorageService): caching full DCR repose
edelauna Mar 27, 2026
8eb8be9
refactor(McpHub): making prompt long running or dismissable
edelauna Apr 3, 2026
f10d254
feat(McpHub): handling concurrent window auth
edelauna Apr 24, 2026
fda3e98
feat(McpHub): making oauth less disruptive to flow
edelauna Apr 24, 2026
ba5345c
fix(esbuild): sequence rebuild calls to prevent Windows EBUSY race on…
edelauna Apr 24, 2026
a127704
refactor(McpHub): cleanup crosswindow auths
edelauna Apr 24, 2026
b9d2852
refactor(SecretStorageService): base64 encode server endpoints to avo…
edelauna Apr 24, 2026
cde464e
refactor(callbackServer): make cancellable
edelauna Apr 24, 2026
4f62b40
fix(i18n): catalan strings
edelauna Apr 24, 2026
dab4e2c
test(oauth): restoring mocked global
edelauna Apr 24, 2026
f0e1121
feat(McpOAuthClientProvider): adding callback to cancel callback server
edelauna Apr 24, 2026
05b399b
test: feedback
edelauna Apr 24, 2026
7840d03
empty commit for ci
edelauna Apr 26, 2026
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
372 changes: 372 additions & 0 deletions apps/vscode-e2e/src/suite/mcp-oauth.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,372 @@
import * as assert from "assert"
import * as fs from "fs/promises"
import * as path from "path"
import * as os from "os"
import * as http from "http"
import * as vscode from "vscode"

import { waitFor, sleep } from "./utils"
import { setDefaultSuiteTimeout } from "./test-utils"

/**
* Minimal MCP-protocol-aware request handler.
*
* The SDK's StreamableHTTPClientTransport uses:
* - GET /mcp → SSE stream (we return 405 to indicate not supported)
* - POST /mcp → JSON-RPC messages (initialize, tools/list, etc.)
*/
function handleMcpRequest(req: http.IncomingMessage, res: http.ServerResponse, endpointsHit: Set<string>): void {
if (req.method === "GET") {
// Signal that we don't support the SSE push channel.
// The SDK treats 405 as "SSE not supported, POST-only mode".
endpointsHit.add("mcp-authed-get")
res.writeHead(405)
res.end()
return
}

// POST — read body, parse JSON-RPC, dispatch
let body = ""
req.on("data", (chunk) => (body += chunk))
req.on("end", () => {
endpointsHit.add("mcp-authed")

let message: { id?: number; method?: string }
try {
message = JSON.parse(body)
} catch {
res.writeHead(400)
res.end()
return
}

// Notifications (no id) → 202 Accepted
if (message.id === undefined) {
res.writeHead(202)
res.end()
return
}

let result: unknown
switch (message.method) {
case "initialize":
result = {
protocolVersion: "2024-11-05",
capabilities: {},
serverInfo: { name: "test-oauth-server", version: "1.0.0" },
}
break
case "tools/list":
result = { tools: [] }
break
case "resources/list":
result = { resources: [] }
break
case "resources/templates/list":
result = { resourceTemplates: [] }
break
default:
result = {}
}

res.writeHead(200, { "Content-Type": "application/json" })
res.end(JSON.stringify({ jsonrpc: "2.0", id: message.id, result }))
})
}

suite("Roo Code MCP OAuth", function () {
setDefaultSuiteTimeout(this)

let tempDir: string
let testFiles: { mcpConfig: string }
let mockServer: http.Server
let mockServerPort: number

// Track which OAuth / MCP endpoints were hit
const endpointsHit: Set<string> = new Set()

suiteSetup(async () => {
// Enable test mode so the OAuth callback server resolves immediately
// without needing a real browser redirect.
process.env.MCP_OAUTH_TEST_MODE = "true"

tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "roo-test-mcp-oauth-"))

mockServer = http.createServer((req, res) => {
const url = req.url || ""
console.log(`[MOCK SERVER] ${req.method} ${url}`)

// ── MCP endpoint ─────────────────────────────────────────────
if (url === "/mcp" || url.startsWith("/mcp?") || url.startsWith("/mcp/")) {
const authHeader = req.headers.authorization
if (!authHeader || !authHeader.startsWith("Bearer ")) {
endpointsHit.add("mcp-401")
res.writeHead(401, {
"WWW-Authenticate": `Bearer resource_metadata="http://localhost:${mockServerPort}/.well-known/oauth-protected-resource"`,
})
res.end()
} else {
// Authenticated — handle as MCP protocol
handleMcpRequest(req, res, endpointsHit)
}
return
}

// ── OAuth discovery / registration / token endpoints ─────────

if (url === "/.well-known/oauth-protected-resource") {
endpointsHit.add("resource-metadata")
res.writeHead(200, { "Content-Type": "application/json" })
res.end(
JSON.stringify({
resource: `http://localhost:${mockServerPort}/mcp`,
authorization_servers: [`http://localhost:${mockServerPort}/auth`],
}),
)
return
}

// SDK constructs: new URL("/.well-known/oauth-authorization-server", "http://host/auth")
// which resolves to http://host/.well-known/oauth-authorization-server (origin-relative)
// Our custom fetchOAuthAuthServerMetadata constructs the RFC 8414 URL with issuer path:
// /.well-known/oauth-authorization-server/auth (with issuer path)
// Handle BOTH forms so our provider gets _authServerMeta.
if (
url === "/.well-known/oauth-authorization-server" ||
url === "/.well-known/oauth-authorization-server/auth"
) {
endpointsHit.add("auth-metadata")
res.writeHead(200, { "Content-Type": "application/json" })
res.end(
JSON.stringify({
issuer: `http://localhost:${mockServerPort}/auth`,
authorization_endpoint: `http://localhost:${mockServerPort}/auth/authorize`,
token_endpoint: `http://localhost:${mockServerPort}/auth/token`,
registration_endpoint: `http://localhost:${mockServerPort}/auth/register`,
code_challenge_methods_supported: ["S256"],
response_types_supported: ["code"],
}),
)
return
}

if (url === "/auth/register" && req.method === "POST") {
endpointsHit.add("register")
res.writeHead(201, { "Content-Type": "application/json" })
res.end(
JSON.stringify({
client_id: "test-client-id",
redirect_uris: ["http://localhost:3000/callback"],
}),
)
return
}

if (url === "/auth/token" && req.method === "POST") {
endpointsHit.add("token")
res.writeHead(200, { "Content-Type": "application/json" })
res.end(
JSON.stringify({
access_token: "test-access-token",
token_type: "Bearer",
expires_in: 3600,
}),
)
return
}

// Capture authorize hits (only reachable if a real browser is present)
if (url.startsWith("/auth/authorize")) {
endpointsHit.add("authorize")
res.writeHead(200, { "Content-Type": "text/plain" })
res.end("Authorization endpoint reached")
return
}

res.writeHead(404)
res.end()
})

// Find an available port
mockServerPort = await new Promise<number>((resolve, reject) => {
mockServer.listen(0, "127.0.0.1", () => {
const addr = mockServer.address()
if (!addr || typeof addr === "string") return reject(new Error("Failed to get address"))
resolve(addr.port)
})
mockServer.on("error", reject)
})

const workspaceDir = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath || tempDir
const rooDir = path.join(workspaceDir, ".roo")
await fs.mkdir(rooDir, { recursive: true })

const mcpConfig = {
mcpServers: {
"test-oauth-server": {
type: "streamable-http",
url: `http://localhost:${mockServerPort}/mcp`,
},
},
}

testFiles = { mcpConfig: path.join(rooDir, "mcp.json") }
await fs.writeFile(testFiles.mcpConfig, JSON.stringify(mcpConfig, null, 2))

console.log("[TEST] Mock server port:", mockServerPort)
console.log("[TEST] MCP config:", testFiles.mcpConfig)
})

suiteTeardown(async () => {
delete process.env.MCP_OAUTH_TEST_MODE

try {
await globalThis.api.cancelCurrentTask()
} catch {
// Task might not be running
}

if (mockServer) {
await new Promise<void>((resolve) => mockServer.close(() => resolve()))
}

for (const filePath of Object.values(testFiles)) {
try {
await fs.unlink(filePath)
} catch {
// ignore
}
}

// Only remove .roo/mcp.json if it's inside the ephemeral tempDir — never
// touch a real workspace's config.
const workspaceDir = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath || tempDir
if (workspaceDir === tempDir || workspaceDir.startsWith(tempDir + path.sep)) {
try {
await fs.unlink(path.join(workspaceDir, ".roo", "mcp.json"))
} catch {
// ignore
}
}

await fs.rm(tempDir, { recursive: true, force: true })
})

setup(async () => {
try {
await globalThis.api.cancelCurrentTask()
} catch {
// ignore
}
endpointsHit.clear()
await sleep(100)
})

teardown(async () => {
try {
await globalThis.api.cancelCurrentTask()
} catch {
// ignore
}
await sleep(100)
})

test("Should complete the full OAuth flow when connecting to an OAuth-protected MCP server", async function () {
// Re-write the config to trigger the file watcher and force a reconnect.
const workspaceDir = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath || tempDir
const mcpConfigPath = path.join(workspaceDir, ".roo", "mcp.json")

await fs.writeFile(
mcpConfigPath,
JSON.stringify(
{
mcpServers: {
"test-oauth-server": {
type: "streamable-http",
url: `http://localhost:${mockServerPort}/mcp`,
},
},
},
null,
2,
),
)

// Step 1: Initial connection attempt gets 401 → triggers OAuth discovery
await waitFor(() => endpointsHit.has("mcp-401"), { timeout: 30_000 })
console.log("[TEST] Got initial 401, OAuth flow started")

// Step 2: SDK discovers OAuth metadata
await waitFor(() => endpointsHit.has("resource-metadata"), { timeout: 15_000 })
console.log("[TEST] Resource metadata fetched")

await waitFor(() => endpointsHit.has("auth-metadata"), { timeout: 15_000 })
console.log("[TEST] Auth server metadata fetched")

// Step 3: Dynamic client registration
await waitFor(() => endpointsHit.has("register"), { timeout: 15_000 })
console.log("[TEST] Client registered")

// Step 4: In MCP_OAUTH_TEST_MODE the callback server resolves immediately with
// a test auth code (no real browser needed). The SDK exchanges it for a token.
await waitFor(() => endpointsHit.has("token"), { timeout: 15_000 })
console.log("[TEST] Access token obtained")

// Step 5: The background _completeOAuthFlow task retries client.connect() with
// the bearer token. Verify the MCP server receives an authenticated request.
await waitFor(() => endpointsHit.has("mcp-authed"), { timeout: 15_000 })
console.log("[TEST] MCP server connected with valid Bearer token")

// Assert the complete OAuth flow ran
assert.ok(endpointsHit.has("mcp-401"), "MCP server should return 401 to trigger OAuth")
assert.ok(endpointsHit.has("resource-metadata"), "Resource metadata discovery should run")
assert.ok(endpointsHit.has("auth-metadata"), "Auth server metadata discovery should run")
assert.ok(endpointsHit.has("register"), "Dynamic client registration should run")
assert.ok(endpointsHit.has("token"), "Token exchange should succeed")
assert.ok(endpointsHit.has("mcp-authed"), "Retry connection should succeed with Bearer token")

console.log("[TEST] MCP OAuth flow completed successfully. Endpoints hit:", [...endpointsHit])
})

test("Should reuse stored token on reconnect without re-running the full OAuth flow", async function () {
// Ensure a token is in SecretStorage before testing reuse — this makes the
// test self-contained regardless of execution order.
await waitFor(() => endpointsHit.has("token"), { timeout: 30_000 })

// Clear hit tracking so we can assert the token endpoint is NOT re-hit.
endpointsHit.clear()
Comment on lines +331 to +337
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make the token-reuse test self-contained (avoid cross-test state dependency).

This test currently depends on the previous test having already persisted a token, which makes isolated/sharded execution unreliable.

Suggested fix
 test("Should reuse stored token on reconnect without re-running the full OAuth flow", async function () {
-		// This test runs after the previous one, so a token is already stored in SecretStorage.
+		// Ensure precondition in this test: acquire and persist token first.
+		await waitFor(() => endpointsHit.has("token"), { timeout: 30_000 })
 		// Trigger another reconnect — the SDK should inject the cached token directly and skip the
 		// browser-based auth flow (no new register or token endpoints should be hit).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test("Should reuse stored token on reconnect without re-running the full OAuth flow", async function () {
// This test runs after the previous one, so a token is already stored in SecretStorage.
// Trigger another reconnect — the SDK should inject the cached token directly and skip the
// browser-based auth flow (no new register or token endpoints should be hit).
// Clear only mcp-related hit tracking (token endpoint should NOT be re-hit)
endpointsHit.clear()
test("Should reuse stored token on reconnect without re-running the full OAuth flow", async function () {
// Ensure precondition in this test: acquire and persist token first.
await waitFor(() => endpointsHit.has("token"), { timeout: 30_000 })
// Trigger another reconnect — the SDK should inject the cached token directly and skip the
// browser-based auth flow (no new register or token endpoints should be hit).
// Clear only mcp-related hit tracking (token endpoint should NOT be re-hit)
endpointsHit.clear()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/vscode-e2e/src/suite/mcp-oauth.test.ts` around lines 327 - 333, The test
"Should reuse stored token on reconnect without re-running the full OAuth flow"
must not rely on previous tests; instead, before triggering the reconnect,
create a valid mock OAuth token and persist it into the same SecretStorage key
the SDK expects (so the SDK will read it), then clear endpointsHit and proceed
with the reconnect assertions; reference the test name and the endpointsHit
variable to locate where to insert the token setup and use the same
SecretStorage API (the in-test secret storage instance) and token key the SDK
uses so the reconnect path is exercised in isolation.

Copy link
Copy Markdown
Author

@edelauna edelauna Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 05b399b — the test now calls await waitFor(() => endpointsHit.has("token"), { timeout: 30_000 }) before clearing endpointsHit, making it self-contained regardless of execution order.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rude

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai Fixed in 05b399b — the test now calls await waitFor(() => endpointsHit.has("token"), { timeout: 30_000 }) before clearing endpointsHit, making it self-contained regardless of execution order.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai Fixed in 05b399b — the test now calls await waitFor(() => endpointsHit.has("token"), { timeout: 30_000 }) before clearing endpointsHit, making it self-contained regardless of execution order.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only users with a collaborator, contributor, member, or owner role can interact with CodeRabbit.


const workspaceDir = vscode.workspace.workspaceFolders?.[0]?.uri.fsPath || tempDir
const mcpConfigPath = path.join(workspaceDir, ".roo", "mcp.json")

// Slightly modify the config to force a reconnect
await fs.writeFile(
mcpConfigPath,
JSON.stringify(
{
mcpServers: {
"test-oauth-server": {
type: "streamable-http",
url: `http://localhost:${mockServerPort}/mcp`,
// A different but valid timeout value triggers config-change detection
timeout: 30,
},
},
},
null,
2,
),
)

// Wait for the MCP server to receive an authenticated request
await waitFor(() => endpointsHit.has("mcp-authed"), { timeout: 30_000 })
console.log("[TEST] Token reuse: MCP server got authenticated request")

// The full OAuth flow should NOT have re-run (token was cached in SecretStorage)
assert.ok(endpointsHit.has("mcp-authed"), "Reconnect should use cached token")
assert.ok(!endpointsHit.has("mcp-401"), "Should not get 401 when token is cached")
assert.ok(!endpointsHit.has("register"), "Should not re-register client when token is cached")

console.log("[TEST] Token reuse test passed. Endpoints hit:", [...endpointsHit])
})
})
5 changes: 4 additions & 1 deletion src/esbuild.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,10 @@ async function main() {
copyLocales(srcDir, distDir)
setupLocaleWatcher(srcDir, distDir)
} else {
await Promise.all([extensionCtx.rebuild(), workerCtx.rebuild()])
// Run sequentially on rebuild to avoid Windows EBUSY races when both
// onEnd hooks copy the same asset directories concurrently.
await extensionCtx.rebuild()
await workerCtx.rebuild()
await Promise.all([extensionCtx.dispose(), workerCtx.dispose()])
}
}
Expand Down
Loading
Loading