feat(mcp): migrate the token_exchange (OBO) arm to the v2 resolver - #31526
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryMigrates the
Confidence Score: 5/5Safe to merge; all four concerns raised in the prior review round are resolved and no new correctness issues were found. The cache key now hashes the full exchange config so a config rotation forces a fresh exchange; eager construction via build_token_exchanger() eliminates the lazy-init race; _post_exchange_endpoint guards non-dict JSON bodies before field parsing; the LazyTokenExchanger wrapper is gone. Core logic is correct: RFC 8693 form is built properly, token_type and issued_token_type are validated before minting a Bearer upstream header, the browser-OAuth challenge is correctly scoped to authorization_code only, and the fail-closed path is exercised by tests. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/outbound_credentials/token_exchanger.py | New pure core for RFC 8693 OBO: config-inclusive cache key, single-flight coordinator, Bearer/issued_token_type validation, and client-auth delegation. Clean. |
| litellm/proxy/_experimental/mcp_server/outbound_credentials/token_exchange_provider.py | Composition root: wires exchanger to real httpx post, guards non-dict JSON bodies, maps transport failures to None. No lazy-init race. Clean. |
| litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py | Adds _token_exchange arm: requires inbound_token, delegates to injected TokenExchanger, propagates typed errors. Null-default exchanger returns misconfigured. Correct. |
| litellm/proxy/_experimental/mcp_server/outbound_credentials/adapter.py | Adds _token_exchange_spec: defers incomplete configs to v1, forwards audience only when set, uses token_url as fallback endpoint. Clean mapping. |
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Wires build_token_exchanger() at egress and scopes browser-OAuth challenge to authorization_code only. Correct fail-closed behavior. |
| litellm/proxy/_experimental/mcp_server/outbound_credentials/types.py | TokenExchangeConfig gains audience and token_endpoint_auth_method. Backward-compatible. |
| tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_token_exchanger.py | Comprehensive: OBO form, caching, rotation, single-flight, TTL expiry, token_type/issued_token_type checks. 344 lines of new tests. |
| tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_token_exchange_provider.py | Tests build_token_exchanger and HTTP edge (transport errors, success, non-object JSON). No real network calls. |
| tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py | Adds token_exchange resolver tests. Existing tests reformatted only, not weakened. |
| tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_adapter.py | Adds full-config mapping, token_url fallback, audience-omission, and incomplete-config defer-to-v1 cases. |
Reviews (9): Last reviewed commit: "feat(mcp): reject a non-access issued_to..." | Re-trigger Greptile
This comment was marked as outdated.
This comment was marked as outdated.
|
@greptileai addressed both findings: the exchanged-token cache key now binds the exchange config (so a config rotation re-exchanges instead of serving a stale token), and the lazy provider wrapper is gone in favor of eager build-once construction. Please re-review. |
|
@greptileai all three inline findings are now addressed as of 745c9df: (1) the exchanged-token cache key binds the full exchange config so a config rotation re-exchanges, (2) the lazy provider wrapper is removed in favor of eager build-once construction, and (3) the post adapter validates the JSON shape and maps a non-object body to a typed upstream_unavailable instead of a 500. Each has a regression test; both new files are at 100% coverage. Please re-review. |
745c9df to
c5dc4af
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: Forwarded Authorization bypasses OBO exchange
- Updated the shared Authorization stripping decision to cover migrated oauth2_token_exchange servers and added a regression test for configured header forwarding.
You can send follow-ups to the cloud agent here.
mateo-berri
left a comment
There was a problem hiding this comment.
Is the bugbot concern legit? Just reping me once you've gotten 5/5 greptile with "No files need special attention", no veria concerns, and no bugbot concerns on the last commit. Or if there are any false positives, a response to each of them as to why it's a false positive
|
|
e9e752c to
967b2f5
Compare
…mode Adds the pure Rfc8693TokenExchanger plus its composition root: the OBO exchange POSTs the RFC 8693 grant through an injected HTTP edge and returns the upstream-bound token as a typed Result, caching and single-flighting per (subject_token, server) so a repeated caller token skips the IdP round-trip. The audience is carried on TokenExchangeConfig and sent only when the operator set one, matching the spec default behavior. Errors are values: a missing endpoint or client credential is misconfigured, an IdP that returns no usable token is upstream_unavailable.
Routes RFC 8693 OBO servers through the v2 resolver: the resolver arm reads the caller's inbound token and swaps it via the injected TokenExchanger, to_server_spec maps a complete oauth2_token_exchange server (endpoint plus client credentials) to TokenExchangeConfig, and the egress wires the LazyTokenExchanger in. A token_exchange server with no caller token fails closed with a plain 401 rather than v1's fall-through to client_credentials, so the call site now scopes the per-server browser-OAuth challenge to authorization_code and lets other modes raise their own.
The exchanged-token cache was keyed only by (subject_token, server_id), so rotating a server's audience, scope, endpoint, client_id, or secret kept serving a token minted for the old config until TTL. The key now hashes the caller token together with the config that minted it, so a config change forces a fresh exchange. Everything is hashed, so no secret is held in the key.
…rapper The token exchanger reads no runtime global at build time (its httpx client is acquired per call), unlike the per-user store, so it does not need lazy first-use construction. Building it once at egress construction removes the first-use init path entirely and keeps the process-lifetime cache.
The post adapter annotated the parsed body as a dict without checking it, so a valid-but-non-object JSON response (list/string/number) was returned as-is and crashed the field parsing with an AttributeError. It now validates the shape at the boundary and returns None for a non-object body, so a malformed IdP response surfaces as a typed upstream_unavailable rather than a server error.
…the v2 OBO exchange
967b2f5 to
5bcf4aa
Compare
|
bugbot run |
|
@greptileai rereview |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Cache key omits auth method
- Included token_endpoint_auth_method in the exchange cache key and added a regression test for auth-method-only rotation.
You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 346088a. Configure here.

Relevant issues
Part of the MCP Gateway v2 migration; the next outbound-credential mode after
authorization_code(which landed via #31473 and #31493, now merged intolitellm_internal_staging). This branch is rebased onto staging, so the diff is just the token_exchange changes.Linear ticket
N/A
Pre-Submission checklist
What this is
The RFC 8693 token exchange (OBO) mode, migrated end to end onto the v2 outbound-credential resolver, the same way
authorization_codewas. A server declaringoauth2_token_exchangenow resolves its upstream credential throughresolve_credentialsinstead of v1'sresolve_mcp_auth/TokenExchangeHandler.The flow is the true on-behalf-of swap: the gateway takes the caller's inbound token, POSTs the RFC 8693 grant to the configured exchange endpoint authenticating as its own OAuth client, and forwards the returned upstream-bound token to the MCP server. The caller's token only ever reaches the IdP exchange endpoint, never the upstream.
Changes
Rfc8693TokenExchanger(new) is the pure core: it builds the RFC 8693 form, runs the exchange through an injected HTTP edge, and returns the upstream token as a typedResult[OAuthToken, CredError]. The exchanged token is cached and single-flighted per caller token, server, and exchange config (the cache key hashes the subject token together with the config that minted it), reusing the shared in-process foundation, so a repeated caller token skips the IdP round-trip and concurrent calls collapse to one exchange; a rotated caller token or a rotated server config hashes to a new key and re-exchanges rather than serving a stale token. v1's exchanged-token cache is per-process too, so there is no cross-replica machinery here.TokenExchangeConfiggains anaudiencefield that is forwarded only when the operator configured one. Bothaudienceandresourceare optional in RFC 8693 and the authorization server applies its own default when neither is sent, so fabricating an audience from the URL risksinvalid_target; sending it only when set matches both the spec's intent and v1.The resolver arm reads the caller's inbound token and swaps it via the injected
TokenExchanger.to_server_specmaps a completeoauth2_token_exchangeserver (an endpoint plus client credentials, mirroring v1'shas_token_exchange_config) toTokenExchangeConfig, and defers an incomplete one to v1. The egress wires theLazyTokenExchangerinto the provider.One deliberate behavior change, gated behind the v2 resolver flag: a token_exchange server hit without a caller token now fails closed with a plain 401 rather than falling through to v1's client_credentials grant. The v2 contract is that a configured mode whose credential is absent errors rather than silently using a weaker source, so the call site now scopes the per-server browser-OAuth challenge to
authorization_codeand lets token_exchange raise its own 401.Errors are modeled as values throughout: a missing endpoint or client credential is
misconfigured, an IdP that returns no usable token isupstream_unavailable, and the I/O lives only in the injected post adapter.Two hardening additions land on top of the migration. The exchanger now validates the RFC 8693 response
token_typebefore minting the upstream header: an exchanged token whosetoken_typeis present and notBearer(for exampleN_A, which RFC 8693 uses for a token that is not a standalone access token) fails closed asupstream_unavailablerather than being forwarded as a bogusBearer, and an absenttoken_typestill defaults toBearerper RFC 6750 so IdPs that omit the field keep workingTokenExchangeConfigalso gainstoken_endpoint_auth_method, threaded from the server config, so an OBO server can authenticate to the exchange endpoint withclient_secret_basic(HTTP Basic, the OIDC default) as well as the existingclient_secret_post. The exchanger builds its client auth through the sharedbuild_token_endpoint_client_authhelper, so Basic sends the credentials in theAuthorizationheader and keeps them out of the body while post keeps them in the form, matching how the v1 token endpoints already apply the two methodsTests
New
test_token_exchanger.pyandtest_token_exchange_provider.pypin the exchange and form, the per-caller-token cache, single-flight under concurrency, TTL expiry on an injected clock, the rotated-token re-exchange, and the misconfigured/upstream_unavailable mappings.test_resolver.pycovers the arm (OBO success, no-token 401, error propagation, fail-closed default) andtest_adapter.pycovers the mapping (complete vs incomplete config, audience present vs omitted, scopes, subject_token_type). The fulloutbound_credentialssuite is green (213), including the non-Bearertoken_typefail-closed cases and theclient_secret_basicvsclient_secret_postclient-auth cases, ruff and format are clean, and basedpyright adds no new errors over the baseline.Screenshots / Proof of Fix
Verified end to end against a real IdP doing RFC 8693, on the branch with
--use_v2_migration_resolver.Setup: Keycloak 26.2 (standard token exchange, GA) in Docker with realm
mcp, acaller-client(alice's source token), a confidentiallitellm-gatewayclient (token exchange enabled), anupstream-mcpaudience client, and audience mappers so the gateway may exchange alice's token for theupstream-mcpaudience. A mock upstream MCP server records theAuthorizationit receives. The proxy runs anoauth2_token_exchangeMCP server pointingtoken_exchange_endpointat Keycloak andurlat the mock.Proof 1, Keycloak performs the real RFC 8693 swap (the exact request the gateway makes):
Proof 2, the v2
token_exchangearm is live on the proxy and fails closed without a caller token (this exact message is from the new_token_exchangearm):Proof 3, the real
resolve_credentials(with the productionbuild_token_exchanger, which POSTs to the live Keycloak) forwards the EXCHANGED token to the upstream, not alice's caller token:Note on discovery: the aggregator's
tools/listpath does not thread the caller token, so anoauth2_token_exchangeserver's tools are not discoverable through the aggregator on the v2 path (v1 masked this with its no-subject client_credentials fallback, which v2 intentionally drops). The arm and the egress are correct as shown; making OBO usable end to end through the aggregator additionally needs the caller token threaded into discovery, tracked as a follow-up.Type
New Feature
Changes
See above.
Note
High Risk
Changes authentication and RFC 8693 token exchange for upstream MCP calls, including fail-closed behavior and what bearer reaches upstream; mistakes could leak caller tokens or break OBO.
Overview
RFC 8693 on-behalf-of (OBO) for MCP servers with
oauth2_token_exchangenow runs through the v2UpstreamCredentialProviderinstead of v1resolve_mcp_auth/TokenExchangeHandler.The gateway takes the caller’s inbound bearer, POSTs the token-exchange grant to the configured IdP (as its own OAuth client), caches and single-flights the result per hashed
(subject token + exchange config), and attaches the exchanged upstream bearer.to_server_specmaps complete server config toTokenExchangeConfig(endpoint ortoken_url, client creds, optionalaudience,token_endpoint_auth_method, scopes); incomplete configs still defer to v1.TokenExchangeConfigdocuments optionalaudience(no fabricated default) and addsaudience+token_endpoint_auth_methodon the model.The resolver’s
token_exchangearm returns 401 when there is no caller token (no v1 client-credentials fallback). Browser OAuth challenges onunauthorizedare limited toauthorization_codeso OBO errors surface as plain 401s. Response hardening rejects non-Bearertoken_typeand clearly non-accessissued_token_typebefore forwarding.build_token_exchanger()wiresRfc8693TokenExchangerat MCP egress construction with the shared httpx POST adapter and in-process cache TTLs matching existing MCP OAuth constants.Reviewed by Cursor Bugbot for commit 346088a. Bugbot is set up for automated code reviews on this repo. Configure here.
Deliberately not implemented
Scope notes from the OBO behavior-contract audit, recorded so review does not mistake these for oversights. The RFC 8707
resourceparameter is not supported; targeting is viaaudienceonly, and none is fabricated when unset since the authorization server applies its own default. Client authentication to the exchange endpoint supportsclient_secret_basicandclient_secret_postonly;private_key_jwtand mTLS are out of scope for now. The response'stoken_typeandissued_token_typeare validated when present (a non-Bearertoken_typeor a refresh/id/samlissued_token_typefails closed) but their absence is tolerated rather than treated as a protocol error. A narrowerscopegranted in the response is not read back. The exchange endpoint URL is not restricted to https, so local and test IdPs work. The exchanged-token cache is in-process only, matching v1; durable or cross-instance persistence is a possible later step for multi-replica deployments. The cache key binds subject token, tenant, endpoint, audience, scopes, and client, but does not parse the subject to key oniss, and per-issuer exchange config routing does not exist since a server has exactly one configured IdP. Transient IdP failures surface as a retryable 503 with no gateway-side backoff loop