Conversation
Implements the MCP authorization spec (2025-06-18) so users can connect custom MCP servers that require OAuth, not just static API keys. When a user adds an MCP server URL we probe it; on a 401 with WWW-Authenticate we walk RFC 9728 (protected-resource metadata) and RFC 8414 (AS metadata), do RFC 7591 dynamic client registration when supported, then run an authorization-code flow with PKCE-S256 and the RFC 8707 resource indicator. Tokens are persisted in localStorage alongside the existing custom-server data and travel with chat requests via the existing headers channel. We refresh expiring tokens just-in-time before each chat send, distinguish AS-side rejections (invalid_grant) from transport blips so a flaky network does not log the user out, and surface a "Reconnect" path on hard failure. UI: AuthorizeStep flow inside the MCP modal handles popup vs full-page redirect (mobile / popup-blocked) and a manual client-id form for AS without DCR. ServerCard shows an Authorized pill with live expiry countdown and a Disconnect button that best-effort-revokes via RFC 7009.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af82d6a1a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let { discovery, serverUrl, serverId, onauthorized, oncancel }: Props = $props(); | ||
|
|
||
| let phase = $state<"idle" | "starting" | "popup" | "manual" | "error" | "done">( | ||
| untrack(() => (discovery.clientInfo ? "idle" : "manual")) |
There was a problem hiding this comment.
When a DCR-less server is added, MCPServerManager.handleAddServer persists a placeholder clientInfo with client_id: ""; on re-authorize, AuthorizeStep currently checks only discovery.clientInfo truthiness, so this placeholder is treated as valid and the manual Client ID form is skipped. The next /api/mcp/oauth/start call then fails with missing client_id, leaving users unable to complete authorization after canceling the first attempt (and similarly after redirect-based fallback paths that don't rewrite client info). This blocks OAuth onboarding for manual-client servers.
Useful? React with 👍 / 👎.
1 participant
Summary
WWW-Authenticate→ RFC 9728 protected-resource metadata → RFC 8414 AS metadata → RFC 7591 DCR (with manual client-id fallback for AS without DCR) → OAuth 2.1 authorization-code with PKCE-S256 + RFC 8707 resource indicator.localStoragealongside the existing custom-server data and travel with chat requests via the existing headers channel; refresh is just-in-time before each chat send and distinguishes AS-side rejections (invalid_grant) from transport blips so a flaky network doesn't silently log users out.AuthorizeStep(popup with full-page-redirect fallback for mobile / popup-blocked) andServerCard"Authorized · expires in …" pill with a live countdown and best-effort RFC 7009 revoke on Disconnect.Files
MCPOAuthState,MCPAuthorizationServerMetadata,MCPClientInformation,MCPOAuthTokensonMCPServer.src/lib/server/mcp/oauth/{canonical,discover,exchange,state}.tsand routes/api/mcp/oauth/{discover,start,callback,refresh,revoke}/+server.ts.src/lib/utils/mcpOAuth.ts(popup orchestration, refresh, revoke, redirect handoff) andsrc/lib/components/mcp/AuthorizeStep.svelte.mcpServers.tsstore (oauth state mutators, JIT refresh),AddServerForm,MCPServerManager,ServerCard, conversation+page.svelte.Security
</>/&/</script>breakout in the inline postMessage script; CSP locked todefault-src 'none'; script-src 'unsafe-inline'.postMessage; redirect URI exact-match (<origin>/api/mcp/oauth/callback); SSRF-safe discovery viassrfSafeFetch+isValidUrl; HTTPS-only.Test plan
npm run check— 0 errorsnpm run lint— cleannpm run test— 173 tests passing (added 12 new for canonical-URI and flow-cookie sign/verify)