fix(mcp): persist DCR client_id from on-create MCP OAuth Authorize & Fetch - #31920
Conversation
Greptile SummaryThis PR fixes on-create "Authorize & Fetch" so that DCR
Confidence Score: 5/5UI-only change with no backend modifications; DCR credentials are forwarded through an existing encrypted-credentials path already verified in production. The fix is narrow and well-contained: it threads DCR client data through the callback signature and records which URL was authorized, both backed by new regression tests that fail on the pre-fix code. The reset-version guard correctly prevents stale token exchanges from updating state after reset. No new data is exposed that wasn't already handled by the backend's encrypt_credentials path. No files require special attention.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/hooks/useMcpOAuthFlow.tsx | Adds resetVersionRef for in-flight-exchange cancellation, relaxes hasPreconfiguredCredentials to client_id-only, and forwards {clientId, clientSecret} from stored flow state to onTokenReceived. Logic is correct and consistent with RFC 7591 public-client semantics. |
| ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx | Introduces authorizedUrl state and clearAuthorizedOAuthState helper; onTokenReceived now merges DCR client credentials into form fields and records the authorized URL; URL/spec/transport changes trigger invalidation. No backend changes required. |
| ui/litellm-dashboard/src/hooks/useMcpOAuthFlow.test.tsx | Adds tests for: hook forwarding of DCR client to callback, in-flight exchange ignored after reset, DCR reuse with existing client_id, fresh registration without client_id. Updated assertions match corrected behavior. |
| ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx | Adds tests for URL/spec_path/transport invalidation of authorized OAuth state on the create form; mock type signatures updated to reflect the new two-argument onTokenReceived contract. |
Reviews (7): Last reviewed commit: "Merge branch 'litellm_internal_staging' ..." | Re-trigger Greptile
This comment was marked as duplicate.
This comment was marked as duplicate.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
9a253ae to
462abc4
Compare
f38c536 to
d682ac8
Compare
462abc4 to
d56e0d1
Compare
d682ac8 to
3775643
Compare
d56e0d1 to
5641933
Compare
3775643 to
8af6771
Compare
5641933 to
680140e
Compare
8af6771 to
2e9e592
Compare
|
@greptileai re-review please (rebased onto the updated base branch). |
680140e to
f984d02
Compare
2e9e592 to
29dce7e
Compare
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stale client_id skips registration
- Cleared fetched OAuth credentials and reset OAuth state when the create form server URL changes, preventing stale DCR client IDs from being reused or saved for a new endpoint.
You can send follow-ups to the cloud agent here.
|
|
Interactive authorization_code MCP servers register an OAuth client via Dynamic Client Registration (RFC 7591) during the authorize flow, but the minted client_id and the discovered token_url were returned to the caller and never written to the server row. The autonomous refresh_token grant reads client_id, client_secret and token_url off the server, so an expired access token could not be refreshed; the user was bounced back to re-authorize and tools/list returned zero tools Persist the DCR client_id (plus client_secret and token_endpoint_auth_method when the registration returns them) and the discovered token_url onto the server row, reusing the encrypt_credentials write that client_credentials and token exchange already use, then refresh the in-memory registry so the value is live at refresh time. Both the v1 refresher and the v2 AuthorizationCodeRefresher read those same fields, so egress needs no change
84ba218 to
efe3523
Compare
c413b9f to
40830ed
Compare
|
bugbot run |
|
@greptileai rereview |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: OpenAPI spec change skips invalidation
- OAuth authorization now tracks the transport-specific target and clears credentials when an OpenAPI spec URL changes.
- ✅ Fixed: Late OAuth repopulates cleared credentials
- OAuth reset now invalidates in-flight exchanges so late token responses cannot repopulate cleared credentials.
You can send follow-ups to the cloud agent here.
…Fetch"
The interactive "Authorize & Fetch" flow on the create form registers an OAuth
client (RFC 7591) against a temporary server that has no DB row, then creates the
real server afterward. useMcpOAuthFlow captured the DCR client_id and client_secret
but passed only the token to onTokenReceived, so the create request dropped the
client identity and the created server could not refresh its access token; its row
had credentials={} and the refresh_token grant 401d at the upstream token endpoint
Forward the registered client to onTokenReceived and write client_id (and
client_secret when present) into the create form credentials, so the create request
carries them and the backend persists them through its existing encrypt_credentials
path. token_url is omitted because it is re-discovered on load (RFC 9728 then 8414);
token_endpoint_auth_method is unused because this flow only ever registers as
client_secret_post or none, never client_secret_basic
40830ed to
c33aeaf
Compare
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: OAuth survives transport switch
- OAuth credentials, DCR client data, tokens, tools, and the authorization target are now cleared when transport changes after authorization.
- ✅ Fixed: authorizedUrl survives modal close
- The modal-close cleanup now clears the stored authorized URL along with the rest of the OAuth state.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit d059a91. Configure here.
…client_oncreate Resolve conflicts in discoverable_endpoints.py and its test by taking the canonical version now on the base branch, which is #31912 squash-merged with its post-review refinements (client_secret and token_endpoint_auth_method persistence, the decrypt/apply helpers, and the DcrRegistrationPersistenceResult result). This branch's copies of those backend commits predate the merge and none of the on-create UI commits touch those files, so taking base keeps the merged behavior with no loss. The on-create UI change sits cleanly on top of the base's token endpoint auth method selector work; the net PR diff is only the four dashboard files
…Fetch (BerriAI#31920) * fix(mcp): persist DCR client_id so interactive OAuth token refresh works Interactive authorization_code MCP servers register an OAuth client via Dynamic Client Registration (RFC 7591) during the authorize flow, but the minted client_id and the discovered token_url were returned to the caller and never written to the server row. The autonomous refresh_token grant reads client_id, client_secret and token_url off the server, so an expired access token could not be refreshed; the user was bounced back to re-authorize and tools/list returned zero tools Persist the DCR client_id (plus client_secret and token_endpoint_auth_method when the registration returns them) and the discovered token_url onto the server row, reusing the encrypt_credentials write that client_credentials and token exchange already use, then refresh the in-memory registry so the value is live at refresh time. Both the v1 refresher and the v2 AuthorizationCodeRefresher read those same fields, so egress needs no change * fix: reuse persisted MCP DCR clients * fix(ui): persist DCR client_id from on-create MCP OAuth "Authorize & Fetch" The interactive "Authorize & Fetch" flow on the create form registers an OAuth client (RFC 7591) against a temporary server that has no DB row, then creates the real server afterward. useMcpOAuthFlow captured the DCR client_id and client_secret but passed only the token to onTokenReceived, so the create request dropped the client identity and the created server could not refresh its access token; its row had credentials={} and the refresh_token grant 401d at the upstream token endpoint Forward the registered client to onTokenReceived and write client_id (and client_secret when present) into the create form credentials, so the create request carries them and the backend persists them through its existing encrypt_credentials path. token_url is omitted because it is re-discovered on load (RFC 9728 then 8414); token_endpoint_auth_method is unused because this flow only ever registers as client_secret_post or none, never client_secret_basic * fix(ui): prevent stale MCP OAuth credentials * fix(ui): reset MCP OAuth authorization state --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> (cherry picked from commit 3235f4a)

Relevant issues
Linear ticket
LIT-4154 https://linear.app/litellm-ai/issue/LIT-4154
This is the stacked follow-up to #31912. It covers variant 2 (on-create "Authorize and Fetch"); #31912 covered variant 1 (post-create authorize).
Pre-Submission checklist
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
The "Authorize and Fetch" button on the create form runs OAuth Dynamic Client Registration (RFC 7591) against a temporary in-memory server that has no DB row, then creates the real server afterward. #31912 persists the DCR
client_idwhen authorize runs against a server that already has a DB row, but on-create there is no row yet, so the mintedclient_idwas dropped and the created server could not refresh (its row hadcredentials={}andtoken_urlnull, and therefresh_tokengrant 401d at the upstream token endpoint).The UI already holds the DCR
client_id(andclient_secretwhen the upstream returns one) from the register step; it was just discarded atonTokenReceived. This change carries it intoform.credentials, so it rides the existing settablecredentials.client_idfield into the create request, and the backend persists it through the sameencrypt_credentialspath it already uses for manually-entered OAuth clients.token_urlis not carried because it is re-discovered on load (RFC 9728 then 8414);token_endpoint_auth_methodis not needed because this flow only ever registers asclient_secret_postornone, neverclient_secret_basic.Backend already persists a create-supplied
client_idencrypted (existing behavior, confirming the mechanism this fix feeds):Live UI before/after (Authorize and Fetch on create, real upstream MCP OAuth, same proxy + Postgres):
Type
Bug Fix
Changes
useMcpOAuthFlowalready captures the DCRclient_idandclient_secretfrom the register response into its flow state and uses them for authorize and token exchange, but it passed only the token to theonTokenReceivedcallback. This forwards the registered client toonTokenReceived, and the create form writesclient_id(andclient_secretwhen present) intoform.credentialsalongside the tokens, so the create request carries them and the backend persists them. No backend change is needed; the create path already encrypts and storescredentials.client_id.A regression test in
useMcpOAuthFlow.test.tsxasserts the hook forwards the registeredclient_id/client_secrettoonTokenReceived; it fails on the pre-fix code (callback received only the token) and passes with the fix.It also flips
hasPreconfiguredCredentialsfrom requiring bothclient_idandclient_secretto requiring justclient_id, so the UI skips re-registration when a client already exists (a public client has aclient_idbut no secret). This matches the backend reuse guard in the base PR; together they give one client per server reused across users, rather than re-minting per authorize.Note
Medium Risk
Touches OAuth credential persistence and invalidation in the dashboard; mistakes could leak wrong clients or break refresh, but scope is UI-only with added regression tests and no backend changes.
Overview
Fixes on-create “Authorize & Fetch” so Dynamic Client Registration
client_id/client_secretare not dropped before the server is created.useMcpOAuthFlownow passes the registered client intoonTokenReceived, treats a preconfiguredclient_idalone as enough to skip re-registration (public clients), and uses a reset version so a late token exchange cannot repopulate credentials afterreset().The create MCP server form merges those values into
form.credentials, records which URL/spec was authorized, and clears OAuth state (credentials, discovered endpoints, tools, flow reset) when MCP URL or OpenAPIspec_pathchanges after authorization so stale DCR clients are not submitted.Tests cover hook forwarding, in-flight exchange after reset, DCR reuse vs fresh registration, and URL/spec invalidation on create.
Reviewed by Cursor Bugbot for commit d059a91. Bugbot is set up for automated code reviews on this repo. Configure here.