feat: add OAuth authentication support for streamable-http MCP servers #7297
Conversation
- Implement OAuthHandler to manage OAuth flows and token storage - Create OAuthStreamableHTTPClientTransport wrapper for OAuth-enabled connections - Add OAuth configuration support to MCP server schema - Store OAuth tokens securely in VS Code global storage - Support token refresh and re-authentication on 401 responses - Add comprehensive tests for OAuth functionality Fixes #7296
dosubot
bot
added
size:XXL
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
|
|
||
| this.transport.start = async () => { | ||
| // Replace fetch | ||
| globalThis.fetch = oauthFetch as typeof fetch |
There was a problem hiding this comment.
This is a critical issue - modifying globalThis.fetch could have unintended side effects on other parts of the application. Could we consider using a more isolated approach, perhaps by:
- Creating a custom fetch instance specifically for this transport
- Using the transport's own HTTP client instead of modifying the global one
- Implementing a proxy pattern that doesn't affect the global scope?
The current approach could break other extensions or VS Code features that rely on the standard fetch behavior.
| } | ||
| }) | ||
|
|
||
| this.server.listen(3000, "localhost", () => { |
There was a problem hiding this comment.
The hardcoded port 3000 could cause conflicts with development servers or be blocked by corporate firewalls. Could we make this configurable or implement port detection?
| try { | ||
| const data = Object.fromEntries(this.tokenStorage) | ||
| await fs.mkdir(path.dirname(this.storageFilePath), { recursive: true }) | ||
| await fs.writeFile(this.storageFilePath, JSON.stringify(data, null, 2)) |
There was a problem hiding this comment.
Storing OAuth tokens in plain JSON is a security concern. VS Code provides a SecretStorage API specifically for sensitive data. Could we refactor this to use:
oauth-
This would provide encrypted storage and better security for user credentials.
| private shouldInterceptRequest(url: string): boolean { | ||
| // This is a simplified check - you might want to make this more sophisticated | ||
| // For now, we'll intercept all requests while this transport is active | ||
| return true |
There was a problem hiding this comment.
Is this intentional? This method always returns true, meaning we intercept ALL fetch requests while the transport is active, not just those to the MCP server. This could impact performance and cause unexpected behavior. Should we check if the URL matches our MCP server URL?
| /** | ||
| * Start the OAuth flow for a server | ||
| */ | ||
| public async authenticate(serverName: string, config: OAuthConfig): Promise<OAuthTokens | null> { |
There was a problem hiding this comment.
If OAuth authentication fails, there's no way for users to retry without restarting VS Code. Could we add a command or method to clear failed authentication state and retry?
| }), | ||
| })) | ||
|
|
||
| describe("OAuthHandler", () => { |
There was a problem hiding this comment.
The test suite is missing coverage for critical error scenarios:
- What happens when the OAuth callback server fails to start?
- How does the system handle token refresh failures?
- What about testing the global fetch modification in OAuthStreamableHTTPClientTransport?
Adding these tests would help ensure the robustness of the OAuth implementation.
| vscode.window.showErrorMessage(`OAuth authentication timeout for ${serverName}`) | ||
| } | ||
| }, | ||
| 5 * 60 * 1000, |
There was a problem hiding this comment.
A 5-minute timeout might be too long from a UX perspective. Users might think the process is stuck. Could we consider:
- Reducing to 2-3 minutes
- Adding a progress notification that updates periodically
- Allowing users to cancel the authentication flow?
| // OAuth configuration (optional) | ||
| oauth: z | ||
| .object({ | ||
| clientId: z.string(), |
There was a problem hiding this comment.
The OAuth configuration is added to the schema but there's no validation of the OAuth URLs or required fields before use. Could we add validation to ensure the URLs are valid and all required OAuth fields are present when oauth is configured?
hannesrudolph
added
the
Issue/PR - Triage
|
Could you follow the MCP specification (https://modelcontextprotocol.io/specification/draft/basic/authorization)), which involves sending a request to /.well-known/oauth-protected-resource to obtain the resource metadata and authorization server URL, and then accessing the authorization server's .well-known/oauth-authorization-server to start the OAuth process? |
Summary
This PR adds native OAuth authentication support for streamable-http MCP servers, addressing Issue #7296. Previously, users had to rely on the mcp-remote package as a workaround to handle OAuth flows. This implementation enables Roo to handle OAuth authentication directly.
Changes
Core Implementation
OAuthHandler: New service to manage OAuth authentication flows
OAuthStreamableHTTPClientTransport: Wrapper for StreamableHTTPClientTransport
McpHub Updates: Enhanced to support OAuth configuration
Configuration
MCP servers can now be configured with OAuth settings:
{ "mcpServers": { "azure-secured-server": { "type": "streamable-http", "url": "http://localhost:8005/mcp", "oauth": { "clientId": "your-client-id", "clientSecret": "your-client-secret", "authorizationUrl": "https://login.microsoftonline.com/tenant/oauth2/v2.0/authorize", "tokenUrl": "https://login.microsoftonline.com/tenant/oauth2/v2.0/token", "scopes": ["User.Read", "email", "openid", "profile"] } } } }Testing
Fixes
Fixes #7296
Notes
Important
Adds OAuth authentication support for streamable-http MCP servers, including new classes for handling OAuth flows and updates to existing server configurations.
OAuthHandlerto manage OAuth 2.0 flows, token storage, and refresh inOAuthHandler.ts.OAuthStreamableHTTPClientTransportto wrapStreamableHTTPClientTransportand handle OAuth headers inOAuthStreamableHTTPClientTransport.ts.McpHub.tsto support OAuth configuration, maintaining backward compatibility with non-OAuth servers.clientId,clientSecret,authorizationUrl,tokenUrl, andscopes.OAuthHandlerin__tests__/OAuthHandler.spec.ts, covering token storage, retrieval, refresh, and authentication flows.This description was created by
for ce1000b. You can customize this summary. It will automatically update as commits are pushed.