feat: add OAuth 2.1 authorization for remote MCP servers#962
Conversation
- A remote MCP server can drop its transport between sends; the next tools() call then rejects and silently kills tool discovery for that server. Detect that close signal and reconnect once before retrying, so a transient drop no longer disables the server for the session. - Add isClosedConnectionError to match the @ai-sdk/mcp close signal by name/message, since MCPClientError isn't exported for instanceof. - mergeMcpTools reconnects once and retries; a failed reconnect or a second failure skips only that server so discovery never blocks send. - Provider tracks a client to serverId reverse map and exposes reconnectClient; reconnectServer is now coalesced and idempotent and re-checks serversRef after the await to drop orphaned connections. - Thread reconnectClient through the chat store and ACP adapter context.
…ingle sync owner)
…support-authenticated-remote-mcp-servers-bearer-api-key # Conflicts: # src/types/acp.ts
…emote-mcp-servers-bearer-api-key
…emote-mcp-servers-bearer-api-key
…(revert before merge)
…emote-mcp-servers-bearer-api-key
Semgrep Security ScanNo security issues found. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
PR Metrics
Updated Thu, 11 Jun 2026 19:24:18 GMT · run #1856 |
…add-oauth-21-authorization-for-mcp-servers # Conflicts: # backend/src/db/powersync-schema.ts # src/dal/index.ts # src/dal/mcp-secrets.test.ts # src/dal/mcp-secrets.ts # src/dal/mcp-servers.test.ts # src/db/powersync/schema.ts # src/lib/mcp-errors.test.ts # src/lib/mcp-errors.ts # src/lib/mcp-provider.test.tsx # src/lib/mcp-provider.tsx # src/settings/mcp-servers.test.tsx # src/settings/mcp-servers.tsx
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Preview environment deployed 🚀
Stack: Auto-destroys on PR close/merge. Login via the bundled Keycloak realm — |
|
|
||
| // OAuth creds (token expired/revoked) or no creds (OAuth-eligible by | ||
| // precedence) → re-authorize. | ||
| return { phase: 'needs-auth' } |
There was a problem hiding this comment.
Authorize shown for token-only servers
Medium Severity
The add-dialog probe uses classifyMcpServerAuth to label servers that advertise OAuth but lack DCR/CIMD as token-only, yet deriveOAuthCardDecision still maps a stored no-credential 401 to needs-auth. The MCP server card then shows Authorize even when the web OAuth flow cannot register a client and will fail.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 632b429. Configure here.
| }, 700) | ||
| return () => clearTimeout(timer) | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [newServerUrl]) |
There was a problem hiding this comment.
Auto-probe runs dialog closed
Medium Severity
The debounced URL auto-detect useEffect keys only on newServerUrl, not whether the Add dialog is open. A pending timer still calls testConnection after the dialog closes, triggering proxy MCP probes and updating add-form state in the background.
Reviewed by Cursor Bugbot for commit 78e4ee1. Configure here.
| setIsTestingConnection(false) | ||
| if (probeIdRef.current === probeId) { | ||
| setIsTestingConnection(false) | ||
| } |
There was a problem hiding this comment.
Stale probe wrong URL
High Severity
In-flight connection probes are only invalidated when a newer probe bumps probeId, not when the URL field changes. Each probe also resets testResult to idle, so resetConnectionTest skips on edit while a test runs. A slow probe can still commit success or OAuth hints for the old URL while the dialog shows a different URL, enabling Add or Add & Authorize for an untested endpoint.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit effeb3f. Configure here.
…and mobile deep-link
|
|
||
| async saveTokens(tokens: OAuthTokens): Promise<void> { | ||
| const existing = await getMcpServerCredentials(this.db, this.serverId) | ||
| const base = existing?.type === 'oauth' ? existing : { type: 'oauth' as const, access_token: '' } |
There was a problem hiding this comment.
if there's no existing oauth cred here, base is just { type: 'oauth', access_token: '' }, so we save tokens without the issuer/tokenEndpoint/clientId binding. fine today since completeMcpOAuthFlow writes the full blob directly — but if completion ever routes through the SDK's auth() path, refresh would silently break into a re-auth loop. mind carrying the binding fields here, or dropping a comment that this is a deliberate placeholder for now?
…thorize duplicate rows
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 6 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3655cc5. Configure here.
| dispatch({ type: 'set-add-authorize-pending', pending: true }) | ||
| try { | ||
| await createRow() | ||
| setOAuthState({ returnContext: '/settings/mcp-servers' }) |
There was a problem hiding this comment.
Shared OAuth return context collision
Medium Severity
MCP authorization only merges returnContext into the same oauth_flow_state store integrations use. Starting a second OAuth flow overwrites where the callback lands, so an MCP code can open Integrations (and vice versa) and neither handler completes the exchange.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 3655cc5. Configure here.
| // Always clear the navigation state so a refresh can't reprocess the callback. | ||
| deps.clearNavState() | ||
| if (!serverId) { | ||
| return |
There was a problem hiding this comment.
Clears OAuth nav without MCP flow
Medium Severity
On the MCP Servers page, any location.state.oauth payload triggers clearNavState() before checking for a pending MCP handshake. If the callback was meant for Integrations (or the handshake is missing), the code is removed from history without being exchanged.
Reviewed by Cursor Bugbot for commit 3655cc5. Configure here.
| issuer: metadata.issuer, | ||
| tokenEndpoint: metadata.token_endpoint, | ||
| scope: tokens.scope, | ||
| }) |
There was a problem hiding this comment.
Token exchange drops refresh token
Low Severity
completeMcpOAuthFlow writes refresh_token: tokens.refresh_token with no fallback when the authorization server omits a new refresh token on exchange, unlike saveTokens and ensureValidMcpOAuthToken, which keep the previous refresh token.
Reviewed by Cursor Bugbot for commit 3655cc5. Configure here.
3 participants
Note
High Risk
Introduces a full OAuth client (PKCE, DCR, callback validation, token refresh) and persists tokens on-device; mistakes in issuer pinning or state handling could weaken authorization, though the change is scoped to MCP server credentials rather than app login.
Overview
Adds OAuth 2.1 authorization for remote MCP servers alongside existing bearer tokens: on-device
mcp_secretsnow stores full OAuth token sets (access/refresh, issuer, DCRclient_id), and connections refresh tokens through the proxy before building MCP headers.A new
mcp-authstack drives discovery (RFC 9728/8414), PKCE + DCR via the MCP SDK, callback checks (CSRFstate+ RFC 9207iss), and platform-specific redirects—web full-page callback, mobile system browser + App Link, desktop Tauri loopback with inline token exchange. OAuth callbacks and deep links now forwardissfor MCP validation; backend CORS expose headers addWWW-Authenticatefor cross-origin auth challenges.The MCP Servers settings UI probes URLs (debounced), classifies 401s (rejected PAT vs “Add & Authorize” vs static token only), runs Add & Authorize with rollback on failure, and shows per-server Authorize / Re-authorize states.
MCPProvidertreats 401s as “needs auth” rather than hard failures when appropriate.Reviewed by Cursor Bugbot for commit 3655cc5. Bugbot is set up for automated code reviews on this repo. Configure here.