Fix: OAuth CORS errors when Connection Type is 'Via Proxy'

[asoorm]

Problem

When users select "Via Proxy" as the Connection Type, OAuth authentication fails with CORS errors because OAuth operations bypass the proxy and execute directly from the browser.

Fixes #995

Solution

Route all OAuth operations through the Express proxy server when connectionType is "proxy", following the same pattern as MCP server connections.

Changes

Server-Side (server/src/index.ts)

Added 4 OAuth proxy endpoints:

• GET /oauth/metadata - Authorization server metadata discovery
• GET /oauth/resource-metadata - Protected resource metadata discovery
• POST /oauth/register - Dynamic Client Registration (DCR)
• POST /oauth/token - Token exchange

Client-Side

• client/src/lib/oauth-proxy.ts (new) - Proxy utility functions
• client/src/lib/oauth-state-machine.ts - Conditional routing based on connectionType
• client/src/components/AuthDebugger.tsx - Accepts connectionType/config props
• client/src/App.tsx - Provides connectionType and config
• client/src/lib/auth.ts - Fix scope to be optional per RFC 7591

Testing

Tested successfully with Keycloak OAuth server. All OAuth operations work without CORS errors in proxy mode. Direct mode still functions for servers with proper CORS configuration.

Backward Compatibility

No breaking changes. Direct connections work as before. Proxy mode is opt-in via existing "Connection Type" selector.

Next Steps

Based on your feedback to this PR, I will add polish, tests, and documentation accordingly.

[asoorm]

@cliffhall PTAL 🙏

[claude]

Claude encountered an error —— View job

Command failed: git fetch origin --depth=20 fix/oauth-cors-proxy-support

---

I'll analyze this and get back to you.

[anthonychu]

Just want to chime in and say I pulled down the branch and it worked. Immediately clicking "Connect" fails (I think this is a separate issue), but clicking "Quick OAuth Flow" successfully registers the client via DCR and authenticates the user. After that, the token is acquired and "Connect" now works, and I can interact with the MCP server normally.

@asoorm Thanks for finding this and submitting the fix. Look forward to seeing it merged.

[cliffhall]

@claude /review

[claude]

Claude finished @cliffhall's task in 3m 5s —— View job

---

PR Review: Fix OAuth CORS errors when Connection Type is 'Via Proxy'

Tasks

• Fetch PR details and diff
• Read all changed files
• Analyze code quality and correctness
• Check security considerations
• Evaluate test coverage needs
• Provide comprehensive review feedback

---

Summary

This PR successfully addresses OAuth CORS issues when using proxy connection type by routing all OAuth operations through the Express proxy server. The implementation is well-structured and follows existing patterns. The code has been tested successfully with Keycloak (confirmed by @anthonychu). However, there are several areas that need attention before merging.

Review Details

✅ Strengths

1. Consistent Architecture: The implementation mirrors the existing MCP proxy pattern, maintaining consistency across the codebase
2. Backward Compatible: Direct connections continue working as before; proxy mode is opt-in
3. Good Separation of Concerns: New oauth-proxy.ts utility cleanly encapsulates proxy-specific logic
4. RFC Compliance Fix: Making scope optional in client metadata (RFC 7591) is a good catch
5. Clear Documentation: Inline comments and commit messages explain the changes well
6. Real-World Testing: Successfully tested with Keycloak OAuth server

🔴 Critical Issues

1. Server-Side Input Validation Issues (Security)

Location: server/src/index.ts lines 803-808, 853-858, 902-909, 954-960

The server endpoints only check if parameters exist but don't validate their format or content:

// Current - only checks existence
const authServerUrl = req.query.authServerUrl as string;
if (!authServerUrl) {
  res.status(400).json({ error: "authServerUrl query parameter is required" });
  return;
}

Issues:

• No URL validation - could allow SSRF (Server-Side Request Forgery) attacks
• No sanitization of user input before making fetch requests
• Attacker could potentially make the proxy server request internal resources

Recommendation: Add URL validation to ensure only HTTP/HTTPS URLs are accepted and optionally allowlist domains:

const authServerUrl = req.query.authServerUrl as string;
if (!authServerUrl) {
  res.status(400).json({ error: "authServerUrl query parameter is required" });
  return;
}

try {
  const url = new URL(authServerUrl);
  if (!['http:', 'https:'].includes(url.protocol)) {
    res.status(400).json({ error: "Only HTTP/HTTPS URLs are allowed" });
    return;
  }
  // Optional: Add domain allowlist check here
} catch (e) {
  res.status(400).json({ error: "Invalid URL format" });
  return;
}

Fix URL validation →

2. Incorrect express.json() Placement (Bug)

Location: server/src/index.ts line 791

// OAuth Proxy Endpoints - for routing OAuth requests through the proxy to avoid CORS issues
app.use(express.json()); // Ensure JSON body parsing is enabled

Issue: This middleware is placed after the /config endpoint and only applies to routes defined after it. If JSON parsing wasn't already enabled globally, this could cause issues with request body parsing for routes defined before this line.

Recommendation: Move app.use(express.json()) to the top of the file where other global middleware is configured, or verify it's already configured globally and remove this duplicate.

Fix middleware placement →

3. Missing Error Types/Codes (Usability)

Location: All proxy functions in client/src/lib/oauth-proxy.ts

The error handling is basic and doesn't distinguish between different types of failures:

if (!response.ok) {
  const error = await response.json().catch(() => ({ error: response.statusText }));
  throw new Error(`Failed to discover OAuth metadata: ${error.error || response.statusText}`);
}

Issue: Users get generic error messages that don't help them diagnose network issues, authentication failures, or CORS problems.

Recommendation: Add more specific error handling:

• Network errors (fetch failures)
• 401/403 (authentication/authorization)
• 404 (endpoint not found)
• 5xx (server errors)
• Include original error details for debugging

⚠️ Important Issues

4. No Tests for New Functionality (Testing)

Missing Coverage:

• No unit tests for oauth-proxy.ts functions
• No integration tests for server-side OAuth proxy endpoints
• Existing AuthDebugger.test.tsx needs updates to test proxy mode

Recommendation: Add comprehensive tests:

// Example test structure needed
describe('oauth-proxy', () => {
  describe('discoverAuthorizationServerMetadataViaProxy', () => {
    it('should successfully fetch metadata through proxy', async () => { ... });
    it('should handle network errors', async () => { ... });
    it('should handle invalid responses', async () => { ... });
  });
  // ... similar for other functions
});

The existing AuthDebugger.test.tsx needs to:

• Mock the proxy utility functions
• Add test cases for both connectionType: "direct" and connectionType: "proxy"
• Verify the correct code path is taken based on connectionType

Add tests for OAuth proxy →

5. Type Safety Concerns (TypeScript)

Location: server/src/index.ts lines 966, 902

const formBody = new URLSearchParams(params as Record<string, string>);
const { registrationEndpoint, clientMetadata } = req.body;

Issues:

• req.body is untyped - could have unexpected shape
• Type assertion without runtime validation
• No validation of clientMetadata structure before forwarding

Recommendation: Add runtime type validation using a schema validator (Zod, Joi, etc.) or at minimum, check expected properties exist.

6. Inconsistent Error Handling Between Direct and Proxy Paths (Consistency)

Location: client/src/lib/oauth-state-machine.ts

The direct and proxy code paths don't handle errors consistently. For example:

• Direct path uses SDK functions which may have their own error handling
• Proxy path has custom error handling in each function
• Errors from proxy vs direct may have different shapes/messages

Recommendation: Wrap both paths in consistent try-catch blocks and normalize error messages.

💡 Suggestions (Non-blocking)

7. Code Duplication in oauth-proxy.ts (DRY Principle)

All four proxy functions have nearly identical fetch logic and error handling. Consider extracting a common helper:

async function proxyFetch<T>(
  endpoint: string,
  method: 'GET' | 'POST',
  config: InspectorConfig,
  body?: object,
  queryParams?: Record<string, string>
): Promise<T> {
  const proxyAddress = getMCPProxyAddress(config);
  const url = new URL(endpoint, proxyAddress);
  
  if (queryParams) {
    Object.entries(queryParams).forEach(([key, value]) => {
      url.searchParams.set(key, value);
    });
  }

  const response = await fetch(url.toString(), {
    method,
    headers: getProxyHeaders(config),
    ...(body && { body: JSON.stringify(body) }),
  });

  if (!response.ok) {
    const error = await response.json().catch(() => ({ error: response.statusText }));
    throw new Error(`Request failed: ${error.error || response.statusText}`);
  }

  return await response.json();
}

8. button.tsx Unrelated Change (Scope Creep)

Location: client/src/components/ui/button.tsx lines 37-38

This formatting change appears unrelated to the PR's purpose. While it's a valid prettier fix, consider removing it to keep the PR focused, or mention it in the PR description.

9. Logging Could Be More Informative (Observability)

Location: Server endpoints in server/src/index.ts

Console.log statements are basic. Consider:

• Adding request IDs for tracing
• Logging response status codes
• Using a proper logger (Winston, Pino)
• Adding more context (timestamp, request source, etc.)

10. Missing JSDoc for Public Functions (Documentation)

Location: client/src/lib/oauth-proxy.ts

While functions have basic comments, JSDoc comments would provide better IDE support:

/**
 * Discover OAuth Authorization Server Metadata via proxy
 * @param authServerUrl - The OAuth authorization server URL
 * @param config - Inspector configuration containing proxy settings
 * @returns Promise resolving to OAuth metadata
 * @throws Error if metadata discovery fails
 */
export async function discoverAuthorizationServerMetadataViaProxy(
  authServerUrl: URL,
  config: InspectorConfig,
): Promise<OAuthMetadata> {
  // ...
}

11. Consider Request Timeout Configuration (Robustness)

The fetch calls in both client and server have no timeout configuration. If an OAuth server is slow or unresponsive, requests could hang indefinitely.

Recommendation: Add timeout configuration:

const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 30000); // 30s timeout

try {
  const response = await fetch(url, {
    signal: controller.signal,
    // ... other options
  });
  // ...
} finally {
  clearTimeout(timeoutId);
}

📋 Testing Recommendations

1. Unit Tests:

   • All oauth-proxy.ts functions with various response scenarios
   • Server endpoint handlers with different input combinations
   • Error handling paths

2. Integration Tests:

   • Full OAuth flow in proxy mode
   • Full OAuth flow in direct mode
   • Switching between proxy and direct modes

3. E2E Tests:

   • Complete OAuth flow with a mock OAuth server
   • Both connection types
   • Error scenarios (network failures, invalid responses)

4. Security Tests:

   • SSRF prevention validation
   • Input sanitization
   • Header injection attempts

🏁 Conclusion

This is a solid foundation that addresses a real user pain point. The architecture is sound and follows existing patterns well. However, the security concerns around input validation are critical and must be addressed before merging.

Before Merging:

1. ✅ Must Fix (Blocking):

   • Add URL validation to prevent SSRF (Issue Progress notifications #1)
   • Fix express.json() placement (Issue Request logging #2)
   • Add basic tests for proxy functionality (Issue SSE transport support #4)

2. 🔶 Should Fix (Strongly Recommended):

   • Add runtime type validation (Issue OAuth support #5)
   • Improve error handling and messages (Issue Completion requests #3)
   • Make error handling consistent (Issue add readme #6)

3. 💭 Nice to Have:

   • Refactor to reduce duplication (Suggestion Tool use #7)
   • Add JSDoc comments (Suggestion make prompts pass args #10)
   • Add request timeouts (Suggestion get tools working #11)

Next Steps

Based on the PR description stating "I will add polish, tests, and documentation accordingly," I recommend:

1. Address the security issues immediately
2. Add test coverage
3. Consider the other suggestions as polish items

Great work on identifying and solving this CORS issue! With these improvements, this will be a valuable addition to the codebase.

---

[cliffhall]

Immediately clicking "Connect" fails (I think this is a separate issue),

@anthonychu verified this.

• If I run the latest inspector (0.18.0) against an OAuth protected server (e.g., https://example-server.modelcontextprotocol.io/mcp), the connect button does nothing.
• If I run npx @modelcontextprotocol/inspector@0.17.0 against the same server, hitting connect redirects me to authenticate.
• All versions after 0.17.0 are broken in this respect.

[asoorm]

@cliffhall please take another look - I've implemented recommendations from Claude's review.

SSRF validation is intentionally not implemented in the OAuth proxy endpoints.
This follows the same pattern as the existing MCP proxy implementation (see
SSE and StreamableHTTP transports in createTransport(), lines 388-414), which
also accepts user-provided URLs without validation.

Rationale:

• MCP Inspector is a local developer tool running on localhost:6277
• Users are trusted developers testing their own servers
• The proxy's purpose is transparent request forwarding
• Existing MCP proxy endpoints use the same approach

If SSRF protection is deemed necessary for MCP Inspector's threat model, I
think this should be addressed holistically across all proxy endpoints (MCP and OAuth)
in a separate security-focused PR, rather than inconsistently in just the
OAuth endpoints.