feat(desktop): add MCP OAuth support for remote servers by shylock-wu · Pull Request #556 · vastsa/PI-Desktop

shylock-wu · 2026-09-17T16:55:11Z

feat(desktop): add MCP OAuth support for remote servers

This PR implements full OAuth authorization capabilities for remote MCP servers (such as Notion, remote API proxies, etc.), enabling secure authentication and token handling.

Key Changes

OAuth Discovery: Implemented McpOAuthManager with RFC 9728 resource metadata discovery and RFC 8414 OAuth 2.0 authorization server metadata discovery.
Dynamic Registration & PKCE: Support RFC 7591 Dynamic Client Registration (DCR) with fallback to manual credentials, and RFC 7636 PKCE (S256) authorization code flow.
Loopback Exchange: Built a local ephemeral HTTP server to securely capture the loopback authorization callback.
Token Management: Encrypted token persistence via safeStorage (with filesystem fallback) and automated proactive refresh for near-expiry tokens.
Runtime Integration: Hooked into UserMcpRuntime to transparently inject Bearer tokens into outgoing HTTP headers and detect 401 Unauthorized responses to prompt re-authentication.
Settings UI & i18n: Added OAuth connection status, connect/disconnect triggers in AgentMcpPage, and updated translations for all 8 supported locales (en, zh-CN, zh-TW, de, es, fr, ko, tr).

Validation

Unit & Integration Tests: 5/5 passing in apps/desktop/test/mcp-oauth.test.mjs
- Discovery across RFC 9728 & RFC 8414
- Full authorization flow (DCR + PKCE + loopback exchange)
- Proactive token refresh via refresh_token
- Token deletion & status verification
- UserMcpRuntime Bearer injection and 401 interception

Scope

Backend: apps/desktop/electron/main/mcp-oauth.ts, user-mcp.ts, ipc/mcp-ipc.ts
Frontend: apps/desktop/src/components/settings/AgentMcpPage.tsx
Locales: packages/i18n/src/locales/*
Shared: packages/shared/src/*

vastsa

Thanks for taking this on — remote HTTP MCP OAuth is a real gap (Notion etc.), and a few of the bones are right: PKCE S256, loopback bound to 127.0.0.1, tokens in host-core secrets.* under secret:mcp:<id>:oauth (not the renderer), and the 8 locales.

This is not mergeable yet. The description overclaims vs the code (safeStorage / filesystem fallback, manual-credential DCR fallback, 401 re-auth during tool calls). Please follow the existing VendorOAuth pattern (start + event + cancel, secrets stay in main) instead of blocking IPC for the whole browser dance.

Blockers

Success toast is dead. mcp/oauth/start returns { status } only; the UI gates on result.ok && status.state === "ready". ok is always undefined, so a successful login still toasts authFailed.
Callback HTML XSS. error / error_description are interpolated into HTML with no escaping. /callback?error=x&error_description=<script>… does not need a valid state. That page opens in the system browser.
Missing RFC 8707 resource. MCP Authorization (2025-06-18) requires resource on both the authorize URL and the token request. Notion-class servers will reject the token. This is the advertised main path.
redirect_uri is http://localhost:<port>/callback while the server listens on 127.0.0.1. If the browser resolves localhost to ::1, the callback never arrives. RFC 8252 / MCP want http://127.0.0.1:<port>/callback.
Bearer is applied only in createEntry. existing ?? this.createEntry(record, oauthToken) ignores a refreshed token on a live client. configurationChanged does not see injected OAuth headers, so setRecords will not rebuild. tools/call 401 is not mapped to authRequired.
IPC blocks for up to 5 minutes, no cancel. Vendor OAuth returns a loginId immediately and streams events. There is no mcp/oauth/cancel, no progress event, and a renderer reload leaves the loopback server up until timeout.
Public IPC + new secret shape + loopback HTTP in main = architecture/security change. Needs an ADR, docs/spec/03-runtime/01-ipc-protocol.md (mcp/oauth/start, McpServerStatus.hasOauth / authRequired), and at least one E2E scenario. None of that is in this PR.

Also fix

No DCR → hardcoded client_id = "pi-desktop". Description promised a manual-credential fallback; there is no UI. Every startLogin re-registers instead of reusing the stored client.
Discovery / token fetch follows redirects; MCP HTTP (ADR 0142) uses redirect: "manual".
expires_in only accepted as a number; many AS return "3600" → never refreshes.
mcp.remove deletes the secret; mcp.transfer that renames the id orphans it.
Loopback page is hardcoded Chinese; the row badge is hardcoded "OAuth".
Token refresh is not serialized (VendorOAuth serializes modify per row). Rotating refresh tokens will race.
Tests are happy-path only. Please add: invalid callback / XSS, resource, 127.0.0.1 redirect, string expires_in, live-connection token rebuild, transfer, IPC ok, cancel/timeout.

Happy to re-review once those land.

vastsa · 2026-09-17T17:12:16Z

+        ...current.filter((status) => status.serverId !== server.id),
+        result.status,
+      ]);
+      if (result.ok && result.status.state === "ready") {


This can never succeed. mcp/oauth/start returns { status } with no ok (see mcp-ipc.ts). After a good login the UI still takes the else branch and toasts extensions.mcp.authFailed.

Either return { ok: true, status } from the handler, or drop the result.ok check and key off status.state.

vastsa · 2026-09-17T17:12:16Z

+        throw new Error(`MCP server ${payload.id} is not an HTTP transport server`);
+      }
+
+      await oauth.startLogin(server.id, server.url);


This invoke waits for the entire browser flow (up to 5 minutes). Vendor OAuth already has the right shape: start returns immediately with a loginId, progress goes out on an event channel, and cancel aborts the loopback server.

Please match that. As written there is no cancel IPC, a settings unmount cannot abort, and a renderer reload leaks the ephemeral HTTP server until timeout/disposeAll.

vastsa · 2026-09-17T17:12:16Z

+      const status = await userMcp.test(payload.id);
+      await refreshUserMcp(currentWorkspacePath());
+      sendToRenderer(IPC.event.pluginChanged, { reason: "mcp", pluginId: payload.id });
+      return { status: { ...status, hasOauth: true } };


api.startMcpOAuth is typed as { ok: boolean; status }, but this payload has no ok. Also mcp.transfer can rename the server id — deleteOAuth is only wired on mcp.remove, so a moved server orphans secret:mcp:<oldId>:oauth and the new id looks unauthenticated.

vastsa · 2026-09-17T17:12:16Z

+
+          if (error) {
+            res.writeHead(400, { "content-type": "text/html; charset=utf-8" });
+            res.end(this.renderHtml(false, `授权失败: ${errorDescription || error}`));


XSS: error / error_description come from the query string and are interpolated into HTML with no escaping. This branch does not require a matching state, so any local hit on /callback?error=…&error_description=<script>… renders attacker HTML in the system browser.

Escape (or better: never interpolate untrusted strings) in renderHtml.

vastsa · 2026-09-17T17:12:16Z

+          }
+
+          // Exchange code for token
+          const tokenParams = new URLSearchParams({


MCP Authorization (2025-06-18) requires the RFC 8707 resource parameter on the token request (and on the authorize URL below). Without it, servers that follow the spec (Notion-class remote MCP) reject the exchange.

Same gap on refreshToken. Also send redirect: "manual" here — discovery/token fetch currently follows redirects, which is the opposite of ADR 0142's MCP HTTP client.

vastsa · 2026-09-17T17:12:16Z

+          }
+
+          const refreshToken = typeof tokenJson.refresh_token === "string" ? tokenJson.refresh_token : undefined;
+          const expiresIn = typeof tokenJson.expires_in === "number" ? tokenJson.expires_in : undefined;


expires_in is often a JSON string ("3600"). Treating only typeof === "number" leaves expiresAt unset, so getValidAccessToken never refreshes.

Number(tokenJson.expires_in) with a finite check. Same in refreshToken (line 503). Refresh itself should be serialized per serverId — rotating refresh tokens will race otherwise (VendorOAuth serializes modify per row for this reason).

vastsa · 2026-09-17T17:12:16Z

+    return updated;
+  }
+
+  private renderHtml(ok: boolean, message: string): string {


message is interpolated raw (<p>${message}</p>). That is the XSS sink for the callback above.

This page is also hardcoded Chinese while the app is fully i18n'd — at least use English, or pass locale in. Do not put untrusted error_description in the <title> either.

vastsa · 2026-09-17T17:12:16Z

+      }
+    }
+
+    const entry = existing ?? this.createEntry(record, oauthToken);


If an entry already exists, oauthToken is fetched and then thrown away. A live HTTP client keeps whatever Authorization was baked in at createEntry.

getValidAccessToken can refresh the secret, but tools/call still sends the old Bearer. configurationChanged compares saved record.headers, which never include the injected token, so setRecords after login will not rebuild the client either (only test() does, because it deletes the entry first).

Rebuild the client when the access token changes, and attach a 401 path on callTool, not only handshake.

vastsa · 2026-09-17T17:12:16Z

      return tools;
    } catch (error) {
+      const msg = (error as Error).message || "";
+      const is401 = msg.includes("401");


message.includes("401") is a brittle stand-in for HTTP 401. plugin-mcp happens to throw mcp server returned ${status}, so handshake works today; a JSON-RPC error or a tools/call 401 will not set authRequired.

Plumb the status code from the transport instead of scraping the error string.

vastsa · 2026-09-17T17:12:17Z

+              </span>
+            ) : status?.hasOauth ? (
+              <span className="agent-capability-badge is-status is-ready">
+                OAuth


Hardcoded English OAuth in an i18n page. Add a locale key (all 8 locales already gained the other MCP auth strings).

- Implement McpOAuthManager with RFC 9728 metadata discovery and RFC 8414 authorization server discovery - Follow VendorOAuth pattern with non-blocking start, loginId tracking, and abort/cancellation - Support RFC 7591 Dynamic Client Registration (DCR) with client_id reuse per registration endpoint - PKCE authorization code flow with RFC 8252 loopback redirect URI (http://127.0.0.1:<port>/callback) - Enforce RFC 8707 resource indicators in authorization, token exchange, and refresh requests - XSS prevention for loopback error page, robust string/number expiresIn parsing, and serial token refresh - Live client rebuild on token update, tool-level 401 interception marking authRequired - OAuth secret transfer on server rename, and redirect: 'manual' per ADR 0142 - Settings UI with progress/completion events and multi-language i18n - ADR 0283, runtime spec updates, E2E-100B scenario, and automated test coverage

vastsa · 2026-09-18T07:32:22Z

收到

Force HTTPS on authorization-server endpoints (loopback excepted), reuse DCR clients only when RFC 8252 portless or exact redirect matches, and consume loopback callbacks only after a matching state. Pass the listed server record through to onAuthorized so a project-level MCP can handshake after login, and unsubscribe the settings OAuth listener on unmount. Refs vastsa#556.

vastsa requested changes Sep 17, 2026

View reviewed changes

shylock-wu force-pushed the feat/mcp-oauth branch 3 times, most recently from 1fc6a08 to d4f9e65 Compare September 18, 2026 03:48

shylock-wu force-pushed the feat/mcp-oauth branch from d4f9e65 to 8e797e9 Compare September 18, 2026 03:50

shylock-wu requested a review from vastsa September 18, 2026 03:58

vastsa merged commit 786184a into vastsa:main Sep 18, 2026
1 check failed

vastsa mentioned this pull request Sep 18, 2026

fix(desktop): harden remote MCP OAuth after landing #582

Merged

shylock-wu deleted the feat/mcp-oauth branch September 18, 2026 13:05

Navigation Menu

feat(desktop): add MCP OAuth support for remote servers - #556

feat(desktop): add MCP OAuth support for remote servers#556
vastsa merged 1 commit into
vastsa:mainfrom
shylock-wu:feat/mcp-oauth

shylock-wu commented Sep 17, 2026

Uh oh!

vastsa left a comment

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa Sep 17, 2026

Uh oh!

vastsa commented Sep 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

shylock-wu commented Sep 17, 2026

Key Changes

Validation

Scope

Uh oh!

vastsa left a comment

Choose a reason for hiding this comment

Blockers

Also fix

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vastsa commented Sep 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants