Skip to content

feat: support OAuth 2.1 for streamable-http MCP servers#11874

Open
edelauna wants to merge 26 commits intoRooCodeInc:mainfrom
edelauna:feat/mcp-oauth-streamable-http
Open

feat: support OAuth 2.1 for streamable-http MCP servers#11874
edelauna wants to merge 26 commits intoRooCodeInc:mainfrom
edelauna:feat/mcp-oauth-streamable-http

Conversation

@edelauna
Copy link
Copy Markdown

@edelauna edelauna commented Mar 6, 2026

Related GitHub Issue

Closes: #8119

Description

Implements OAuth 2.1 support for streamable-http MCP servers per RFC 9728 (Protected Resource Metadata), RFC 8414 (Authorization Server Metadata), RFC 7591 (Dynamic Client Registration), and RFC 8707 (Resource Indicators).

Key design decisions:

  • Non-blocking OAuth: When client.connect() throws UnauthorizedError, the OAuth browser flow runs in the background via _completeOAuthFlow() so the extension (chat window, other servers) isn't blocked waiting for the user's browser session.

  • SDK workarounds: The MCP SDK's discoverOAuthMetadata() constructs the RFC 8414 well-known URL incorrectly for auth servers with path components (e.g., https://example.com/auth/public). It uses new URL("/.well-known/oauth-authorization-server", issuer) which is origin-relative and discards the path. This causes cascading failures: metadata discovery fails → dynamic client registration uses a wrong fallback URL → 404. See upstream issues:

We work around this by:

  1. Performing our own RFC 9728 + RFC 8414 discovery (utils/oauth.ts)
  2. Pre-registering the client using the correct registration_endpoint
  3. Correcting the authorization URL in redirectToAuthorization()
  4. Exchanging auth codes directly via exchangeCodeForTokens() using the correct token_endpoint

Token persistence: Tokens are stored in VS Code's SecretStorage via SecretStorageService, keyed by server host. Cached tokens are reused on reconnect without re-running the full OAuth flow.

  • Reconnect after auth: After token exchange, the stale connection is deleted and connectToServer() is called fresh — the SDK's transport can't be reused after UnauthorizedError because its internal _abortController is already set.

New files:

  • McpOAuthClientProvider.ts — Implements the SDK's OAuthClientProvider interface
  • SecretStorageService.ts — Thin wrapper around VS Code SecretStorage for OAuth tokens
  • utils/oauth.ts — RFC 8414-compliant metadata discovery (replaces SDK's broken implementation)
  • utils/callbackServer.ts — Local HTTP server for OAuth redirect callback with CSRF state validation

Test Procedure

Unit tests (42 tests across 4 files):
npx vitest run services/mcp/tests/McpOAuthClientProvider.spec.ts services/mcp/tests/SecretStorageService.spec.ts services/mcp/utils/tests/oauth.spec.ts services/mcp/utils/tests/callbackServer.spec.ts

  • McpOAuthClientProvider.spec.ts — 25 tests covering create, clientMetadata, token storage/expiry, PKCE, redirect, auth code, close
  • SecretStorageService.spec.ts — 7 tests covering CRUD, key isolation, malformed data
  • oauth.spec.ts — 8 tests covering RFC 8414 URL construction for path/no-path issuers, error handling
  • callbackServer.spec.ts — 2 tests covering callback handling and CSRF state validation

E2e test (apps/vscode-e2e/src/suite/mcp-oauth.test.ts):

  • Full OAuth flow test: mock server returns 401 → resource metadata → auth server metadata → dynamic registration → token exchange → authenticated MCP connection
  • Token reuse test: reconnect uses cached token without re-running OAuth

Manual testing:

		"temporal-kapa": {
			"type": "streamable-http",
			"url": "https://temporal.mcp.kapa.ai"
		},
		"Claude Code (figma)" : { // <------ seems like figma hardcodes specific client name
			"type": "streamable-http",
			"url": "https://mcp.figma.com/mcp"
		}
image

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue.
  • Scope: My changes are focused on the linked issue.
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes.
  • Documentation Impact: I have considered if my changes require documentation updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required.

Additional Notes

  • The SDK workarounds can be removed once the upstream issues are resolved (primarily typescript-sdk#545).
  • MCP_OAUTH_TEST_MODE=true env var enables headless e2e testing by making the callback server resolve immediately with a test auth code.
  • The client_name in OAuth client registration uses the MCP server config key (e.g., "figma") so providers see a meaningful name.

Interactively review PR in Roo Code Cloud

Comment thread src/services/mcp/utils/callbackServer.ts Fixed
@edelauna edelauna changed the title Feat/mcp oauth streamable http feat: support OAuth 2.1 for streamable-http MCP servers Mar 6, 2026
@edelauna edelauna marked this pull request as ready for review March 6, 2026 14:45
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. Enhancement New feature or request labels Mar 6, 2026
@edelauna
Copy link
Copy Markdown
Author

edelauna commented Mar 7, 2026

Adding Screenshares of the sign-in and usage process.

MCP OAuth Flow:

oauth-connect-example-compressed.mp4

MCP Usage:

mcp-usage-example.mp4

@edelauna edelauna force-pushed the feat/mcp-oauth-streamable-http branch 2 times, most recently from 4c87637 to 653b71e Compare March 12, 2026 20:18
@hannesrudolph
Copy link
Copy Markdown
Collaborator

@roomote is this going to break anything?

@roomote-v0
Copy link
Copy Markdown
Contributor

roomote-v0 Bot commented Mar 19, 2026

Proofessor Clock   See task

I reviewed the PR for breaking changes. All 104 tests pass (57 new OAuth + 47 existing McpHub) and TypeScript compiles cleanly.

Will it break existing functionality? No. The OAuth code path is only entered for streamable-http connections; stdio and sse servers are completely unaffected. The ConnectedMcpConnection type gains only an optional authProvider field. The callTool() retry logic is gated on connection.authProvider being defined, so non-OAuth servers just re-throw errors as before.

Things worth being aware of (not breaking):

  1. Small latency addition for all streamable-http servers: McpOAuthClientProvider.create() calls fetchOAuthAuthServerMetadata() which makes one HTTP request to /.well-known/oauth-protected-resource for every streamable-http connection -- even non-OAuth ones. It 404s gracefully (returns null), but adds a network round-trip to connection setup.

  2. Restart clears OAuth tokens: restartConnection() calls deleteOAuthData() for streamable-http servers, forcing users to re-authenticate through the browser on every manual restart. This is intentional but could be surprising.

  3. MCP_OAUTH_TEST_MODE env var: The callback server has a test-mode escape hatch (process.env.MCP_OAUTH_TEST_MODE === "true") that bypasses the real OAuth browser flow. Low risk since it's an obscure name, but worth documenting that it should never be set in production.

  4. Local HTTP callback server: Binds to 127.0.0.1 on a random port for the OAuth redirect. The server is started lazily (only when OAuth is actually needed) and has proper cleanup. In heavily restricted environments, binding a local port could theoretically fail.

Copy link
Copy Markdown
Collaborator

@hannesrudolph hannesrudolph left a comment

Choose a reason for hiding this comment

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

Architecturally sound PR. The OAuth 2.1 implementation correctly handles RFC 9728 + RFC 8414 discovery, works around the SDK's broken well-known URL construction for path-prefixed issuers, and the non-blocking design (detaching the browser flow from waitUntilReady()) is well done. Token lifecycle, multi-window coordination via SecretStorage.onDidChange, and the reconnect-from-scratch strategy (required because the SDK's transport can't be reused after UnauthorizedError) are all correct. Test coverage is thorough (42 unit + 2 e2e). One P1 UX concern on mid-session re-auth and four P2 items inline.

Comment thread src/services/mcp/McpHub.ts
Comment thread src/services/mcp/utils/callbackServer.ts
Comment thread src/services/mcp/McpHub.ts
Comment thread src/services/mcp/McpHub.ts
Comment thread src/services/mcp/utils/callbackServer.ts
@hannesrudolph
Copy link
Copy Markdown
Collaborator

@roomote do a full review please and leave inline comments.

@roomote-v0
Copy link
Copy Markdown
Contributor

roomote-v0 Bot commented Mar 19, 2026

Fixaroo Clock   See task

Completed full review with 8 inline comments and pushed security hardening fixes. All 57 unit tests pass, lint and type checks clean.

View commit | Revert commit

Copy link
Copy Markdown
Contributor

@roomote-v0 roomote-v0 Bot left a comment

Choose a reason for hiding this comment

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

Full Review: OAuth 2.1 for streamable-http MCP servers

Overall this is a solid implementation. The SDK workarounds for broken RFC 8414 discovery are well-documented, the non-blocking OAuth flow design is thoughtful, and test coverage is comprehensive (57 tests across 4 files).

I've pushed a commit with security hardening fixes (see inline comments for details). The remaining comments below are observations and suggestions for the PR author to consider.

What works well

  • Clean separation of concerns: McpOAuthClientProvider, SecretStorageService, callbackServer, oauth.ts
  • Lazy callback server startup avoids binding ports unnecessarily
  • Token refresh with deduplication (_refreshPromise) prevents thundering herd
  • Multi-window awareness via SecretStorage.onDidChange watcher is a nice touch
  • The _completeOAuthFlow fire-and-forget pattern keeps the extension responsive

Items addressed in my commit (bde2bc3)

  1. CSRF state token increased from 8 to 16 bytes (128-bit entropy per OAuth 2.1)
  2. Content-Security-Policy header added to callback HTML response
  3. Callback server now closes on CSRF state validation failure
  4. stopCallbackServer handles already-closed servers gracefully
  5. MCP_OAUTH_TEST_MODE guard tightened with VSCODE_PID check

Additional observations (not blocking)

See inline comments.

Comment thread src/services/mcp/McpHub.ts
Comment thread src/services/mcp/McpHub.ts
Comment thread src/services/mcp/McpHub.ts Outdated
Comment thread src/services/mcp/McpOAuthClientProvider.ts Outdated
Comment thread src/services/mcp/McpOAuthClientProvider.ts Outdated
Comment thread src/services/mcp/utils/callbackServer.ts Outdated
Comment thread src/services/mcp/utils/oauth.ts
Comment thread src/services/mcp/McpHub.ts Outdated
@edelauna edelauna force-pushed the feat/mcp-oauth-streamable-http branch from 3d0eb27 to 11798c2 Compare March 25, 2026 00:35
Comment thread src/services/mcp/utils/callbackServer.ts Fixed
Comment thread src/services/mcp/utils/callbackServer.ts Fixed
Comment thread src/services/mcp/utils/callbackServer.ts Fixed
@edelauna edelauna force-pushed the feat/mcp-oauth-streamable-http branch from 11798c2 to 5aeed4f Compare March 27, 2026 02:37
@hannesrudolph
Copy link
Copy Markdown
Collaborator

@roomote is this good to go?

@roomote-v0
Copy link
Copy Markdown
Contributor

roomote-v0 Bot commented Apr 22, 2026

Fixaroo Clock   See task

I've reviewed the PR and run all checks. Here's the summary:

Tests: All 69 unit tests pass across 4 test files (McpOAuthClientProvider.spec.ts, SecretStorageService.spec.ts, oauth.spec.ts, callbackServer.spec.ts).

Type checking: tsc --noEmit passes cleanly with no errors.

i18n: All 18 locale files have consistent OAuth callback keys.

Code review findings -- the implementation looks solid:

  • RFC compliance is well-handled: RFC 9728 (Protected Resource Metadata), RFC 8414 (Authorization Server Metadata), RFC 7591 (Dynamic Client Registration), and RFC 8707 (Resource Indicators) are all implemented correctly.
  • The SDK workaround for broken well-known URL construction is well-documented and properly scoped.
  • Non-blocking OAuth flow design is clean -- _initiateOAuthFlow() shows a confirmation toast before opening the browser, and _watchForOAuthTokens() handles multi-window scenarios via SecretStorage.onDidChange.
  • Token refresh with deduplication (_refreshPromise) and proper cleanup on failure.
  • Lazy callback server startup via _ensureCallbackServer() avoids wasting ports for non-OAuth servers.
  • Negative cache (_nonOAuthCache) with TTL prevents repeated discovery probes on non-OAuth servers.
  • XSS protection in the callback HTML -- error messages use safe i18n strings rather than reflected query params.
  • CSRF state validation on the callback endpoint.

No blocking issues found. The PR is ready for merge from a code quality and test coverage perspective.

@edelauna edelauna force-pushed the feat/mcp-oauth-streamable-http branch 2 times, most recently from 83dbb12 to fda3e98 Compare April 24, 2026 03:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENHANCEMENT] Support MCP OAuth 2.1 for HTTP MCP servers (discovery + PKCE)

3 participants