From 7486e75ae5df1b16680b8bc3d8e092e87e76e5e1 Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Tue, 30 Jun 2026 20:20:30 +0530 Subject: [PATCH] fix(mcp): actionable MCP auth-failure errors + custom-header coverage (#4289) (#4321) --- .../channels/mcp/ConnectAuthModal.test.tsx | 71 +++++- .../channels/mcp/ConnectAuthModal.tsx | 46 +++- .../mcp/InstalledServerDetail.test.tsx | 23 ++ .../channels/mcp/InstalledServerDetail.tsx | 9 +- app/src/components/channels/mcp/types.ts | 6 + app/src/lib/i18n/ar.ts | 6 + app/src/lib/i18n/bn.ts | 6 + app/src/lib/i18n/de.ts | 6 + app/src/lib/i18n/en.ts | 6 + app/src/lib/i18n/es.ts | 6 + app/src/lib/i18n/fr.ts | 6 + app/src/lib/i18n/hi.ts | 6 + app/src/lib/i18n/id.ts | 6 + app/src/lib/i18n/it.ts | 6 + app/src/lib/i18n/ko.ts | 6 + app/src/lib/i18n/pl.ts | 6 + app/src/lib/i18n/pt.ts | 6 + app/src/lib/i18n/ru.ts | 6 + app/src/lib/i18n/zh-CN.ts | 4 + app/src/services/api/mcpClientsApi.ts | 11 +- src/openhuman/mcp_client/client_tests.rs | 99 ++++++++ src/openhuman/mcp_registry/connections.rs | 236 ++++++++++++++---- src/openhuman/mcp_registry/ops.rs | 42 +++- src/openhuman/mcp_registry/types.rs | 11 + 24 files changed, 565 insertions(+), 71 deletions(-) diff --git a/app/src/components/channels/mcp/ConnectAuthModal.test.tsx b/app/src/components/channels/mcp/ConnectAuthModal.test.tsx index 2e0d03852..6a663f5b9 100644 --- a/app/src/components/channels/mcp/ConnectAuthModal.test.tsx +++ b/app/src/components/channels/mcp/ConnectAuthModal.test.tsx @@ -1,7 +1,8 @@ import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import ConnectAuthModal from './ConnectAuthModal'; +import en from '../../../lib/i18n/en'; +import ConnectAuthModal, { authHintMessageKey, reconnectErrorMessage } from './ConnectAuthModal'; const mockConnect = vi.fn(); const mockUpdateEnv = vi.fn(); @@ -451,3 +452,71 @@ describe('ConnectAuthModal', () => { expect(screen.queryByDisplayValue('Authorization')).not.toBeInTheDocument(); }); }); + +// A `t` that resolves against the real English bundle, so these tests also prove +// the three new `mcp.connectAuth.authError.*` keys actually exist in en.ts. +const tEn = (key: string): string => (en as Record)[key] ?? key; + +describe('authHintMessageKey (#4289)', () => { + it('maps each core auth_hint code to its i18n key', () => { + expect(authHintMessageKey('oauth_required')).toBe('mcp.connectAuth.authError.oauthRequired'); + expect(authHintMessageKey('token_rejected')).toBe('mcp.connectAuth.authError.tokenRejected'); + expect(authHintMessageKey('credential_required')).toBe( + 'mcp.connectAuth.authError.credentialRequired' + ); + }); + + it('returns null for an unknown or absent code', () => { + expect(authHintMessageKey('something_else')).toBeNull(); + expect(authHintMessageKey(undefined)).toBeNull(); + }); + + it('every mapped key resolves to non-placeholder English copy', () => { + for (const code of ['oauth_required', 'token_rejected', 'credential_required']) { + const key = authHintMessageKey(code); + expect(key).not.toBeNull(); + const copy = tEn(key as string); + expect(copy).not.toBe(key); // resolved, not echoed back + expect(copy.length).toBeGreaterThan(0); + } + }); +}); + +describe('reconnectErrorMessage (#4289)', () => { + it('returns null when the reconnect actually connected', () => { + expect(reconnectErrorMessage({ status: 'connected' }, tEn)).toBeNull(); + }); + + it('OAuth-only 401 → "use Sign in" copy, never the raw message', () => { + const msg = reconnectErrorMessage({ status: 'unauthorized', auth_hint: 'oauth_required' }, tEn); + expect(msg).toBe(tEn('mcp.connectAuth.authError.oauthRequired')); + }); + + it('rejected static token 401 → token-rejected copy', () => { + const msg = reconnectErrorMessage({ status: 'unauthorized', auth_hint: 'token_rejected' }, tEn); + expect(msg).toBe(tEn('mcp.connectAuth.authError.tokenRejected')); + }); + + it('401 with no/unknown hint falls back to generic reconnect copy', () => { + // Missing hint… + expect(reconnectErrorMessage({ status: 'unauthorized' }, tEn)).toBe( + tEn('mcp.connectAuth.reconnectFailed') + ); + // …and an unrecognized code (forward-compat with a future core reason). + expect( + reconnectErrorMessage({ status: 'unauthorized', auth_hint: 'brand_new_reason' }, tEn) + ).toBe(tEn('mcp.connectAuth.reconnectFailed')); + }); + + it('a generic (non-401) failure surfaces its own message', () => { + expect(reconnectErrorMessage({ status: 'disconnected', error: 'timed out' }, tEn)).toBe( + 'timed out' + ); + }); + + it('a non-401 failure with no message falls back to generic copy', () => { + expect(reconnectErrorMessage({ status: 'disconnected' }, tEn)).toBe( + tEn('mcp.connectAuth.reconnectFailed') + ); + }); +}); diff --git a/app/src/components/channels/mcp/ConnectAuthModal.tsx b/app/src/components/channels/mcp/ConnectAuthModal.tsx index bc8ae7827..38fe2938c 100644 --- a/app/src/components/channels/mcp/ConnectAuthModal.tsx +++ b/app/src/components/channels/mcp/ConnectAuthModal.tsx @@ -68,6 +68,44 @@ const applyScheme = (scheme: 'bearer' | 'raw', value: string): string => { return v; }; +/** Map a core `auth_hint` reason code (returned by `update_env` on a 401) to its + * i18n key, so the user sees what to DO rather than an opaque "connect failed": + * • `oauth_required` → use the browser Sign-in (a pasted token won't work) + * • `token_rejected` → the token was wrong/expired + * • `credential_required` → auth is needed but none was provided + * Returns `null` for an unknown/absent code so the caller falls back to generic + * copy. Exported for unit testing. (#4289) */ +export const authHintMessageKey = (code: string | undefined): string | null => { + switch (code) { + case 'oauth_required': + return 'mcp.connectAuth.authError.oauthRequired'; + case 'token_rejected': + return 'mcp.connectAuth.authError.tokenRejected'; + case 'credential_required': + return 'mcp.connectAuth.authError.credentialRequired'; + default: + return null; + } +}; + +/** Derive the error string to show after an `update_env` reconnect, or `null` + * when it actually connected. A 401 (`status === 'unauthorized'`) maps to the + * actionable `auth_hint` copy — the raw 401 message is withheld server-side, so + * we never show it; any other non-connected status surfaces its message (or a + * generic fallback). Pure + exported so the branch is unit-tested without a full + * modal render. (#4289) */ +export const reconnectErrorMessage = ( + result: { status: string; auth_hint?: string; error?: string }, + t: (key: string) => string +): string | null => { + if (result.status === 'connected') return null; + if (result.status === 'unauthorized') { + const key = authHintMessageKey(result.auth_hint); + return key ? t(key) : t('mcp.connectAuth.reconnectFailed'); + } + return result.error ?? t('mcp.connectAuth.reconnectFailed'); +}; + /** Whether a field name is an `Authorization` header (offers a Bearer scheme). */ const isAuthorizationField = (name: string): boolean => name.toLowerCase() === 'authorization'; @@ -369,8 +407,12 @@ const ConnectAuthModal = ({ server, onClose, onConnected }: ConnectAuthModalProp if (Object.keys(env).length > 0) { log('connect-with-auth server_id=%s keys=%o', server.server_id, Object.keys(env)); const result = await mcpClientsApi.updateEnv({ server_id: server.server_id, env }); - if (result.status !== 'connected') { - throw new Error(result.error ?? t('mcp.connectAuth.reconnectFailed')); + // 401 → actionable reason (oauth-required / token-rejected / + // credential-required); other failure → its message; null → connected. + const errMsg = reconnectErrorMessage(result, t); + if (errMsg) { + setError(errMsg); + return; } tools = result.tools ?? []; } else { diff --git a/app/src/components/channels/mcp/InstalledServerDetail.test.tsx b/app/src/components/channels/mcp/InstalledServerDetail.test.tsx index c999230f2..665f38ac5 100644 --- a/app/src/components/channels/mcp/InstalledServerDetail.test.tsx +++ b/app/src/components/channels/mcp/InstalledServerDetail.test.tsx @@ -358,6 +358,29 @@ describe('InstalledServerDetail', () => { await waitFor(() => expect(screen.getByText('bad token')).toBeInTheDocument()); }); + it('surfaces actionable auth copy (not the raw 401) when reconfigure hits a 401 (#4289)', async () => { + // update_env now returns status=unauthorized + a reason code; the raw 401 + // message is withheld, so the reconfigure form must map the code to copy. + mockUpdateEnv.mockResolvedValue({ + server_id: 'srv-1', + status: 'unauthorized', + env_keys: ['API_KEY', 'DB_URL'], + auth_hint: 'oauth_required', + }); + render( + {}} /> + ); + fireEvent.click(screen.getByRole('button', { name: 'Reconfigure' })); + fireEvent.change(screen.getByLabelText('API_KEY'), { target: { value: 'k' } }); + fireEvent.change(screen.getByLabelText('DB_URL'), { target: { value: 'u' } }); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Save & reconnect' })); + }); + await waitFor(() => + expect(screen.getByText(/pasted token will not be accepted/i)).toBeInTheDocument() + ); + }); + // ---------------------------------------------------------------------- // Tool Execution Playground gating (PR review fix) // ---------------------------------------------------------------------- diff --git a/app/src/components/channels/mcp/InstalledServerDetail.tsx b/app/src/components/channels/mcp/InstalledServerDetail.tsx index a487c946c..2fab30a48 100644 --- a/app/src/components/channels/mcp/InstalledServerDetail.tsx +++ b/app/src/components/channels/mcp/InstalledServerDetail.tsx @@ -10,7 +10,7 @@ import { mcpClientsApi } from '../../../services/api/mcpClientsApi'; import Button from '../../ui/Button'; import { clearConfigChat } from './ConfigAssistantPanel'; import ConfigHelpModal from './ConfigHelpModal'; -import ConnectAuthModal from './ConnectAuthModal'; +import ConnectAuthModal, { authHintMessageKey } from './ConnectAuthModal'; import McpStatusBadge from './McpStatusBadge'; import McpToolList from './McpToolList'; import McpToolPlayground from './McpToolPlayground'; @@ -205,6 +205,13 @@ const InstalledServerDetail = ({ env: reconfigValues, }); setTools(result.tools ?? []); + if (result.status === 'unauthorized') { + // A 401 after reconfigure: show the actionable auth reason (use Sign in + // / token rejected / credential required) the same way the Connect + // dialog does — the raw 401 message is withheld server-side (#4289). + const key = authHintMessageKey(result.auth_hint); + throw new Error(key ? t(key) : t('mcp.detail.reconfigureReconnectFailed')); + } if (result.status !== 'connected') { throw new Error(result.error ?? t('mcp.detail.reconfigureReconnectFailed')); } diff --git a/app/src/components/channels/mcp/types.ts b/app/src/components/channels/mcp/types.ts index bb0bc7cca..07b56d339 100644 --- a/app/src/components/channels/mcp/types.ts +++ b/app/src/components/channels/mcp/types.ts @@ -75,4 +75,10 @@ export type ConnStatus = { status: ServerStatus; tool_count: number; last_error?: string; + /** + * Stable auth-failure reason code refining `status === 'unauthorized'`: + * `'oauth_required'` / `'token_rejected'` / `'credential_required'`. Present + * only for a 401; never carries the raw message / OAuth metadata URL (#4289). + */ + auth_hint?: string; }; diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index 43b1bb318..63a1402dd 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -1585,6 +1585,12 @@ const messages: TranslationMap = { 'mcp.connectAuth.oauthOrToken': 'هل لديك بالفعل رمز وصول؟ الصقه بدلاً من ذلك كرأس Authorization أدناه.', 'mcp.connectAuth.oauthTimeout': 'انتهت مهلة انتظار تسجيل الدخول عبر المتصفح. حاول مرة أخرى.', + 'mcp.connectAuth.authError.oauthRequired': + 'يستخدم هذا الخادم OAuth. استخدم “تسجيل الدخول عبر المتصفح” — لن يتم قبول رمز مميز ملصوق.', + 'mcp.connectAuth.authError.tokenRejected': + 'رفض الخادم هذا الرمز المميز. تأكد من أنه صحيح وأن صلاحيته لم تنتهِ.', + 'mcp.connectAuth.authError.credentialRequired': + 'يتطلب هذا الخادم مصادقة. أضف رمزًا مميزًا أو سجّل الدخول.', 'onboarding.skipForNow': 'التخطي الآن', 'onboarding.localAI.continueWithCloud': 'متابعة مع السحابة', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index 3dacf7e1d..e030d1d1b 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -1617,6 +1617,12 @@ const messages: TranslationMap = { 'ইতিমধ্যে একটি অ্যাক্সেস টোকেন আছে? এর পরিবর্তে নিচে Authorization হেডার হিসেবে পেস্ট করুন।', 'mcp.connectAuth.oauthTimeout': 'ব্রাউজার সাইন-ইনের জন্য অপেক্ষার সময় শেষ হয়েছে। আবার চেষ্টা করুন।', + 'mcp.connectAuth.authError.oauthRequired': + 'এই সার্ভারটি OAuth ব্যবহার করে। “ব্রাউজার দিয়ে সাইন ইন করুন” ব্যবহার করুন — পেস্ট করা টোকেন গ্রহণ করা হবে না।', + 'mcp.connectAuth.authError.tokenRejected': + 'সার্ভার এই টোকেনটি প্রত্যাখ্যান করেছে। এটি সঠিক এবং এর মেয়াদ শেষ হয়নি তা যাচাই করুন।', + 'mcp.connectAuth.authError.credentialRequired': + 'এই সার্ভারের জন্য প্রমাণীকরণ প্রয়োজন। একটি টোকেন যোগ করুন, অথবা সাইন ইন করুন।', 'onboarding.skipForNow': 'এখনই এড়িয়ে যান', 'onboarding.localAI.continueWithCloud': 'ক্লাউডের সাথে চালিয়ে যান', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index d18219781..a30bf7a6c 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -1661,6 +1661,12 @@ const messages: TranslationMap = { 'Sie haben bereits ein Zugriffstoken? Fügen Sie es stattdessen unten als Authorization-Header ein.', 'mcp.connectAuth.oauthTimeout': 'Zeitüberschreitung beim Warten auf die Browser-Anmeldung. Versuchen Sie es erneut.', + 'mcp.connectAuth.authError.oauthRequired': + 'Dieser Server verwendet OAuth. Nutzen Sie “Im Browser anmelden” — ein eingefügtes Token wird nicht akzeptiert.', + 'mcp.connectAuth.authError.tokenRejected': + 'Der Server hat dieses Token abgelehnt. Prüfen Sie, ob es korrekt ist und nicht abgelaufen ist.', + 'mcp.connectAuth.authError.credentialRequired': + 'Dieser Server erfordert eine Authentifizierung. Fügen Sie ein Token hinzu oder melden Sie sich an.', 'onboarding.skipForNow': 'Vorerst überspringen', 'onboarding.localAI.continueWithCloud': 'Fahren Sie mit der Cloud fort.', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index be62cded8..dfd023fca 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -1963,6 +1963,12 @@ const en: TranslationMap = { 'mcp.connectAuth.oauthOrToken': 'Already have an access token? Paste it as an Authorization header below instead.', 'mcp.connectAuth.oauthTimeout': 'Timed out waiting for browser sign-in. Try again.', + 'mcp.connectAuth.authError.oauthRequired': + 'This server uses OAuth. Use “Sign in with browser” — a pasted token will not be accepted.', + 'mcp.connectAuth.authError.tokenRejected': + 'The server rejected this token. Check that it is correct and has not expired.', + 'mcp.connectAuth.authError.credentialRequired': + 'This server requires authentication. Add a token, or sign in.', 'mcp.detail.enable': 'Enable', 'mcp.detail.disable': 'Disable', 'mcp.status.disabled': 'Disabled', diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index 588770e62..24b6619dd 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -1652,6 +1652,12 @@ const messages: TranslationMap = { '¿Ya tienes un token de acceso? Pégalo abajo como encabezado Authorization en su lugar.', 'mcp.connectAuth.oauthTimeout': 'Se agotó el tiempo de espera del inicio de sesión en el navegador. Inténtalo de nuevo.', + 'mcp.connectAuth.authError.oauthRequired': + 'Este servidor usa OAuth. Usa “Iniciar sesión con el navegador” — no se aceptará un token pegado.', + 'mcp.connectAuth.authError.tokenRejected': + 'El servidor rechazó este token. Comprueba que sea correcto y que no haya caducado.', + 'mcp.connectAuth.authError.credentialRequired': + 'Este servidor requiere autenticación. Agrega un token o inicia sesión.', 'onboarding.skipForNow': 'Saltar por ahora', 'onboarding.localAI.continueWithCloud': 'Continuar con la nube', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index f7a475c3b..8dda734a6 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -1661,6 +1661,12 @@ const messages: TranslationMap = { "Vous avez déjà un jeton d'accès ? Collez-le plutôt ci-dessous comme en-tête Authorization.", 'mcp.connectAuth.oauthTimeout': 'Délai dépassé en attendant la connexion via le navigateur. Réessayez.', + 'mcp.connectAuth.authError.oauthRequired': + 'Ce serveur utilise OAuth. Utilisez “Se connecter avec le navigateur” — un jeton collé ne sera pas accepté.', + 'mcp.connectAuth.authError.tokenRejected': + "Le serveur a rejeté ce jeton. Vérifiez qu'il est correct et qu'il n'a pas expiré.", + 'mcp.connectAuth.authError.credentialRequired': + 'Ce serveur nécessite une authentification. Ajoutez un jeton ou connectez-vous.', 'onboarding.skipForNow': "Passer pour l'instant", 'onboarding.localAI.continueWithCloud': 'Continuer avec Cloud', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index 7b56906f9..fe331dbe4 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -1616,6 +1616,12 @@ const messages: TranslationMap = { 'क्या आपके पास पहले से एक एक्सेस टोकन है? इसके बजाय इसे नीचे Authorization हेडर के रूप में पेस्ट करें।', 'mcp.connectAuth.oauthTimeout': 'ब्राउज़र साइन-इन की प्रतीक्षा का समय समाप्त हो गया। पुनः प्रयास करें।', + 'mcp.connectAuth.authError.oauthRequired': + 'यह सर्वर OAuth का उपयोग करता है। “ब्राउज़र से साइन इन करें” का उपयोग करें — पेस्ट किया गया टोकन स्वीकार नहीं किया जाएगा।', + 'mcp.connectAuth.authError.tokenRejected': + 'सर्वर ने इस टोकन को अस्वीकार कर दिया। जाँचें कि यह सही है और इसकी समय-सीमा समाप्त नहीं हुई है।', + 'mcp.connectAuth.authError.credentialRequired': + 'इस सर्वर के लिए प्रमाणीकरण आवश्यक है। एक टोकन जोड़ें, या साइन इन करें।', 'onboarding.skipForNow': 'अभी के लिए छोड़ें', 'onboarding.localAI.continueWithCloud': 'बादल के साथ जारी रखें', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index 6e3fc3996..9aecf6c3b 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -1625,6 +1625,12 @@ const messages: TranslationMap = { 'mcp.connectAuth.oauthOrToken': 'Sudah punya token akses? Tempelkan sebagai header Authorization di bawah ini saja.', 'mcp.connectAuth.oauthTimeout': 'Waktu menunggu proses masuk lewat browser habis. Coba lagi.', + 'mcp.connectAuth.authError.oauthRequired': + 'Server ini menggunakan OAuth. Gunakan “Masuk dengan browser” — token yang ditempelkan tidak akan diterima.', + 'mcp.connectAuth.authError.tokenRejected': + 'Server menolak token ini. Pastikan token benar dan belum kedaluwarsa.', + 'mcp.connectAuth.authError.credentialRequired': + 'Server ini memerlukan autentikasi. Tambahkan token, atau masuk.', 'onboarding.skipForNow': 'Lewati Sekarang', 'onboarding.localAI.continueWithCloud': 'Lanjutkan dengan Cloud', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index bbdf52fda..7ec8203e6 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -1650,6 +1650,12 @@ const messages: TranslationMap = { 'mcp.connectAuth.oauthOrToken': 'Hai già un token di accesso? Incollalo invece come intestazione Authorization qui sotto.', 'mcp.connectAuth.oauthTimeout': "Tempo scaduto in attesa dell'accesso dal browser. Riprova.", + 'mcp.connectAuth.authError.oauthRequired': + 'Questo server usa OAuth. Usa “Accedi con il browser” — un token incollato non verrà accettato.', + 'mcp.connectAuth.authError.tokenRejected': + 'Il server ha rifiutato questo token. Verifica che sia corretto e che non sia scaduto.', + 'mcp.connectAuth.authError.credentialRequired': + "Questo server richiede l'autenticazione. Aggiungi un token oppure accedi.", 'onboarding.skipForNow': 'Salta per ora', 'onboarding.localAI.continueWithCloud': 'Continua con Cloud', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index b3d5fb346..029dcde2a 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -1607,6 +1607,12 @@ const messages: TranslationMap = { 'mcp.connectAuth.oauthOrToken': '이미 액세스 토큰이 있나요? 대신 아래에 Authorization 헤더로 붙여넣으세요.', 'mcp.connectAuth.oauthTimeout': '브라우저 로그인 대기 시간이 초과되었습니다. 다시 시도하세요.', + 'mcp.connectAuth.authError.oauthRequired': + '이 서버는 OAuth를 사용합니다. “브라우저로 로그인”을 사용하세요. 붙여넣은 토큰은 사용할 수 없습니다.', + 'mcp.connectAuth.authError.tokenRejected': + '서버가 이 토큰을 거부했습니다. 토큰이 올바르고 만료되지 않았는지 확인하세요.', + 'mcp.connectAuth.authError.credentialRequired': + '이 서버는 인증이 필요합니다. 토큰을 추가하거나 로그인하세요.', 'onboarding.skipForNow': '지금 건너뛰기', 'onboarding.localAI.continueWithCloud': '클라우드 계속하기', 'onboarding.localAI.useLocalAnyway': '어쨌든 로컬 AI 사용(기기에 권장되지 않음)', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index a9260fc3c..c680e23f2 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -1638,6 +1638,12 @@ const messages: TranslationMap = { 'Masz już token dostępu? Wklej go zamiast tego poniżej jako nagłówek Authorization.', 'mcp.connectAuth.oauthTimeout': 'Upłynął limit czasu oczekiwania na logowanie w przeglądarce. Spróbuj ponownie.', + 'mcp.connectAuth.authError.oauthRequired': + 'Ten serwer używa OAuth. Użyj “Zaloguj się przez przeglądarkę” — wklejony token nie zostanie zaakceptowany.', + 'mcp.connectAuth.authError.tokenRejected': + 'Serwer odrzucił ten token. Sprawdź, czy jest poprawny i czy nie wygasł.', + 'mcp.connectAuth.authError.credentialRequired': + 'Ten serwer wymaga uwierzytelnienia. Dodaj token lub zaloguj się.', 'onboarding.skipForNow': 'Pomiń na razie', 'onboarding.localAI.continueWithCloud': 'Kontynuuj z chmurą', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index 017a49d64..d6d4271dc 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -1655,6 +1655,12 @@ const messages: TranslationMap = { 'Já tem um token de acesso? Cole-o abaixo como cabeçalho Authorization.', 'mcp.connectAuth.oauthTimeout': 'Tempo esgotado aguardando o login no navegador. Tente novamente.', + 'mcp.connectAuth.authError.oauthRequired': + 'Este servidor usa OAuth. Use “Entrar com o navegador” — um token colado não será aceito.', + 'mcp.connectAuth.authError.tokenRejected': + 'O servidor rejeitou este token. Verifique se está correto e se não expirou.', + 'mcp.connectAuth.authError.credentialRequired': + 'Este servidor requer autenticação. Adicione um token ou entre.', 'onboarding.skipForNow': 'Ignorar por agora', 'onboarding.localAI.continueWithCloud': 'Continuar com a nuvem', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 28459b4ca..bcf9d3adf 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -1635,6 +1635,12 @@ const messages: TranslationMap = { 'mcp.connectAuth.oauthOrToken': 'Уже есть токен доступа? Вставьте его ниже как заголовок Authorization.', 'mcp.connectAuth.oauthTimeout': 'Истекло время ожидания входа через браузер. Попробуйте снова.', + 'mcp.connectAuth.authError.oauthRequired': + 'Этот сервер использует OAuth. Используйте “Войти через браузер” — вставленный токен не будет принят.', + 'mcp.connectAuth.authError.tokenRejected': + 'Сервер отклонил этот токен. Убедитесь, что он правильный и не истёк.', + 'mcp.connectAuth.authError.credentialRequired': + 'Этот сервер требует аутентификации. Добавьте токен или войдите.', 'onboarding.skipForNow': 'Пропустить сейчас', 'onboarding.localAI.continueWithCloud': 'Продолжить с Облако', 'onboarding.localAI.useLocalAnyway': diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index 1e985f618..8055dcfb5 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -1538,6 +1538,10 @@ const messages: TranslationMap = { 'mcp.connectAuth.oauthWaiting': '正在等待登录…', 'mcp.connectAuth.oauthOrToken': '已经有访问令牌?请改为在下方将其粘贴为 Authorization 请求头。', 'mcp.connectAuth.oauthTimeout': '等待浏览器登录超时。请重试。', + 'mcp.connectAuth.authError.oauthRequired': + '此服务器使用 OAuth。请使用“使用浏览器登录”——粘贴的令牌将不被接受。', + 'mcp.connectAuth.authError.tokenRejected': '服务器拒绝了此令牌。请检查它是否正确且尚未过期。', + 'mcp.connectAuth.authError.credentialRequired': '此服务器需要身份验证。请添加令牌或登录。', 'onboarding.skipForNow': '暂时跳过', 'onboarding.localAI.continueWithCloud': '继续使用云', 'onboarding.localAI.useLocalAnyway': '无论如何使用本地人工智能(不推荐用于您的设备)', diff --git a/app/src/services/api/mcpClientsApi.ts b/app/src/services/api/mcpClientsApi.ts index f26608ae3..ae9316789 100644 --- a/app/src/services/api/mcpClientsApi.ts +++ b/app/src/services/api/mcpClientsApi.ts @@ -77,10 +77,19 @@ interface ConfigAssistResult { interface UpdateEnvResult { server_id: string; - status: 'connected' | 'disconnected'; + status: 'connected' | 'disconnected' | 'disabled' | 'unauthorized'; env_keys: string[]; tools?: McpTool[]; error?: string; + /** + * Stable auth-failure reason code present when `status === 'unauthorized'`: + * `'oauth_required'` (use Sign in — a pasted token won't work), + * `'token_rejected'` (credential sent but refused), or + * `'credential_required'` (auth needed, none provided). The raw 401 message + * is intentionally withheld server-side (it leaks the OAuth metadata URL), + * so the UI maps this code to localized copy (#4289). + */ + auth_hint?: string; } /** Non-secret registry-credentials snapshot. Secret *values* are never returned. */ diff --git a/src/openhuman/mcp_client/client_tests.rs b/src/openhuman/mcp_client/client_tests.rs index 47024df7c..bdfcee8f3 100644 --- a/src/openhuman/mcp_client/client_tests.rs +++ b/src/openhuman/mcp_client/client_tests.rs @@ -521,6 +521,105 @@ async fn bearer_auth_is_attached_to_initialize() { assert_eq!(init.server_info["name"], "bearer-server"); } +/// 401 unless the single custom header `X-Custom-Token: tok-xyz` is present — +/// proves the `McpAuthConfig::Header` arm reaches the wire (#4289). +async fn custom_header_required_handler( + headers: AxumHeaderMap, + Json(body): Json, +) -> Response { + if headers.get("x-custom-token").and_then(|v| v.to_str().ok()) != Some("tok-xyz") { + return ( + StatusCode::UNAUTHORIZED, + "missing custom header".to_string(), + ) + .into_response(); + } + Json(json!({ + "jsonrpc": "2.0", + "id": body["id"].clone(), + "result": { + "protocolVersion": LATEST_PROTOCOL_VERSION, + "capabilities": {}, + "serverInfo": { "name": "custom-header-server", "version": "1.0.0" } + } + })) + .into_response() +} + +/// 401 unless BOTH `X-Client-Key` and `Authorization` are present — proves the +/// `McpAuthConfig::Headers` (multi-header) arm sends every header (#4289). +async fn multi_header_required_handler( + headers: AxumHeaderMap, + Json(body): Json, +) -> Response { + let key_ok = headers.get("x-client-key").and_then(|v| v.to_str().ok()) == Some("ck-1"); + let auth_ok = + headers.get(AUTHORIZATION).and_then(|v| v.to_str().ok()) == Some("Bearer multi-secret"); + if !(key_ok && auth_ok) { + return (StatusCode::UNAUTHORIZED, "missing a header".to_string()).into_response(); + } + Json(json!({ + "jsonrpc": "2.0", + "id": body["id"].clone(), + "result": { + "protocolVersion": LATEST_PROTOCOL_VERSION, + "capabilities": {}, + "serverInfo": { "name": "multi-header-server", "version": "1.0.0" } + } + })) + .into_response() +} + +#[tokio::test] +async fn custom_header_auth_is_attached_to_initialize() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let app = Router::new().route("/", post(custom_header_required_handler)); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + let client = McpHttpClient::with_options( + format!("http://{addr}/"), + 2, + McpAuthConfig::Header { + name: "X-Custom-Token".into(), + value: "tok-xyz".into(), + }, + McpClientIdentityConfig::default(), + ); + let init = client.initialize().await.expect("initialize"); + assert_eq!(init.server_info["name"], "custom-header-server"); +} + +#[tokio::test] +async fn multi_header_auth_all_attached_to_initialize() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let app = Router::new().route("/", post(multi_header_required_handler)); + tokio::spawn(async move { + axum::serve(listener, app).await.unwrap(); + }); + let client = McpHttpClient::with_options( + format!("http://{addr}/"), + 2, + McpAuthConfig::Headers { + headers: vec![ + crate::openhuman::config::HttpHeader { + name: "X-Client-Key".into(), + value: "ck-1".into(), + }, + crate::openhuman::config::HttpHeader { + name: "Authorization".into(), + value: "Bearer multi-secret".into(), + }, + ], + }, + McpClientIdentityConfig::default(), + ); + let init = client.initialize().await.expect("initialize"); + assert_eq!(init.server_info["name"], "multi-header-server"); +} + #[test] fn display_description_runs_full_sanitization_pipeline() { let tool = McpRemoteTool { diff --git a/src/openhuman/mcp_registry/connections.rs b/src/openhuman/mcp_registry/connections.rs index a8360850f..fdc964593 100644 --- a/src/openhuman/mcp_registry/connections.rs +++ b/src/openhuman/mcp_registry/connections.rs @@ -211,13 +211,70 @@ fn connections() -> &'static RwLock>> { /// observe a torn snapshot — e.g. the message updated but the auth flag stale, /// which a two-map design would allow if `all_status` interleaved between the /// two writes (#3719). +/// Why an MCP HTTP 401 happened, refining `ServerStatus::Unauthorized` so the UI +/// can tell the user what to actually DO. Derived purely from two signals — the +/// server's `WWW-Authenticate` challenge (does it advertise OAuth?) and whether +/// the user supplied any credential — so it never leaks the OAuth metadata URL. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum AuthFailureKind { + /// Server advertised an OAuth `resource_metadata` challenge. The right path + /// is the browser Sign-in flow; a pasted static token won't be accepted + /// (the Notion-style case in #4289). + OauthRequired, + /// A static credential WAS sent but the server refused it — wrong/expired. + TokenRejected, + /// 401 with no credential supplied and no OAuth challenge — plain auth that + /// the user simply hasn't filled in yet. + CredentialRequired, +} + +impl AuthFailureKind { + /// Stable wire code consumed by the frontend (maps to localized copy). Kept + /// in sync with the FE `auth_hint` switch — see ConnectAuthModal. + fn as_code(self) -> &'static str { + match self { + Self::OauthRequired => "oauth_required", + Self::TokenRejected => "token_rejected", + Self::CredentialRequired => "credential_required", + } + } +} + +/// Pure 401-reason decision, factored out so it's unit-testable without a live +/// dial. `oauth_advertised` dominates: an OAuth-only server commonly 401s a +/// pasted static bearer, and telling the user "token wrong" would misdirect +/// them — the real action is Sign in. +fn classify_auth_failure(oauth_advertised: bool, has_credential: bool) -> AuthFailureKind { + if oauth_advertised { + AuthFailureKind::OauthRequired + } else if has_credential { + AuthFailureKind::TokenRejected + } else { + AuthFailureKind::CredentialRequired + } +} + +/// Classify a connect error: `Some(kind)` only when the root cause is a typed +/// MCP HTTP 401 (`McpUnauthorizedError`), `None` for any generic transport +/// error. Walks the whole `anyhow` chain so the classification survives +/// `?`/`.context()` wrapping, and reads the typed `resource_metadata` field +/// (not the message string) to decide whether OAuth was advertised. +fn auth_failure_kind(err: &anyhow::Error, has_credential: bool) -> Option { + let unauthorized = err.chain().find_map(|cause| { + cause.downcast_ref::() + })?; + let oauth_advertised = unauthorized.resource_metadata.is_some(); + Some(classify_auth_failure(oauth_advertised, has_credential)) +} + #[derive(Clone)] struct ConnectFailure { message: String, - /// `true` when the failure was an MCP HTTP 401 → drives + /// `Some(kind)` when the failure was an MCP HTTP 401 → drives /// `ServerStatus::Unauthorized` so the UI offers a re-auth path instead of - /// a raw error blob. - unauthorized: bool, + /// a raw error blob, with `kind` refining which re-auth affordance to show. + /// `None` for a generic (non-401) transport error. + auth: Option, } static LAST_ERRORS: OnceLock>> = OnceLock::new(); @@ -226,16 +283,6 @@ fn last_errors() -> &'static RwLock> { LAST_ERRORS.get_or_init(|| RwLock::new(HashMap::new())) } -/// Whether a connect failure's root cause was an MCP HTTP 401 (auth required). -/// Uses a typed `downcast` on the `anyhow` chain — not string matching — so the -/// classification can't drift from the message wording. -fn is_unauthorized_error(err: &anyhow::Error) -> bool { - // Walk the whole source chain (not just the outermost error) so the - // classification survives any `?`/`.context()` wrapping a caller may add. - err.chain() - .any(|cause| cause.is::()) -} - /// Read the most recent connect-failure message for `server_id`. `None` when /// the server has never failed, or when the most recent connect succeeded. pub async fn last_error_for(server_id: &str) -> Option { @@ -246,13 +293,27 @@ pub async fn last_error_for(server_id: &str) -> Option { .map(|f| f.message.clone()) } +/// The stable auth-failure reason code for `server_id`'s most recent connect, +/// or `None` when the last failure wasn't a 401 (or there was none). Reads the +/// classification recorded by [`connect`] so callers (e.g. `update_env`) can +/// surface actionable copy WITHOUT re-leaking the raw 401 message / OAuth +/// metadata URL (#3719, #4289). +pub async fn auth_hint_for(server_id: &str) -> Option<&'static str> { + last_errors() + .read() + .await + .get(server_id) + .and_then(|f| f.auth) + .map(AuthFailureKind::as_code) +} + /// Whether `server_id`'s most recent connect failed due to HTTP 401. pub async fn needs_auth(server_id: &str) -> bool { last_errors() .read() .await .get(server_id) - .is_some_and(|f| f.unauthorized) + .is_some_and(|f| f.auth.is_some()) } /// Drop any recorded error (generic or auth-required) for `server_id`. Called on @@ -288,20 +349,32 @@ pub async fn connect(config: &Config, server: &InstalledServer) -> anyhow::Resul ); } Err(err) => { + // Re-derive whether the user supplied a credential the SAME way the + // dial did (`build_http_auth` over stored env) so the 401 reason has + // one source of truth. Cold failure path — one extra store read is + // negligible. + let has_credential = { + let env: Vec<(String, String)> = store::load_env_values(config, &server.server_id) + .unwrap_or_default() + .into_iter() + .collect(); + !matches!(build_http_auth(&env), McpAuthConfig::None) + }; + let auth = auth_failure_kind(err, has_credential); // Record the raw diagnostic AND the 401 classification together in a // single record under one lock, so a concurrent `all_status` can't // observe a torn snapshot (message set but auth flag stale). - let unauthorized = is_unauthorized_error(err); last_errors().write().await.insert( server.server_id.clone(), ConnectFailure { message: err.to_string(), - unauthorized, + auth, }, ); tracing::debug!( - "[mcp-registry] last_error recorded server_id={} unauthorized={unauthorized} err={err}", - server.server_id + "[mcp-registry] last_error recorded server_id={} auth={:?} err={err}", + server.server_id, + auth.map(AuthFailureKind::as_code), ); } } @@ -538,19 +611,21 @@ pub async fn call_tool( fn classify_server_status( enabled: bool, connected_tool_count: Option, - auth_required: bool, + auth_failure: Option, generic_error: Option, -) -> (ServerStatus, u32, Option) { +) -> (ServerStatus, u32, Option, Option<&'static str>) { if !enabled { - (ServerStatus::Disabled, 0, None) + (ServerStatus::Disabled, 0, None, None) } else if let Some(n) = connected_tool_count { - (ServerStatus::Connected, n, None) - } else if auth_required { - (ServerStatus::Unauthorized, 0, None) + (ServerStatus::Connected, n, None, None) + } else if let Some(kind) = auth_failure { + // 401: surface the stable reason CODE (not the raw message) so the UI + // shows the right re-auth affordance without leaking the metadata URL. + (ServerStatus::Unauthorized, 0, None, Some(kind.as_code())) } else if let Some(err) = generic_error { - (ServerStatus::Error, 0, Some(err)) + (ServerStatus::Error, 0, Some(err), None) } else { - (ServerStatus::Disconnected, 0, None) + (ServerStatus::Disconnected, 0, None, None) } } @@ -583,14 +658,14 @@ pub async fn all_status(config: &Config) -> Vec { }; let failure = failures_snapshot.get(&s.server_id); - let (status, tool_count, last_error) = classify_server_status( + let (status, tool_count, last_error, auth_hint) = classify_server_status( s.enabled, connected_tool_count, - failure.is_some_and(|f| f.unauthorized), + failure.and_then(|f| f.auth), // Only a generic (non-401) failure carries a surfaced message; the // 401 message is withheld (it leaks the OAuth metadata URL). failure - .filter(|f| !f.unauthorized) + .filter(|f| f.auth.is_none()) .map(|f| f.message.clone()), ); @@ -601,6 +676,7 @@ pub async fn all_status(config: &Config) -> Vec { status, tool_count, last_error, + auth_hint: auth_hint.map(str::to_string), }); } out @@ -689,7 +765,8 @@ mod tests { // Live-connection tests require a real MCP subprocess and live in // tests/json_rpc_e2e.rs. Keep this slot for sync helper tests. use super::{ - build_http_auth, classify_server_status, credential_safe_dial_url, is_unauthorized_error, + auth_failure_kind, build_http_auth, classify_auth_failure, classify_server_status, + credential_safe_dial_url, AuthFailureKind, }; use crate::openhuman::config::McpAuthConfig; use crate::openhuman::mcp_client::McpUnauthorizedError; @@ -697,52 +774,101 @@ mod tests { #[test] fn classify_server_status_priority_order() { + let oauth = Some(AuthFailureKind::OauthRequired); // Disabled wins over everything (even a live connection / 401 / error). assert_eq!( - classify_server_status(false, Some(3), true, Some("boom".into())), - (ServerStatus::Disabled, 0, None) + classify_server_status(false, Some(3), oauth, Some("boom".into())), + (ServerStatus::Disabled, 0, None, None) ); - // Connected → tool count surfaced, no error. + // Connected → tool count surfaced, no error, no hint. assert_eq!( - classify_server_status(true, Some(5), true, Some("boom".into())), - (ServerStatus::Connected, 5, None) + classify_server_status(true, Some(5), oauth, Some("boom".into())), + (ServerStatus::Connected, 5, None, None) ); - // Not connected + 401 → Unauthorized, and NO raw error is leaked even - // when a generic error is also recorded. + // Not connected + 401 → Unauthorized + reason CODE, and NO raw error is + // leaked even when a generic error is also recorded. assert_eq!( - classify_server_status(true, None, true, Some("MCP unauthorized … HTTP 401".into())), - (ServerStatus::Unauthorized, 0, None) + classify_server_status( + true, + None, + Some(AuthFailureKind::TokenRejected), + Some("MCP unauthorized … HTTP 401".into()) + ), + (ServerStatus::Unauthorized, 0, None, Some("token_rejected")) ); - // Not connected + generic error (no 401) → Error + message. + // Not connected + generic error (no 401) → Error + message, no hint. assert_eq!( - classify_server_status(true, None, false, Some("timed out".into())), - (ServerStatus::Error, 0, Some("timed out".into())) + classify_server_status(true, None, None, Some("timed out".into())), + (ServerStatus::Error, 0, Some("timed out".into()), None) ); // Not connected, no error → Disconnected. assert_eq!( - classify_server_status(true, None, false, None), - (ServerStatus::Disconnected, 0, None) + classify_server_status(true, None, None, None), + (ServerStatus::Disconnected, 0, None, None) ); } #[test] - fn is_unauthorized_error_classifies_typed_401_only() { - // A typed 401 from the transport → auth required (regardless of the - // message wording, since we downcast rather than string-match). - let unauth = anyhow::Error::new(McpUnauthorizedError { + fn classify_auth_failure_oauth_dominates() { + // OAuth advertised → Sign-in, regardless of whether a token was pasted + // (a static bearer on an OAuth-only server is the #4289 repro). + assert_eq!( + classify_auth_failure(true, false), + AuthFailureKind::OauthRequired + ); + assert_eq!( + classify_auth_failure(true, true), + AuthFailureKind::OauthRequired + ); + // No OAuth, credential sent → it was refused (wrong/expired). + assert_eq!( + classify_auth_failure(false, true), + AuthFailureKind::TokenRejected + ); + // No OAuth, no credential → user just hasn't authenticated yet. + assert_eq!( + classify_auth_failure(false, false), + AuthFailureKind::CredentialRequired + ); + } + + #[test] + fn auth_failure_kind_reads_typed_401_only() { + // Typed 401 advertising OAuth resource metadata → OauthRequired even + // though a credential was supplied. + let oauth = anyhow::Error::new(McpUnauthorizedError { endpoint: "https://example.com".into(), resource_metadata: Some("https://example.com/.well-known/x".into()), }); - assert!(is_unauthorized_error(&unauth)); + assert_eq!( + auth_failure_kind(&oauth, true), + Some(AuthFailureKind::OauthRequired) + ); - // A 401 survives `?`-style context wrapping (anyhow keeps the root - // downcastable), matching how the error reaches `connect`. - let wrapped = unauth.context("connecting to MCP server"); - assert!(is_unauthorized_error(&wrapped)); + // Plain 401 (no OAuth challenge) + a supplied credential → TokenRejected, + // and the classification survives `?`-style context wrapping. + let plain = anyhow::Error::new(McpUnauthorizedError { + endpoint: "https://example.com".into(), + resource_metadata: None, + }) + .context("connecting to MCP server"); + assert_eq!( + auth_failure_kind(&plain, true), + Some(AuthFailureKind::TokenRejected) + ); + // Same plain 401 with NO credential → CredentialRequired. + let plain2 = anyhow::Error::new(McpUnauthorizedError { + endpoint: "https://example.com".into(), + resource_metadata: None, + }); + assert_eq!( + auth_failure_kind(&plain2, false), + Some(AuthFailureKind::CredentialRequired) + ); - // Any other transport failure → NOT auth required (stays generic Error). + // Any other transport failure → None (stays a generic Error). let other = anyhow::anyhow!("MCP HTTP 500 — upstream exploded"); - assert!(!is_unauthorized_error(&other)); + assert_eq!(auth_failure_kind(&other, true), None); } fn kv(pairs: &[(&str, &str)]) -> Vec<(String, String)> { diff --git a/src/openhuman/mcp_registry/ops.rs b/src/openhuman/mcp_registry/ops.rs index f05dce43b..455e25f5e 100644 --- a/src/openhuman/mcp_registry/ops.rs +++ b/src/openhuman/mcp_registry/ops.rs @@ -600,17 +600,37 @@ pub async fn mcp_clients_update_env( )], )) } - Err(err) => Ok(RpcOutcome::new( - json!({ - "server_id": server_id, - "status": "disconnected", - "env_keys": server.env_keys, - "error": err.to_string(), - }), - vec![format!( - "update_env persisted env for server_id={server_id} but reconnect failed: {err}" - )], - )), + Err(err) => { + // A 401 is surfaced as `unauthorized` + a stable `auth_hint` code + // (oauth_required / token_rejected / credential_required) the UI maps + // to actionable copy — the raw message is WITHHELD because it leaks + // the OAuth metadata URL (#3719, #4289). Generic transport failures + // keep their diagnostic message under `disconnected`. + match connections::auth_hint_for(server_id).await { + Some(hint) => Ok(RpcOutcome::new( + json!({ + "server_id": server_id, + "status": "unauthorized", + "env_keys": server.env_keys, + "auth_hint": hint, + }), + vec![format!( + "update_env persisted env for server_id={server_id} but reconnect was unauthorized: {hint}" + )], + )), + None => Ok(RpcOutcome::new( + json!({ + "server_id": server_id, + "status": "disconnected", + "env_keys": server.env_keys, + "error": err.to_string(), + }), + vec![format!( + "update_env persisted env for server_id={server_id} but reconnect failed: {err}" + )], + )), + } + } } } diff --git a/src/openhuman/mcp_registry/types.rs b/src/openhuman/mcp_registry/types.rs index ad4803b29..232ff173e 100644 --- a/src/openhuman/mcp_registry/types.rs +++ b/src/openhuman/mcp_registry/types.rs @@ -210,6 +210,14 @@ pub struct ConnStatus { pub status: ServerStatus, pub tool_count: u32, pub last_error: Option, + /// Stable reason code refining a `ServerStatus::Unauthorized` so the UI can + /// render actionable copy: `"oauth_required"` (use Sign in — a pasted token + /// won't work), `"token_rejected"` (a credential was sent but refused), or + /// `"credential_required"` (auth needed, none provided). `None` for every + /// non-401 status. Only this code crosses the wire — never the raw 401 + /// message / OAuth metadata URL (#3719, #4289). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub auth_hint: Option, } // ── Smithery registry DTOs ─────────────────────────────────────────────────── @@ -620,8 +628,11 @@ mod tests { status: ServerStatus::Connected, tool_count: 3, last_error: None, + auth_hint: None, }; let v = serde_json::to_value(&s).unwrap(); assert_eq!(v["status"], json!("connected")); + // `auth_hint` is omitted from the wire when absent (skip_serializing_if). + assert!(v.get("auth_hint").is_none()); } }