Inspector client: Added proxy fetch for use by auth (to avoid CORS issues)#1047
Inspector client: Added proxy fetch for use by auth (to avoid CORS issues)#1047BobDickinson wants to merge 8 commits intomodelcontextprotocol:mainfrom
Conversation
…n using browser fetch)
|
PR closed by accident when client branch inadvertently cleaned up. |
Connects in Direct Mode / Fails in Via ProxyFAIL-web-client-oauth-proxy-fetch.movResourceProtected Resource Metadata |
|
@BobDickinson also a some auth-related tests are failing ... |
…en because the TLS verification changed. Without pinned deps (package lock) it's hard to track down the dep change actually caused this - fix was to use http instead of https example.com test urls.
…which should fix the failures reported with proxy auth. I have not yet verified that locally (pending repro details).
… on error string text matching to detect an OAuth 401)
Proxy mode OAuth: example-server vs GuildbridgeThe problem where Issue: In proxy mode, OAuth recovery ran for example-server but not Guildbridge, even though both return a normal 401 with TL;DRBefore the fix, two things broke proxy OAuth relative to direct mode:
After the fix: the proxy forwards 401 headers/body on the HTTP response where it can, puts Unauthenticated
|
| Layer | Change |
|---|---|
| Proxy → browser (HTTP) | On upstream 401, forward WWW-Authenticate and raw body (see server/src/index.ts createCustomFetch / sendProxiedUnauthorized). Expose WWW-Authenticate for CORS where needed. |
| Proxy → browser (JSON-RPC) | On failed forward after a session exists, server/src/mcpProxy.ts returns JSON-RPC code -32099 with serializable error.data including upstream401 and/or httpStatus when known. |
| Client | isConnectionAuthError (client/src/lib/connectionAuthErrors.ts) treats SseError / StreamableHTTPError 401, UnauthorizedError, and McpError with -32099 plus auth-shaped data—no dependency on a single vendor’s error_description string. |
Hook-level tests live in client/src/lib/hooks/__tests__/useConnection.test.tsx under “Inspector proxy McpError auth recovery”.
Takeaway
- Do not infer missing OAuth from curl: both hosts return a Bearer challenge with
resource_metadataandinvalid_tokenJSON. - Do treat proxy fidelity (HTTP headers/body vs JSON-RPC
data) and structured client checks as the contract for “run OAuth recovery,” not message substrings tied to one server’s wording.
|
Via Proxy is working... via-proxy-working.mov |
|
@claude review |
|
Claude finished @cliffhall's task in 2m 17s —— View job PR Review: Inspector client proxy fetch for OAuth/CORSTasks:
OverviewThis PR fixes a real, well-documented problem: OAuth auth flows in proxy mode were broken by CORS restrictions and fragile string-matching in A few issues are worth addressing before merge, ranging from a functional memoization bug to a security concern. Issues1.
|
There are many types of failures in the auth process due to CORS when authenticating from a browser environment, including response header stripping, failure to access server or auth server endpoints (including, but not limited to, server metadata endpoints), failure in token exchange, and I'm sure others.
Issue #995 and the associated PR #996 raised a similar (maybe the same?) issue.
Type of Change
Changes Made
This PR takes a different approach than #996, which is that we remote only the fetch function to the proxy server (so it can run from a Node environment instead of the browser). When in "proxy" mode we use a client side proxyFetch function, which we pass to all auth functions (which were designed for this exact situation, such that they all take an optional fetchFn param). This prevents CORS issue from impacting auth in any way, while still relying on the auth SDK functions for all auth logic.
This change is fairly lightweight (< 100 lines of non-test code).
Related Issues
This PR is related to #995 and the associated PR #996, but takes a different approach. I am interested to learn whether this approach also solves the issues targeted there (I have tested my use cases, but not the use cases from the issue/PR or mentioned in the comments there).
Testing
Test Results and/or Instructions
Tested with:
https://example-server.modelcontextprotocol.io/mcp- Works, as beforehttps://api.githubcopilot.com/mcp/- Previously failed discovery due to CORS, now fails because DCR not supported, but demonstrates that this fix worksChecklist
npm run prettier-fix)