fix(mcp): persist DCR client_id so interactive OAuth token refresh works

[tin-berri]

Relevant issues

Linear ticket

LIT-4154 https://linear.app/litellm-ai/issue/LIT-4154

This PR covers variant 1 (post-create authorize). Variant 2 (on-create "Authorize and Fetch") is tracked in the same ticket and handled in a stacked follow-up PR.

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 requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Screenshots / Proof of Fix

An interactive authorization_code MCP server registers an OAuth client via Dynamic Client Registration (RFC 7591) during the authorize flow. The minted client_id is not re-derivable from discovery, so if it is not persisted the autonomous refresh_token grant has no client identity and the upstream rejects the refresh. The user is silently forced to re-authorize and tools/list returns zero tools.

Live proxy on localhost:4000 backed by real Postgres, authorizing against a real upstream MCP server's OAuth (its token endpoint is hit for real).

Before (this fix absent), the server row stores none of the client identity and the refresh is rejected:

# server row: empty credentials, no token_url
$ docker exec db psql ... -c "select alias, credentials::text, token_url from \"LiteLLM_MCPServerTable\""
 linear | {} | (null)

# expire the stored access token, then list tools as the owning user
$ curl -s "http://localhost:4000/mcp-rest/tools/list?server_id=$SID" -H "Authorization: Bearer $KEY"
{"tools": [], "error": null, "message": "Successfully retrieved tools"}     # 0 tools

# proxy log: refresh POST is rejected because no client_id is sent
refresh_user_oauth_token: refresh request failed for user=default_user_id server=...:
  Client error '401 Unauthorized' for url 'https://<upstream>/token'

After (this fix), authorizing the same server persists the client identity and the refresh succeeds:

# server row now carries an encrypted client_id and the discovered token_url
$ docker exec db psql ... -c "select alias, (credentials ? 'client_id') as has_client_id, token_url from \"LiteLLM_MCPServerTable\""
 linear | t | https://<upstream>/token

# expire the stored access token, then list tools
$ curl -s "http://localhost:4000/mcp-rest/tools/list?server_id=$SID" -H "Authorization: Bearer $KEY"
{"tools": [ ... 55 tools ... ], "error": null, "message": "Successfully retrieved tools"}    # 55 tools

# proxy log: refresh succeeded and the rotated token was persisted (fresh expires_at)
refresh_user_oauth_token: refreshed token for user=default_user_id server=...

The same server-side fields feed both refreshers (v1 refresh_user_oauth_token and the v2 AuthorizationCodeRefresher), so once the row carries client_id and token_url, both authenticate with no egress-side change.

Scope: this covers the flow where a server already exists in the DB and is authorized from the tools page. The separate "Authorize and Fetch" on-create flow runs DCR against a temporary in-memory server that has no DB row and creates the real server afterward, so it needs its own link between the authorize session and the create; that is a known follow-up and is intentionally out of scope here.

Type

Bug Fix

Changes

register_client_with_server performs the upstream DCR call and previously returned the registration response straight to the caller without persisting it. This adds _persist_dcr_client_registration, which validates the RFC 7591 response with a typed pydantic model and writes client_id (plus client_secret when the registration returns one, and token_endpoint_auth_method only when it is client_secret_basic, since the token-endpoint auth already defaults to client_secret_post when unset) together with the discovered token_url onto the server row. token_url is included in the partial update only when the in-memory server already carries it, so the update never overwrites an existing value with NULL. It reuses the existing encrypt_credentials write that client_credentials and token exchange already use, so the secrets are encrypted at rest the same way, and it refreshes the in-memory registry via update_server so the value is live at the next refresh rather than only after a reload. Persistence failures are logged and never raised, so an authorize still returns to the caller if the write fails.

Regression tests in test_discoverable_endpoints.py assert the DCR response is persisted with the right client_id / client_secret / token_endpoint_auth_method and token_url, and that token_url is omitted from the update when the server has none; they fail on the pre-fix code and pass with the fix.

Beyond persisting the client, the register guard now reuses an existing client_id instead of re-registering per authorize (it previously re-ran DCR unless both client_id and client_secret were present, so public clients minted a fresh client on every authorize). One client per server, shared across users, is the OAuth standard; re-minting would orphan earlier users' refresh tokens. Confidential and config-defined servers already reused their configured client. A regression test asserts a server that already has a client_id is reused without an outbound DCR call.

Persistence is gated to a full proxy admin. register_client_with_server writes to the server row only when mcp_register passes persist_credentials=True, which it does only for a PROXY_ADMIN caller (_user_is_full_admin); the unauthenticated root /register route and any non-admin management caller register and return without persisting. Establishing the one shared client per server is an admin action and users only authorize and reuse it, so a non-admin, even one with access to the server, cannot bind a caller-controlled client, whose secret it also receives, onto the shared row before the admin sets one up. The reuse guard additionally blocks overwriting an existing client. Regression tests assert the public route and a non-admin management caller do not persist while a full admin does.

Config.yaml-defined servers are unaffected and are not the target of this change. They live in memory only (config_mcp_servers), never in LiteLLM_MCPServerTable, and config is reload-authoritative, so their client_id / client_secret must be declared in YAML; token_url / authorization_url are discovered (RFC 9728 then 8414) so those are optional. A config oauth2 server that omits client_id and leans on DCR will authorize once with an ephemeral client_id but cannot refresh, since nothing reload-safe holds that client_id. This persistence fix applies only to servers that have a DB row (UI-created), where the gateway can save the DCR-minted client identity.

---

Note

Medium Risk
Changes OAuth client identity storage and registration guards on the proxy; persistence is admin-gated but mishandling could still affect shared refresh behavior across users.

Overview
Fixes MCP interactive OAuth so refresh_token grants can authenticate after access tokens expire, by persisting Dynamic Client Registration (RFC 7591) results on DB-backed MCP servers instead of discarding them after register_client_with_server.

Persistence: When persist_credentials=True, the gateway validates the upstream DCR response, writes encrypted client_id / optional client_secret / token_endpoint_auth_method (and token_url only when already known) via the existing update_mcp_server path, and refreshes the in-memory registry. Failures are logged only; registration still returns to the caller.

Reuse / no re-DCR: Registration now short-circuits if the server already has a client_id (not only when both id and secret were set), and can load and apply credentials from the DB when the registry is stale—skipping outbound DCR and returning the dummy client response when appropriate. Concurrent admin persist can detect an existing row and return reused instead of overwriting.

Who may persist: Only the authenticated management mcp_register path passes persist_credentials=True for full proxy admins; the public /register route and non-admin callers register without writing shared server credentials.

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

[tin-berri]

@greptileai

[greptile-apps]

Greptile Summary

This PR persists the Dynamic Client Registration (RFC 7591) client_id onto the MCP server's DB row so that autonomous refresh_token grants can authenticate as the registered OAuth client. Previously, DCR issued a fresh client_id on every authorize call and discarded it, causing token refresh to fail after access token expiry.

• Admin-gated persistence: register_client_with_server gains a persist_credentials flag; the management /register endpoint sets it only for PROXY_ADMIN callers, so unauthenticated and non-admin routes never bind a caller-supplied DCR client to the shared server row.
• Reuse-before-re-DCR: The function now short-circuits when mcp_server.client_id is already set in memory, and also checks the DB (with a second check inside _persist_dcr_client_registration) to handle the race where a concurrent admin authorized between the in-memory check and the DCR call.
• Safe partial update: token_url is only included in the UpdateMCPServerRequest when the in-memory server carries it, preventing a conditional-spread None from nulling an existing DB column. Credentials are encrypted at rest via the existing encrypt_credentials path and merged (not replaced) with any prior credential fields via update_mcp_server's merge logic.

Confidence Score: 5/5

Safe to merge — the admin-only gate is correct, credentials are encrypted via the existing path, the conditional token_url spread prevents NULL overwrites, and the double-check pattern adequately mitigates concurrent-persist races.

The persistence logic correctly gates writes behind PROXY_ADMIN, reuses the existing encrypt_credentials path, and the update_mcp_server merge semantics ensure new DCR credentials don't wipe unrelated existing fields. No auth bypass, data loss, or incorrect credential exposure was found in the changed paths.

No files require special attention.

Important Files Changed

Filename	Overview
litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py	Adds _DcrClientRegistration, _PersistedDcrCredentials, and helper functions for persisting and reusing DCR client registrations; introduces persist_credentials flag to register_client_with_server with correct gating logic
litellm/proxy/management_endpoints/mcp_management_endpoints.py	Minimal one-line change: passes persist_credentials=_user_is_full_admin(user_api_key_dict) to register_client_with_server, correctly limiting persistence to PROXY_ADMIN callers
tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py	Adds 7 new mock-based tests covering DCR persistence, token_url omission, reuse on stale registry, concurrent-persist race, public-route non-persistence, and admin vs non-admin gating; no real network calls
tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py	Strengthens existing admin test by adding persist_credentials=True assertion; adds new non-admin test verifying persist_credentials=False for INTERNAL_USER callers

_{Reviews (9): Last reviewed commit: "fix: reuse persisted MCP DCR clients" | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 86.20690% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
..._experimental/mcp_server/discoverable_endpoints.py	86.20%	12 Missing ⚠️

📢 Thoughts on this report? Let us know!

[veria-ai]

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 2 · PR risk: 0/10

[tin-berri]

@greptileai I pushed a fix for the token_url NULL-clobber finding: token_url is now only included in the update when present, so exclude_unset leaves the column untouched instead of overwriting it with NULL. Added a regression test (test_register_client_does_not_clobber_token_url_when_absent).

[tin-berri]

@greptileai re-review please: addressed the token_url NULL-clobber (now conditional), and added register-once/reuse of an existing client_id so multi-user servers don't re-mint per authorize. New regression tests cover both.

[tin-berri]

@greptileai re-review: added a security fix for the finding that the unauthenticated root /register route persisted DCR results. Persistence is now gated to the authenticated management path (mcp_register passes persist_credentials=True); the public route registers and returns without writing to the server row. Regression tests cover both halves.

[tin-berri]

@greptileai re-review please. Rebased onto latest litellm_internal_staging (clears a stale-base CI failure). Also dropped the cast on the persisted credentials dict; MCPCredentials already declares token_endpoint_auth_method as a Literal, so the dict is now typed directly.

[tin-berri]

@greptileai re-review please. Addressed the finding that a non-admin with server access could trigger persistence: persist_credentials is now gated to a full PROXY_ADMIN (_user_is_full_admin), so user-side registration returns the DCR response without writing shared client credentials. Added a non-admin regression test asserting no persist.

[tin-berri]

bugbot run

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ tin-berri
❌ cursoragent
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[tin-berri]

bugbot run

[tin-berri]

@greptileai rereview pls

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.

Autofix Details

Bugbot Autofix prepared fixes for all 3 issues found in the latest run.

• ✅ Fixed: Non-admin skips DB client reuse
  • Persisted DCR client reuse now runs before outbound registration regardless of persist_credentials, including non-admin registration calls.
• ✅ Fixed: Reuse reports success without refresh
  • Reuse now applies the persisted client credentials to the request server before returning success, so skipped DCR still leaves usable credentials in memory.
• ✅ Fixed: DCR response after skipped persist
  • When a concurrent persist wins after outbound DCR, the registration response now short-circuits to the shared dummy client instead of returning the losing upstream client_id.

_{You can send follow-ups to the cloud agent here.}

[mateo-berri]

@greptileai

[mateo-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 2704b55. Configure here.}

[mateo-berri]

LGTM; thanks!