feat(mcp/ui): expose true_passthrough and oauth_delegate auth types with a no-auth warning - #32414
Conversation
Greptile SummaryExposes
Confidence Score: 5/5Safe to merge. The implementation is correct across all changed paths and is well-covered by new parametrized tests; the only finding is a docstring inaccuracy in the TypedDict field. All correctness fixes are verified by new unit tests (no-persist contract, sessionStorage commit on save, x-mcp header forwarding, no-forward-when-admission-only, gateway authorize redirect). The previously flagged
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/rest_endpoints.py | Extends oauth2-header forwarding to the two new auth types while correctly gating it on the presence of the primary x-litellm-api-key header, preventing the admission credential from being forwarded upstream when Authorization doubled as the admission fallback. |
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Introduces _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES to unify the oauth2/true_passthrough/oauth_delegate discovery trigger, replacing three separate auth_type == MCPAuth.oauth2 checks so the two code paths (config-YAML and DB) cannot drift. |
| litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py | Replaces the inline auth_type != "oauth2" guard in authorize_with_server with a shared _raise_if_not_oauth2 helper that accepts the client-forwarded modes; a deferred import handles the circular dependency with mcp_server_manager. |
| litellm/proxy/_experimental/mcp_server/server.py | Adds _redact_mcp_resource_url (origin-only, strips path/query/userinfo) and threads mcp_auth_mode + mcp_server_resource into StandardLoggingMCPToolCall for audit attribution without credential leakage. |
| litellm/types/utils.py | Adds mcp_auth_mode and mcp_server_resource to StandardLoggingMCPToolCall; the mcp_server_resource docstring incorrectly states "scheme + host + path" when the implementation only logs the origin (path is also stripped). |
| ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx | Adds true_passthrough and oauth_delegate dropdown options, renders the shared TruePassthroughWarning and PassthroughAuthorizeSection, and correctly avoids writing the browser-only token into form.credentials in onTokenReceived for the new modes. |
| ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx | Introduces getEffectiveAuthType() so all in-form decisions use the live form selection rather than the saved record; fixes buildPayload to use values.auth_type, extends onTokenReceived and the save path's sessionStorage branch to cover the new modes, and adds the shared warning and authorize section. |
| ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.tsx | Extends isPassthrough gate with isClientForwardedTokenMode under a unified usesBrowserHeldToken flag so token reading, auth-gate rendering, and listMCPTools enablement all cover the two new auth types consistently. |
| ui/litellm-dashboard/src/components/mcp_tools/TruePassthroughWarning.tsx | New shared component that renders a security warning Alert when true_passthrough is selected; renders nothing for all other auth types. |
| ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx | New shared component for the browser-only Authorize & Fetch flow; optional client ID/secret fields cover IdPs without DCR support, and the component correctly renders null for all other auth types. |
| ui/litellm-dashboard/src/components/mcp_tools/types.tsx | Adds TRUE_PASSTHROUGH and OAUTH_DELEGATE to AUTH_TYPE and exports isClientForwardedTokenMode as the single predicate for the two new modes, preventing drift between the create and edit forms. |
| ui/litellm-dashboard/src/hooks/useTestMCPConnection.tsx | Extends requiresOAuthToken to include browser-held token modes so the connection tester blocks until a session token exists for true_passthrough and oauth_delegate. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_rest_endpoints.py | Adds parametrized tests for client-forwarded mode header extraction and the no-forward case when x-litellm-api-key is absent; existing oauth2 forwarding test correctly updated to add the primary key header to match the new gate. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py | New parametrized tests cover the authorize redirect for client-forwarded modes, the no-persist contract for the token exchange, and a guard test confirming oauth2 DOES persist (making the no-persist assertion meaningful). |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py | Adds parametrized tests for _redact_mcp_resource_url covering credential-in-path, userinfo, query, fragment, port, and edge cases (None, empty, non-URL). |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py | Adds a parametrized test verifying that build_mcp_server_from_table triggers upstream OAuth metadata discovery for both new auth types. |
| ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx | Adds tests for warning rendering, PassthroughAuthorizeSection visibility, and the no-credentials-in-form guard for onTokenReceived in browser-only mode. |
| ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx | Comprehensive new test suite for warning rendering, sessionStorage token persistence on save for both new modes, x-mcp header forwarding, the no-unauthenticated-fetch gate, and the buildPayload regression fix. |
| ui/litellm-dashboard/src/components/mcp_tools/mcp_tools.test.tsx | Adds parametrized tests verifying the auth gate and x-mcp header forwarding for both new auth types in the Tools playground viewer. |
Reviews (13): Last reviewed commit: "refactor(mcp): share _UPSTREAM_OAUTH_DIS..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
5491624 to
9a0b6bb
Compare
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 2 · PR risk: 0/10 |
3df3b59 to
c36ab87
Compare
|
Addressed the recent review findings in c36ab87 and d7152f1 Sensitive upstream URL in logs: Fan-out Authorization bypass: the withhold decision for the client-forwarded modes is computed once and honored in both the forwarding branch and the later Duplicate Authorization headers are rejected with a 400 at the MCP ingress header converter, since a duplicate would otherwise make which forwarded token is used ambiguous (the ASGI header list collapses to last-wins) The "preemptive 401 ignores per-server auth" and "untested duplicate forwarding block" findings were already handled earlier in this stack: the connect-time preemptive 401 for both new modes consults |
|
bugbot run |
d7152f1 to
8aab440
Compare
|
@greptileai rereview |
|
bugbot run |
50923de to
bd2bc06
Compare
|
Addressed both Bugbot findings from the last review Edit OAuth temp auth_type stale (High): getTemporaryPayload in the edit form read the stored auth_type instead of the dropdown selection, so switching an existing oauth2 server to true_passthrough / oauth_delegate and running browser authorize built the temp relay server as oauth2 and persisted the token, breaking the browser-only contract. It now reads values.auth_type, matching onTokenReceived and the submit payload (50923de) Connect gate misses sanitized alias (Medium): the connect-time preemptive 401 matched x-mcp-{alias}-authorization against the raw lowercased alias only, but dashboard clients send the sanitize_mcp_alias_for_header form (alias "pt-server" arrives as header key "pt_server"), which egress resolves via lookup_mcp_server_auth_in_headers. So a per-server token bound with a sanitized alias was forwarded at egress yet still 401'd at connect. The gate now resolves through the same lookup_mcp_server_auth_in_headers, so connect and egress agree. That lives in the base PR #31989 (b2ea36f) since the connect gate moved there; this PR inherits it, with a regression test that a sanitized-alias per-server header skips the preemptive challenge |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bd2bc06. Configure here.
|
@greptileai rereview |
a0c3bfd to
df0e855
Compare
|
@greptileai rereview |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e7cb22e. Configure here.
…through modes The 'auth_type is true_passthrough or oauth_delegate' check was duplicated inline across both server forms' browser-authorize temp payloads, the edit form's onTokenReceived and tool-preview gate, PassthroughAuthorizeSection, and mcp_tools' usesBrowserHeldToken. Extracted a single isClientForwardedTokenMode helper in types.tsx and routed every site through it so the set of client-forwarded modes lives in one place and cannot drift. Also replaced a pre-existing nested ternary in the authorize button label surfaced by touching the file.
…th types The config-YAML loader and the DB loader each defined their own local tuple (oauth2, true_passthrough, oauth_delegate) to decide which auth types trigger upstream OAuth endpoint discovery, under two different names. Hoisted them to a single module constant _UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPES so the two load paths cannot drift on which modes get discovery.
…r the pass-through modes The create form wrote the upstream token obtained by Authorize & Fetch into form.credentials for every mode, so for true_passthrough / oauth_delegate the browser-held token leaked into the OAuth flow's getCredentials (preview requests) and the redirect-persist cache, and was a step away from server-level credential persistence. onTokenReceived now early-returns for the client-forwarded modes, holding the token only in local state for preview (mirroring the edit form), instead of writing it into form.credentials.
The create/edit forms passed several large object literals inline as arguments (persist-state JSON.stringify, storeMCPOAuthUserCredential, setToken, transport-clear setFieldsValue), tripping local/no-large-inline-object-arg. Assigned each to a named variable at the call site - a pure, behavior-preserving refactor verified by the create/edit suites - which lowers the whole-tree count so the eslint baseline is 512 rather than being raised to accommodate them.
2a5a9bd to
a3f1873
Compare
|
@greptileai rereview |
|
bugbot run |
…ctive auth type The edit form decided auth mode from the saved mcpServer.auth_type in fetchTools while the authorize flow used the current form value, so a token authorized after switching the form to a client-forwarded mode was never forwarded as the x-mcp header until the server was saved. A shared getEffectiveAuthType (form value falling back to the saved record) is now the single decision point for token receipt and tool loading The save path classified the staged token with getMcpOAuthMode, which returns null for true_passthrough and oauth_delegate, so the staged token was dropped on save instead of being committed to sessionStorage the way the create form's submit path does. The passthrough branch now also covers the client-forwarded modes; the token still never enters the server row
|
bugbot run |
|
@greptileai rereview |
46a4d6f to
bff2c95
Compare
mateo-berri
left a comment
There was a problem hiding this comment.
create_mcp_server.test.tsx has one vacuously passing assertion; useTestMCPConnection.tsx has an inline auth-type check that should use the shared helper.
Legit greptile concern? Or no?
…okenMode helper The helper extraction missed this call site, leaving an inline duplicate of the two-mode check that could drift from the shared definition
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 43726f2. Configure here.
…ion on the tools preview Authorization doubles as the admission fallback when x-litellm-api-key is absent, so a caller who authenticated the preview request that way had their LiteLLM key forwarded to the upstream as the oauth2/client-forwarded token. The preview now forwards Authorization only when the primary admission header is present, which is how the dashboard has always sent it; with no primary header there is no upstream token on the request at all. Applies to oauth2 and both client-forwarded modes; parametrized regression test plus the admission header added to the existing extraction tests to mirror the real UI request shape
…tadata The redacted resource kept the path, but hosted MCP servers routinely embed the credential in the path (for example /mcp/s/<token>/mcp), and mcp_tool_call_metadata is readable by a caller who can invoke the tool, so the path leaked the upstream credential into spend logs. Only scheme, host, and port are logged now
…y gate and tools preview The gateway authorize/token/register gate and the preview header extraction each carried their own inline copy of the oauth2 + client-forwarded mode set, which could drift from the discovery constant the registry builders use; all three surfaces mean the same thing (modes that run the upstream OAuth browser flow), so they now read the one constant
02157d7 to
d4e02ac
Compare
|
@greptileai rereview |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d4e02ac. Configure here.

Relevant issues
Follow-up to #31989 (stacked on
litellm_mcp_passthrough_delegate_modes; retarget tolitellm_internal_stagingafter it merges)Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Steps to verify in the Admin UI (dev server:
npm run devinui/litellm-dashboard, proxy on localhost:4000):Type
🆕 New Feature
Changes
The parent PR added
true_passthroughandoauth_delegateas first-class MCPauth_typevalues on the backend, but the dashboard's create and edit forms still only offered the legacy dropdown entries, so the only way to select the new modes was the REST API. This adds both values to theAUTH_TYPEconstant and to the Authentication dropdown increate_mcp_server.tsxandmcp_server_edit.tsx, labeled "True Passthrough (no LiteLLM auth)" and "OAuth Delegate (client-supplied upstream token)"Because
true_passthroughturns off admission auth entirely for that server, selecting it now renders a warning Alert directly under the dropdown: anyone who can reach the gateway can call the server without a LiteLLM key, the caller's Authorization header is forwarded to the upstream verbatim, per-key and per-team rate limits and spend tracking do not apply, and the upstream is fully responsible for authenticating callers. The Alert points at OAuth Delegate as the alternative when callers should still authenticate to LiteLLM. The warning lives in one sharedTruePassthroughWarningcomponent rendered by both forms, so the copy and the trigger condition cannot drift between create and edit; it also keeps the two render functions flat, which is what the eslint complexity budget wantsNeither mode requires an authentication value or OAuth credential fields, so the existing
AUTH_TYPES_REQUIRING_AUTH_VALUE/AUTH_TYPES_REQUIRING_CREDENTIALSgating is untouched and no extra fields appear for themThese modes persist no upstream credentials, which used to mean the admin had no way to preview tools or configure the tool allowlist at create/edit time; tools/list simply hit the upstream unauthenticated and 401ed. The second commit closes that gap by reusing the existing OAuth authorize machinery in browser-only mode. Selecting either auth type renders an authorize section: the admin authorizes against the upstream (dynamic client registration and PKCE by default, with optional client ID / client secret fields for IdPs that do not support dynamic registration, e.g. a pre-registered Slack app), the token lands in sessionStorage exactly like the legacy PKCE passthrough path, and the tools preview forwards it via the per-server x-mcp-{alias}-authorization header that the passthrough resolver arm already accepts. Nothing is written to the server row or the per-user credential store: the create payload keeps excluding credentials for these auth types, and the parent PR's behavior contract (server-level persist = nothing) still holds. The tool allowlist configured this way does persist, since it is server configuration rather than a credential. On the backend, the tools preview endpoint now also extracts the Authorization header for the two new auth types so a create-time preview reaches the passthrough arm authenticated
The server detail page's Tool Testing Playground had the same blind spot: it gated browser-held token handling on the legacy PKCE passthrough shape, so a server in either new mode listed tools unauthenticated and showed "Failed to fetch MCP tools" with no way to authorize. The third commit extends the playground's gate to both modes (read the sessionStorage token, forward it via the x-mcp header, evict on 401, show the playground's own Authorize gate when absent) and unblocks the gateway's relayed authorize/register/token endpoints for them; previously those endpoints 400ed for anything but oauth2, which would have sent the playground's Authorize button to an error page. Registry builds now run the same RFC 9728/8414 endpoint discovery for these modes that oauth2 rows get, since their rows never store an authorization_url. DCR persistence stays off on this path and the minted token remains browser-held
The edit form keyed two browser-held token decisions off the saved record instead of the admin's in-flight form selection (a review round caught both): fetchTools decided whether to forward the sessionStorage token from mcpServer.auth_type, so a token authorized right after switching the form to a client-forwarded mode was not used for tool loading until the server was saved, and the save path classified the staged token with getMcpOAuthMode, which returns null for the new modes, so the token was dropped on save instead of committed to sessionStorage the way the create form's submit does. A shared getEffectiveAuthType (form value falling back to the saved record) is now the single decision point for receipt and tool loading, and the save path's sessionStorage branch covers the client-forwarded modes; the token still never enters the server row
The tools preview endpoint forwarded the raw Authorization header upstream for oauth2 and the client-forwarded modes, but Authorization is also the admission fallback when x-litellm-api-key is absent, so a caller who authenticated that way had their LiteLLM key forwarded to the upstream. The preview now forwards Authorization only when the primary admission header is present, which is how the dashboard has always sent the pair
Tests: the create form suite covers the warning appearing when True Passthrough is selected and staying hidden for OAuth Delegate, the authorize section rendering for both new modes and not for API Key; the edit form suite covers the warning rendering for a stored
true_passthroughserver and not foroauth_delegate, the sessionStorage token being forwarded as the x-mcp header for anoauth_delegateserver, and the authorize hint plus no unauthenticated fetch when atrue_passthroughserver has no browser token; the backend suite covers the preview endpoint extracting the Authorization header for both new auth types, the authorize endpoint redirecting to the upstream IdP for both modes, and registry builds discovering upstream OAuth endpoints for them; the playground suite covers the Authorize gate and the x-mcp header forwarding for both modesTwo further correctness fixes came out of an audit of the modes against gateway and OAuth norms. The preemptive-401 connect gate for both modes only inspected the request-wide Authorization, so a caller who bound the upstream token via the per-server
x-mcp-{alias}-authorizationheader (the required shape in a multi-server aggregate, where the request-wide Authorization is withheld) was spuriously 401'd at connect even though egress already honored that header; the gate now recognizes the per-server header mode-correctly (true_passthrough treats any Authorization or the per-server header as the upstream token; oauth_delegate still requires a distinctx-litellm-api-keyso a lone Authorization consumed for admission is never mistaken for an upstream token), and the preemptive raise is gated to single-server scopes so a multi-server aggregate degrades gracefully instead of one missing token failing the whole connect. Separately, the browser-only Authorize was writing the upstream access and refresh token toLiteLLM_MCPUserCredentialsbecause the temporary OAuth-relay server was cached with a hardcoded oauth2 auth_type; the forms now send the real auth_type for these modes, soneeds_user_oauth_tokenis false and the exchange skips storage while still returning the token to the browser session, keeping the persist-nothing contract. The tool-call logs also carrymcp_auth_modeandmcp_server_resourcenow so a relayed request can be attributed in an audit to its mode and upstream target without logging any credential; the logged resource is the origin only (scheme, host, port), because hosted MCP servers routinely embed the credential in the URL pathDeliberately out of scope for these PRs, tracked as passthrough v2 extensions so they are not re-filed as gaps. Faithful relay of a mid-session upstream 401/403 on the tool-CALL path (today it becomes a JSON-RPC isError; the REST tools/list path already relays and the aggregate already degrades to an empty list); a per-alias needs-reauth signal on the multi-server aggregate so a client can re-auth just the one server whose token expired (v1 absorbs a failing server to an empty list without naming it); a connect-time upstream probe for a present-but-revoked token (a stateless mode forwards and lets the upstream reject, so this is a UX nicety with a per-connect round-trip cost, not a correctness fix); an admission-credential-stripped attestation field on the audit log (needs the strip decision threaded from header-prep); documentation steering audience-locked tokens to
oauth2_token_exchangerather than passthrough (forwarding a mis-audienced token and letting the upstream reject it is the correct confused-deputy-safe behavior, so the work is docs, not code); and Authorization header hygiene (duplicate/multi-scheme rejection, size caps) which is a pre-existing v1-wide concern rather than something these modes introduced.Note
Medium Risk
Changes touch OAuth relay, token persistence boundaries, and header forwarding on auth-critical MCP paths;
true_passthroughintentionally weakens LiteLLM admission, which increases exposure if misconfigured.Overview
Exposes
true_passthroughandoauth_delegatein the MCP server create/edit UI, with a True Passthrough warning, a browser-only Authorize & Fetch flow (optional client ID/secret, token in sessionStorage only), and the same Authorize gate in the tools playground.On the proxy, shared
_UPSTREAM_OAUTH_DISCOVERY_AUTH_TYPESdrives upstream OAuth metadata discovery for these modes (not onlyoauth2). The gateway authorize/token/register paths allow them via_raise_if_not_oauth2, andauthorize_with_serveruses that guard. Tools preview forwards OAuth headers for client-forwarded modes only whenx-litellm-api-keyis present so a LiteLLM key inAuthorizationis not sent upstream. Tool-call logging addsmcp_auth_modeand origin-onlymcp_server_resourcevia_redact_mcp_resource_url.Tests cover authorize redirects, no DB persist on token exchange for client-forwarded modes, discovery on registry build, preview header behavior, URL redaction, and dashboard flows.
Reviewed by Cursor Bugbot for commit d4e02ac. Bugbot is set up for automated code reviews on this repo. Configure here.