fix(mcp): rework the dashboard credential-field lifecycle so the OAuth app is upstream-scoped

[tin-berri]

Relevant issues

Follow-up to #32735

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

• I have added meaningful tests
• My PR passes all CI/CD checks (e.g., lint, format, unit tests)
• My PR's scope is as isolated as possible; it only solves 1 specific problem
• I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to 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

Live proxy on localhost:4000 backed by real Postgres, running real dynamic client registration against Linear's authorization server; no mocks. Both runs captured at 931b617 (the change is UI payload assembly; the two servers below reproduce the exact payloads the dashboard sends before and after this change)

First, a stand-in for the org's pre-registered app, minted once directly at Linear:

$ curl -s https://mcp.linear.app/.well-known/oauth-authorization-server | jq -r .registration_endpoint
https://mcp.linear.app/register
$ curl -s -X POST https://mcp.linear.app/register -H 'Content-Type: application/json' \
    -d '{"client_name":"LiteLLM Org App (demo)","redirect_uris":["http://localhost:4000/callback"],"grant_types":["authorization_code","refresh_token"],"response_types":["code"],"token_endpoint_auth_method":"none"}'
{"client_id": "aTk6vD...", ...}

Before this change the dashboard stripped credentials for these modes, so every true_passthrough/oauth_delegate create looked like linear_pt_blank below; after, admin-entered credentials survive into the payload, producing linear_pt_orgapp:

$ curl -s -X POST http://localhost:4000/v1/mcp/server -H 'x-litellm-api-key: sk-1234' -H 'Content-Type: application/json' \
    -d '{"server_name":"linear_pt_orgapp","url":"https://mcp.linear.app/mcp","transport":"http","auth_type":"true_passthrough","credentials":{"client_id":"aTk6vD..."}}'
$ curl -s -X POST http://localhost:4000/v1/mcp/server -H 'x-litellm-api-key: sk-1234' -H 'Content-Type: application/json' \
    -d '{"server_name":"linear_pt_blank","url":"https://mcp.linear.app/mcp","transport":"http","auth_type":"true_passthrough"}'

$ psql -d litellm_cfg_repro -c "SELECT alias, auth_type, credentials ? 'client_id' AS has_configured_client FROM \"LiteLLM_MCPServerTable\" ORDER BY created_at;"
 linear_pt_orgapp | true_passthrough | t
 linear_pt_blank  | true_passthrough | f

The internal user's Tools-page Authorize (register, then the authorize relay):

$ curl -s -X POST 'http://localhost:4000/v1/mcp/server/oauth/<orgapp-id>/register' ... 
{'client_id': '<server-id placeholder>', 'client_secret': 'dummy'}     <- short-circuit; no upstream DCR call

$ curl -s -X POST 'http://localhost:4000/v1/mcp/server/oauth/<blank-id>/register' ...
{'client_id_prefix': 'mwDn6f...', 'is_dummy': False}                   <- DCR fallback still mints a fresh Linear client

$ curl -s -o /dev/null -w '%{redirect_url}' 'http://localhost:4000/v1/mcp/server/oauth/<orgapp-id>/authorize?redirect_uri=...&state=...&code_challenge=...&response_type=code&client_id=<placeholder>' -H 'Authorization: Bearer sk-1234'
-> https://mcp.linear.app/authorize?...client_id=aTk6vD...&scope=read+write
   client_id_is_org_app: True     <- the relay substitutes the configured app server-side; the browser never needs it

Token contract unchanged, checked after the flows:

$ psql -d litellm_cfg_repro -c "SELECT count(*) AS stored_user_tokens FROM \"LiteLLM_MCPUserCredentials\";"
 0

Durability of the persisted app, also at 931b617: the credentials are encrypted at rest, survive a full proxy restart (decrypt-on-load through the registry), and the edit path persists the same way

$ psql -d litellm_cfg_verify -tAc "SELECT credentials::text FROM \"LiteLLM_MCPServerTable\" WHERE server_id='<id>';"
{'keys': ['client_id', 'client_secret'], 'client_id_plaintext_in_db': False, 'secret_plaintext_in_db': False}

# kill proxy, relaunch, same authorize relay call:
{'redirect_host': 'mcp.linear.app', 'client_id_is_org_app_after_restart': True}

# blank server, then PUT /v1/mcp/server with credentials.client_id (the edit form's payload):
update HTTP:202 ; row has client: t
{'relay_uses_org_app_after_edit': True}

Type

🆕 New Feature

Changes

The create and edit forms render optional OAuth Client ID / Client Secret fields for the client-forwarded token modes (true_passthrough / oauth_delegate), but AUTH_TYPES_REQUIRING_CREDENTIALS excluded those modes, so the submit path stripped the values and the server row never got a client. The consequence for internal users: the Tools-page Authorize has no client input, so on upstreams without dynamic client registration (a pre-registered Slack app being the canonical case) the flow dead-ends for everyone except the admin who typed the app into the form for that one browser session; on DCR-capable upstreams every authorize minted a throwaway client, invisible to any org-level scope or consent policy that keys off a stable app identity

This PR adds the two modes to AUTH_TYPES_REQUIRING_CREDENTIALS in both forms so admin-entered credentials persist as declared config, and updates the copy from "(optional, not saved)" to "(optional, saved)" with the surrounding text now distinguishing the two lifetimes: the browser-authorized access token is still never saved anywhere, while a configured app is stored with the server so internal users authorize through it. Empty fields keep producing no credentials in the payload, so leave-blank still means the DCR fallback

No backend changes are needed: create/update already persist and encrypt credentials for any auth_type, register_client_with_server short-circuits when the server has a client_id, and both the authorize and token relays substitute the stored client server-side, so the Tools-page flow picks the configured app up with zero further wiring (proven live above). The client secret never reaches the browser; register returns a placeholder and the relays authenticate upstream with the stored secret. Mode classification is unaffected: has_client_credentials keys off oauth2_flow (never stamped here) and all passthrough predicates key off auth_type alone. This composes with #32735, which stops runtime-DCR-minted clients from being persisted; together they make "configured" mean exactly "an admin declared it"

Tests: parametrized create and edit tests asserting admin-entered credentials ARE included in the submit payload for both modes while the browser-held token still appears nowhere and no per-user credential is written (both fail with the gate reverted; mutation-checked), plus the existing browser-held-token tests keep passing unchanged, pinning that blank fields still omit credentials

Follow-up commit cf1b407 addresses the review finding on blank-field semantics: the backend's partial credential update merges per key, so blank fields on edit keep whatever app is stored, while the section copy claimed blank meant dynamic client registration. The copy now follows the same convention the M2M credential fields already use (create: "leave blank to use dynamic client registration"; edit: "leave blank to keep the currently saved app"), and removing a stored app is an explicit checkbox on the edit form that saves an explicit-null credential write: MCPCredentials permits null values, encrypt_credentials skips nulls, and the merge overrides the stored keys, returning the server to dynamic client registration with zero backend changes. Live proof at cf1b407: create a true_passthrough server with an app (row shows client_id/secret set: true true), send the removal write (PUT credentials {client_id: null, client_secret: null}, HTTP 202), the row flips to false false, and the register endpoint stops short-circuiting and mints a fresh DCR client at Linear. The removal path is pinned by a test asserting the checkbox produces exactly the explicit-null payload (fails with the save branch disabled), and the existing blank-save test keeps asserting that untouched fields omit credentials entirely

Follow-up commit 57051d3 addresses the review finding that the OAuth invalidation path silently wiped the declared app: clearHeldOAuthToken resets the shared CLEARED_ON_INVALIDATION fields whenever the authorization identity changes (e.g. a URL edit after Authorize), which was correct when form.credentials only ever held minted material but now discarded admin config for the client-forwarded modes. A shared preservedDeclaredAppCredentials helper (types.tsx, used by both forms) re-applies the declared app across the reset with two guards that keep the original wipe semantics intact everywhere they matter: the preserve never applies when auth_type itself changed, because the previous mode's onTokenReceived may have written a fetched token or DCR client into the same field (switching oauth2 to true_passthrough with a stale DCR client in form.credentials would otherwise persist minted material onto the row), and it only ever keeps the client_id/client_secret keys so token-shaped values cannot ride through under any sequence. Regression tests cover both directions and are mutation-checked: disabling the preserve fails the three URL-change-after-authorize tests (create + both edit modes), and dropping the auth_type guard fails the oauth2-to-passthrough switch test that asserts the DCR-minted client is wiped from the payload

---

Note

Medium Risk
Changes OAuth credential assembly and invalidation in admin forms; mistakes could persist wrong clients or leak tokens into server rows, but behavior is heavily test-covered and scoped to dashboard UI.

Overview
Client-forwarded MCP modes (true_passthrough / oauth_delegate) now treat optional OAuth client ID/secret as saved server config (not browser-only), while access tokens stay session-only. Create/edit include these modes in AUTH_TYPES_REQUIRING_CREDENTIALS so admin-entered apps reach the API; copy and labels change from “not saved” to “saved”.

Shared helpers in types.tsx (preservedDeclaredAppCredentials, withoutMintedTokenCredentials, credentialAuthClass) separate declared app from minted token/DCR material. OAuth2 DCR clients live in a ref instead of form.credentials, so they are not mistaken for a passthrough app or leaked on auth-type switches. Invalidation and OAuth redirect restore re-apply the declared app after clearing tokens; submit paths strip tokens for client-forwarded rows.

PassthroughAuthorizeSection adds edit-aware placeholders (keep existing vs discard on cross-class auth switch), an upstream-change warning, and a remove saved app checkbox that sends explicit null credentials on edit. Create/edit forms add upstream URL change warnings, safer server-switch state reset, and broader tests for payloads, invalidation, and restore behavior.

^{Reviewed by Cursor Bugbot for commit 5dbd608. Bugbot is set up for automated code reviews on this repo. Configure here.}

[greptile-apps]

Greptile Summary

This PR reworks the MCP dashboard credential lifecycle for true_passthrough and oauth_delegate modes so that admin-entered OAuth client IDs and secrets are saved as server config (enabling Tools-page authorize to relay through a declared org app), while browser-authorized access tokens remain strictly session-scoped and are never written to the server row.

• Adds TRUE_PASSTHROUGH and OAUTH_DELEGATE to AUTH_TYPES_REQUIRING_CREDENTIALS in both forms; introduces preservedDeclaredAppCredentials / withoutMintedTokenCredentials helpers applied at every write site (submit, onTokenReceived, clearHeldOAuthToken, OAuth-resume restore) to guarantee only declared app keys reach the server row.
• Moves the DCR-minted OAuth client out of form.credentials into a dcrClientRef so it can never be misclassified as a declared passthrough app; adds an explicit-null "remove saved app" checkbox on the edit form.
• Fixes both previously-flagged issues: removeStoredApp is now reset on server switch, and admin-entered credentials are preserved through clearHeldOAuthToken instead of being wiped on URL/endpoint changes after authorize.

Confidence Score: 5/5

Safe to merge; both previously-flagged issues are fixed with mutation-checked tests, and token-stripping helpers are applied consistently at every write site.

Both previously-flagged issues are resolved. The token-stripping helpers are applied consistently at every write site across both forms. Remaining observations are a UX warning gap and a stale duplicate comment block, neither of which affects correctness or security.

No files require special attention; both forms apply the credential helpers symmetrically.

Important Files Changed

Filename	Overview
ui/litellm-dashboard/src/components/mcp_tools/types.tsx	Adds three new pure helpers: preservedDeclaredAppCredentials, withoutMintedTokenCredentials, and credentialAuthClass; also adds credentials to the MCPServer interface with a redaction note.
ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx	Adds TRUE_PASSTHROUGH/OAUTH_DELEGATE to AUTH_TYPES_REQUIRING_CREDENTIALS; introduces dcrClientRef; patches clearHeldOAuthToken to re-apply declared app after reset. Contains a duplicate comment block at lines 747-752.
ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx	Mirrors create-form changes; resets removeStoredApp and appMayNotMatchUpstream on server switch (fixes previous issue); adds explicit-null removal path.
ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx	Adds isEditing/savedAuthType/removeStoredApp/appMayNotMatchUpstream props; credential-class-aware placeholder copy; Remove saved OAuth app checkbox.
ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx	Extensive new parametrized tests for credential persistence, auth-type switches, DCR-client isolation, URL-change preservation, and re-authorize stale-sibling cleanup.
ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx	New tests for credential persistence, URL-change preserve, explicit-null removal, server-switch checkbox reset, and OAuth-resume restore.
ui/litellm-dashboard/src/components/mcp_tools/types.test.tsx	Unit tests for the three new helpers with well-specified edge cases.
ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.test.tsx	New test file covering credential-class-aware placeholder/copy scenarios.

_{Reviews (10): Last reviewed commit: "fix(mcp): reset the remove-app checkbox ..." | Re-trigger Greptile}

[codspeed-hq]

Merging this PR will not alter performance

✅ 31 untouched benchmarks

---

_{Comparing litellm_mcp_configured_client_for_passthrough (5dbd608) with litellm_internal_staging (dacf1cf)}

[tin-berri]

bugbot run

[tin-berri]

@greptileai rereview

[tin-berri]

Both findings addressed with real commits

57051d3 fixes the P1: clearHeldOAuthToken no longer wipes the declared app on an intra-mode identity change. A shared preservedDeclaredAppCredentials helper (types.tsx, used by both forms) re-applies the admin-entered client_id/client_secret across the CLEARED_ON_INVALIDATION reset, with two guards that keep the original wipe wherever minted material can exist: the preserve never applies when auth_type itself changed (switching oauth2 to a client-forwarded mode with a stale DCR client in form.credentials would otherwise persist minted material onto the row), and it only keeps the two declared-app keys so token-shaped values cannot ride through under any sequence. Regression tests cover both directions and were mutation-checked: disabling the preserve fails the three URL-change-after-authorize tests, and dropping the auth_type guard fails the oauth2-to-passthrough switch test

cf1b407 fixes the blank-fields finding: the copy now states the keep-existing convention the backend merge actually implements (matching the M2M fields' "leave blank to keep existing" pattern; create keeps the dynamic-client-registration wording, which is true there), and removing a stored app is an explicit checkbox on the edit form that saves credentials {client_id: null, client_secret: null}; MCPCredentials allows nulls, encrypt_credentials skips them, and the merge overrides the stored keys. Verified live against a real proxy: the row's client flips to absent and the register endpoint goes back to minting a fresh DCR client at Linear. Pinned by a payload test that fails with the save branch disabled

@greptileai

[greptile-apps]

Both follow-up commits are solid. Here's the verification:

57051d3 (preservedDeclaredAppCredentials) — the helper is correct:

• The two guards (authTypeChanged early return, DECLARED_APP_CREDENTIAL_KEYS allowlist) are exactly the right shape. The authTypeChanged guard correctly reads "auth_type" in changedValues from the form's onValuesChange payload rather than comparing current vs previous values, which is reliable because antd only includes actually-changed keys in that object.
• Both forms call the preserve before form.resetFields([...CLEARED_ON_INVALIDATION]) and conditionally setFieldsValue after, which is the correct ordering (reset then re-apply, rather than trying to patch before reset).
• Mutation-tested in three directions (URL-change-after-authorize for create + both edit modes, and the oauth2→passthrough switch test for the guard) — that covers the two independent invariants.

cf1b407 (blank-field semantics + explicit removal) — clean:

• The isEditing / removeStoredApp / onRemoveStoredAppChange props on PassthroughAuthorizeSection are minimal: the checkbox only renders when all three conditions hold (isEditing && onRemoveStoredAppChange in JSX), which prevents an orphaned checkbox on the create form.
• The explicit-null override in mcp_server_edit.tsx defensively re-checks isClientForwardedTokenMode(restValues.auth_type) even though the checkbox is gated the same way — fine, belt-and-suspenders is appropriate here given the credential write goes to the DB.
• The blankMeaning placeholder swap (create: "dynamic client registration" vs edit: "keep existing") matches the backend's actual merge semantics and aligns with how the M2M credential fields already document the convention.

One minor note: eslint-metrics.json ticks up by 2 on local/no-large-inline-object-arg (512→514). If the repo treats this budget as a ratchet, worth a quick check that neither of the two new inline objects can be trivially extracted, but this is a low-severity housekeeping item, not a blocker.

Everything else is well-addressed. Confidence Score: 5/5. The feature is ready for maintainer review.

[tin-berri]

@greptileai rereview

[tin-berri]

bugbot run

[tin-berri]

Reworked the whole credential-field lifecycle instead of patching the reported symptoms one by one. A fan-out audit of every state transition surfaced 13 bugs (2 high, 5 medium, 6 low), including two nobody had flagged, and this commit (f1f8bfc) implements one coherent design for them.

The organizing principle, from the review discussion: the upstream OAuth app (client_id/client_secret) is a property of the UPSTREAM, not the gateway mode, so it must survive an auth-type switch and be invalidated only by a real upstream change or an explicit new client. The two high-severity findings both violated that. First, the backend update_mcp_server treated any auth_type change as "replace credentials", so a true_passthrough to oauth_delegate switch silently wiped the stored app server-side (no frontend fix could reach it, since the GET redacts secrets so the edit form has nothing to resend); it now classifies both client-forwarded modes as one credential class and merges within it, replacing only across classes. Second, an abandoned interactive re-authorize could rehydrate a DCR-minted client into form.credentials and persist it onto a client-forwarded row; the DCR client is now held in a ref and never enters the form store, which structurally removes that leak class rather than filtering it after the fact.

On top of that: the invalidation preserve is now a pure declared-app filter (no mode/transition guard), so the app survives client-forwarded<->client-forwarded switches, round trips, and url/endpoint edits; a url/endpoint change keeps the typed app and shows a non-blocking "may not match the new upstream" warning (the keep-and-warn behavior chosen in review) rather than silently wiping it; onTokenReceived path-replaces the credentials so a re-authorize with fewer token fields leaves no stale siblings; submit and temp-preview payloads for the client-forwarded modes are filtered to declared keys as an independent backstop; restored sessionStorage snapshots strip minted token keys; and the section copy is credential-class-aware. A live-proxy run also surfaced a pre-existing prisma-python issue (a raw None for the Json? credentials field raises "value is required but not set"); the clear path now writes Json(None), verified live that a cross-class switch clears the app without the 500 it hit before.

Coverage: 154 frontend tests plus 26 backend, mutation-checked at each seam (disabling the preserve fails the keep tests; leaking the DCR into the form fails the no-persist test; the raw-inequality backend predicate fails the class-merge tests; dropping the minted-strip fails the rotation test). Live-verified against a real proxy and Postgres: true_passthrough to oauth_delegate keeps the app and the authorize relay still uses it, a cross-class switch clears it, explicit removal falls back to DCR, and a configured app short-circuits registration.

@greptileai

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[tin-berri]

Scope note (2026-07-10): this PR was split. The backend credential-class merge (the true_passthrough <-> oauth_delegate server-side keep, the prisma Json(None) clear, the client-rotation minted-key strip) now lives in its companion PR #32815 and should merge first; this PR is now the dashboard half only. The dashboard's keep-existing copy for a within-class switch is accurate once #32815 is in place. All backend files were removed from this branch; the diff here is frontend-only (8 files under ui/litellm-dashboard).

The dashboard rework makes the OAuth app upstream-scoped in the forms: the DCR-minted client is held in a ref and never enters form.credentials (so it can never be collected as a client-forwarded server's declared app), the invalidation preserve is a pure declared-app filter so the typed app survives mode switches and round trips, a url/endpoint change keeps the app and shows a non-blocking "may not match the new upstream" warning rather than wiping it, onTokenReceived path-replaces the token set, submit and temp-preview payloads for the client-forwarded modes are filtered to declared keys, restored sessionStorage snapshots strip minted token keys, and the section copy is credential-class-aware. 154 frontend tests, mutation-checked.

[tin-berri]

@greptileai

[tin-berri]

bugbot run

[tin-berri]

Thanks Bugbot; all three findings on the current head are addressed in 1e78e96, each with a regression test.

Modal close retains DCR client (High): the DCR ref was cleared in handleCancel but not in the modal-close effect, so a parent dismiss (or submit-then-close) left it set and the next oauth2 create could carry the prior server's registered client. The modal-close effect now clears dcrClientRef.current and setAppMayNotMatchUpstream(false), matching handleCancel; a test renders, authorizes to set the ref, rerenders with isModalVisible=false, and asserts getCredentials and the temp payload no longer carry the client (fails if the clear is reverted).

Temp payload omits DCR ref (Medium): getTemporaryPayload now merges dcrClientRef for the oauth2 case, mirroring getCredentials, so a re-authorize reuses the registered client instead of re-registering; the DCR-ref test asserts getTemporaryPayload().credentials includes the client (fails if the merge is dropped).

Stale upstream warning after close (Low): same modal-close effect now resets appMayNotMatchUpstream, so the amber banner never persists into a fresh session.

The two older findings (CF mode switch drops app; blank fields keep stored app) were on superseded commits and are resolved on head: the invalidation preserve keeps the declared app across a true_passthrough <-> oauth_delegate switch, and blank-on-edit is the intended keep-existing convention with class-aware copy plus an explicit removal checkbox that writes an explicit-null credential.

[tin-berri]

bugbot run

[tin-berri]

@greptileai rereview

[cursor]

✅ 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 1e78e96. Configure here.}

[mateo-berri]

LGTM; thanks!

[mateo-berri]

@greptileai

let me run greptile one more time

[mateo-berri]

mcp_server_edit.tsx and types.tsx: the edit form silently omits the upstream-mismatch warning, and withoutMintedTokenCredentials returning an empty object can leave the edit form restore path showing blank credential fields

Are either of these greptile concerns legit?

[tin-berri]

bugbot run

[cursor]

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

[tin-berri]

@greptileai rereview

[mateo-berri]

bugbot run

[tin-berri]

bugbot run

[tin-berri]

@greptileai rereview

[cursor]

✅ 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 8650677. Configure here.}

[tin-berri]

@greptileai rereview

[tin-berri]

bugbot run

[cursor]

✅ 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 5dbd608. Configure here.}

[tin-berri]

Two minor follow-ups noted from review, neither affects correctness, security, or the persisted data; leaving them for a later pass rather than expanding this PR

Stale duplicate comment in create_mcp_server.tsx handleFormValuesChange: after the warning-dismiss logic was un-gated from the stale-token check, the explanatory comment ended up duplicated; the earlier block is a leftover and can be dropped

UX warning gap on edit: the upstream-mismatch warning keys off the form's credentials, which the server GET redacts to null, so changing the URL on a server that has a stored (redacted) app does not surface the warning. There is no data loss (blank means keep-existing, so the stored app is preserved and never wiped); closing the gap needs a non-secret "has configured client" signal on the GET response, so it is a small backend follow-up rather than part of this PR

[mateo-berri]

LGTM; thanks!