feat(mcp): admit gateway DCR session bearers at the aggregate /mcp scope - #33190
Conversation
Greptile SummaryThis PR closes the admission loop for the aggregate
Confidence Score: 5/5Safe to merge. The new admission arm, cross-org grant model, credential scrubbing, and DCR flow are all implemented with clear fail-closed contracts and no new exploitable paths identified. The auth surface is large and genuinely complex, but the design is carefully layered: the un-forgeable server-only marker is stripped at construction time, each grant source is resolved independently through the same canonical resolver the key path uses, and every egress path scrubs admission credentials on credential shape rather than on the admission flag alone. The DCR flow single-use guards use atomic Redis INCR and fail closed on any fault. The only issues found are a wrong return-type annotation and a performance note about admitted_source_grants being recomputed several times per request, neither of which affects correctness or security. user_api_key_auth_mcp.py carries the bulk of the new auth/authz logic; the type annotation fix for _get_allowed_mcp_servers_for_org is the one mechanical change worth making before merge.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py | Major extension: adds the DCR session bearer admission arm, per-source grant union with cross-org ceilings, egress credential scrubbing, and billing attribution for admitted subjects. Logic is well-structured; _get_allowed_mcp_servers_for_org return type annotation does not reflect the new None return path. |
| litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py | New file implementing the stateless DCR register/authorize/complete/token flow. PKCE S256 enforced, single-use claims via atomic Redis INCR, cross-type confusion blocked by extra=forbid on each sealed model. |
| litellm/proxy/_types.py | Adds two server-only fields with exclude=True and a before_validator that strips them from any caller-supplied dict, preventing forgery. |
| litellm/proxy/auth/auth_checks.py | Refactors get_org_object to raise typed OrganizationNotFoundError for a confirmed missing row, allowing callers to distinguish absence from a DB fault. |
| litellm/proxy/hooks/parallel_request_limiter_v3.py | Extends mcp_per_team rate-limit descriptors to cover each team in mcp_source_team_rpm_limits, so admitted subjects with no single team_id are throttled per granting team. |
| litellm/proxy/management_endpoints/ui_sso.py | Adds _persist_return_to_cookie and _sso_return_to_redirect as shared helpers so every sign-in path honors a same-origin or control-plane return_to consistently. |
| litellm/proxy/auth/login_utils.py | Adds encode_ui_session_jwt that stamps an exp claim on every UI session cookie JWT, unifying all login paths through one helper. |
| litellm/proxy/proxy_server.py | Login endpoints now use encode_ui_session_jwt and honor litellm_cp_return_to; failures are swallowed so a stale cookie never blocks a completed sign-in. |
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Adds operator_open_server_ids as single owner of open-channel reachability; admitted subjects bypass the admin short-circuit and no_mcp_servers opt-out that are key-credential-only rules. |
| litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py | Routes gateway DCR client_id-prefixed requests to the new aggregate handlers; non-DCR paths are unchanged. |
Reviews (18): Last reviewed commit: "refactor(mcp): trim redundant comments a..." | Re-trigger Greptile
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
PR overviewThis PR updates the experimental MCP gateway DCR flow so gateway-issued DCR session bearer tokens can be admitted at the aggregate The PR has made substantial progress, with 14 issues already addressed, but two security issues remain open. The most important remaining concern is that DCR session tokens are not bound to the specific MCP servers the user approved, allowing a malicious client to use a consented session across all servers available to that user. There is also a refresh-token replay concern across workers when token state is not enforced through a shared atomic backend. These should be fixed before the feature is considered safe to merge. Open issues (2)
Fixed/addressed: 14 · PR risk: 7/10 |
1731965 to
65211a9
Compare
f9b90ee to
90eb828
Compare
65211a9 to
001064b
Compare
186c862 to
a22182f
Compare
1dedeaa to
1e3e792
Compare
|
bugbot run |
|
@greptileai rereview |
|
bugbot run |
|
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 5d67eab. Configure here.
5d67eab to
26ec746
Compare
26ec746 to
e44e407
Compare
e44e407 to
6455df5
Compare
|
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 6455df5. Configure here.
6455df5 to
0e1be8a
Compare
|
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 0e1be8a. Configure here.
0e1be8a to
435f6d7
Compare
|
bugbot run |
|
@greptileai rereview |
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 c16cdea. Configure here.
There was a problem hiding this comment.
some non-blocking questions:
- Is there any way to turn off the aggregate DCR surface (register/authorize/token plus session admission)? afaict it's always on once MCP is mounted, and the only real kill switch is rotating the master key. Is that intended?
- Am I reading
_SingleUseGuardright that the single-use code/refresh guards are per-worker on multi-worker deploys unless Redis is configured, and that with Redis they fail closed during an outage? Is that OK / should we document it?
…nt (LIT-3637) Admits a keyless SSO user (no virtual key) at the aggregate /mcp endpoint from a gateway DCR session bearer, resolving team/org/SCIM/budget authorization fresh on every call. - Aggregate DCR front door: stateless /register (sealed llm_dcrc_ client ids), SSO-backed /authorize + /authorize/complete, and /token minting identity-only session tokens with PKCE, single-use codes/flows, and rotating refresh tokens. - Admission: a session-shaped Authorization at the aggregate scope opens via _admit_gateway_session, reloads the live user, and runs the centralized policy gate; failures return the RFC 9728 invalid_token challenge. Gated on the un-forgeable, server-only mcp_admitted_user_subject marker, so virtual-key and JWT auth are unchanged. - Authorization model: an admitted subject is resolved as one plain UserAPIKeyAuth per grant source (its own grants, plus each team it is a live roster member of), each answered by the SAME resolver virtual keys use, then unioned. That branch is the FIRST statement of BOTH public resolvers, so no single-credential prelude runs for it and a fault in a lookup it never uses cannot deny its grants. A source team counts only while it is a live grantor: roster membership, not blocked, and neither the team nor its owning org over budget (enforced through the SAME _team_max_budget_check / _organization_max_budget_check owners common_checks uses for keys). Each team source carries that team's own org, so the existing org ceiling caps it; for a keyless source the org list only ever intersects (a ceiling must not become a grant) and an unresolvable ceiling denies rather than silently uncapping, on both the server and tool axes. _roster_team_object is the single owner of "which teams count": a team whose roster no longer lists the user neither grants servers nor throttles, in one place. - Rate limits: the subject is bounded by its user rpm/tpm AND by the per-server mcp_rpm_limit of the team a call is ATTRIBUTED to — the same single source billing charges, from the same owner. A key charges its one pinned team's bucket; a keyless subject has no team_id, so admission stamps each granting team's limit map onto the auth (server-only field, stripped from validated input like the marker) and the limiter emits that team's mcp_per_team descriptor. Charging every granting team instead would let one cross-team user drain several teams' SHARED buckets on a single call and block their other members; and a server the user's OWN grant reaches charges no team bucket at all, because no team provided it. Per-KEY MCP limits do not apply because there is no key. - Wrapper channels: the manager-level union treats the admitted subject by the same grant model. The admin-role short-circuit and the absolute no_mcp_servers early-return are key-credential rules and never apply to it (a session bearer is a third-party client credential, not the dashboard, and the subject's opt-out silences only its own source). Operator-open channels (allow_all_keys, the user's own BYOM submissions) are owned by one operator_open_server_ids helper that BOTH the server union and the admitted tool resolution consult (suppress-BYOM-when- explicitly-scoped is a key-credential rule and never applies to the subject, whose user row carries the DB-default empty mcp_servers), so an open-channel server is default-open for tools instead of listable but uninvokable. - Redirect URIs: one owner, validate_redirect_uri_shape, decides redirect-URI hygiene (bad scheme, fragment, missing host, userinfo, backslash host) and resolves allowlisted native callbacks, shared by DCR registration and the OAuth endpoints. Registration keeps a deliberately wider trust policy than validate_trusted_redirect_uri: public dynamic registration accepts any https client, and its controls are mandatory S256 PKCE plus the consent screen. - Egress leak-defense: a gateway admission credential (session bearer / bridge envelope) is scrubbed from EVERY egress header context, anchored to the credential shape, so it can never be forwarded upstream and replayed. - Single-use guard: auth-code, refresh and connect-flow claims resolve the proxy's cross-worker redis cache themselves rather than trusting the cache passed in, and fail CLOSED on a Redis fault instead of falling back to a per-worker count that a captured id could replay through another worker. - Sign-in return_to: one shared, never-raising helper persists a safe return_to for every sign-in branch (SSO/Okta/generic and username/password), and every branch RESUMES through the same _sso_return_to_redirect the SSO callback uses, so however a deployment signs in the stored value is honored identically (same-origin path directly; control_plane_url via the one-time login-code handoff). A stale cookie is ignored rather than failing a completed sign-in. - Budgets, both halves: ENFORCEMENT (an already over-budget team or its owning org stops being a grantor, in the source gate) and ACCOUNTING (a team-derived tool call is billed to the granting team and ITS org, so that budget accumulates and the right organization is charged). A server the user's own grant reaches bills the user; when several teams grant one server the pick is the lowest team_id, stable and auditable. Billing rides a COPY, so authorization still sees the full union, and it is inert when the target server cannot be resolved from the tool name. Deferred (tracked): client-selected server scoping of the session token (LIT-4680). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Compress the security rationale in the gateway-session admission path of user_api_key_auth_mcp.py, keeping the load-bearing "why" and dropping the restatement, and remove a garbled dead comment in get_allowed_tools_for_server In the tests, hoist the duplicated _team / _admitted_subject fixtures to module-level factories and parametrize the four fail-closed session-bearer variants into one case. No behavior change; the 294 tests in the file still pass
c16cdea to
ffa0dff
Compare
|
bugbot run |
|
@greptileai rereview |
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 ffa0dff. Configure here.
Relevant issues
/mcpendpoint from a gateway DCR session bearer, resolving team/org/SCIM/budget authorization fresh on every call.Changes
/mcpscope: session-shaped bearer -> live user reload -> centralized policy gate, gated on an un-forgeable server-only marker so virtual-key and JWT auth are byte-identical.mcp_rpm_limitbinds on the union: admission stamps each roster team's limit map onto the auth (server-only, unforgeable like the marker) and the limiter emits onemcp_per_teamdescriptor per applicable team, so a cross-team user cannot outrun any team's throttle and a team only throttles what it actually granted. A source team counts only while it is a live grantor — roster member, not blocked, and neither the team nor its owning org over budget (enforced via the same_team_max_budget_check/_organization_max_budget_checkownerscommon_checksuses for keys) — decided in one place for grants, tools and throttles together. The manager wrapper honors the same model: the admin-role full-registry short-circuit and the absoluteno_mcp_serversearly-return are key-credential rules that never apply to an admitted subject, and oneoperator_open_server_idsowner feeds both the server union and the admitted tool resolution so open-channel servers (allow_all_keys, own BYOM) are default-open for tools rather than listable but uninvokable.cursor://) register and userinfo-bearing URIs (https://claude.ai@attacker.example/cb) are rejected.return_tofor every sign-in branch, so the connect flow resumes after login.What does NOT change: virtual-key and JWT authorization take the same paths and the same code as before; the admitted subject is the only caller shape that fans out.
Things a reviewer will ask about. Registration deliberately accepts any https client rather than reusing
validate_trusted_redirect_uri's same-origin/allowlist policy: public dynamic registration must let a hosted MCP client register, and its controls are mandatory S256 PKCE plus the consent screen. Per-team BUDGET attribution for the union stays deferred on purpose: spend attributes to the user, whose own budget is enforced, and charging one team's shared budget for a server several teams grant has no correct attribution. Rate limits do not have that problem (all applicable descriptors are enforced together), so every granting team'smcp_rpm_limitnow binds. Client-selected server scoping of the session token is tracked as LIT-4680.Operational note: there is no off-switch for the aggregate DCR surface. The
mcp_gateway_dcrflag was removed earlier in the stack, so/register,/authorize,/tokenand the admission arm are always available, and the lazy loader registers them on path match, so they materialize on any proxy that receives a request to those paths whether or not/mcpis mounted or any MCP server exists. Exposure is bounded by the flow rather than by configuration:/registeris stateless and mints no credential (theclient_idIS the sealed registration), and/authorizeinterposes LiteLLM sign-in, so a token can only be minted by an authenticated LiteLLM user who then completes the deliberatePOST /authorize/complete. Revocation does not require rotating the master key; because admission reloads the live user on every call, deactivating or deleting the user cuts off outstanding session bearers at the next tool call, andallowed_ips/allowed_routesstill bound the/mcpside. Rotating the master key remains the only way to invalidate every outstanding session token at once, since the session keys derive from itOperational note: single-use claims need Redis on multi-worker deployments. The auth-code, refresh-token and connect-flow claims are recorded through the proxy's shared Redis when one is configured, and Redis is then the sole authority, so any Redis fault fails the claim closed instead of degrading to a per-worker count. Two consequences are worth knowing before deploying: a multi-replica proxy with no Redis configured gets the guarantee per worker only, which leaves a replay window across workers, and a Redis outage fails every claim closed, so gateway DCR sign-in and refresh stop working for its duration rather than degrading
Stacked on #33189 (aggregate flow) -> #33188 -> #33182; #33174 is merged.
Linear ticket
Part of LIT-3637 (PR 4 of the stack: the admission edge that turns a gateway session bearer into a request admitted as the litellm user, closing the loop end to end)
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
Admission edge — proven live end to end on a real proxy (:4139, Postgres,
--use_v2_migration_resolver) by walking the full flow to a session token, then presenting it at the aggregate/mcpendpoint:Cross-org grant model — proven live against real Postgres exercising the real
get_org_object/get_team_object/get_object_permissionresolvers, before/after on the identical seed:Type
🆕 New Feature
Changes
The stack so far lets a DCR client register, sign in, and receive an identity-only session bearer; this PR is the admission edge that accepts that bearer at the aggregate
/mcpendpoint. A new arm inprocess_mcp_requestfires when the request targets the aggregate scope (no server named byx-mcp-serversor the path) and theAuthorizationbearer is session-shaped. The aggregate DCR front door is always-on — the earliermcp_gateway_dcrconfig flag was removed inrefactor(mcp): make the aggregate DCR front door always-onearlier in the stack — so the arm is gated on request shape, not a setting. It opens the session token, reloads the live litellm user the token references through the SAME_reload_admitted_userthe bridge user-subject envelope path already uses, and runs the admitted identity through the centralized policy gate, so the user's present team, org, budget, and SCIM state gate the request rather than anything frozen at sign-in. Unlike the bridge arm it injects no per-server credential; the session seals no upstream token, those are vaulted per user and resolved at egress by user idThe token is a reference, not an authorization: the signature proves the user signed in, but authorization is resolved fresh on every call, so deactivating the user or their team takes effect immediately. A session-shaped bearer that is expired, tampered, minted under a different master key, or is a refresh token presented at the tool edge fails closed with the aggregate
invalid_tokenchallenge from PR 1; a non-session bearer never enters this arm and falls through to the existing oauth2 path unchangedThe aggregate-scope test (no named server, no path target) that PR 1 inlined for the challenge is factored into one
_is_aggregate_mcp_scopehelper now shared by the challenge and this admission arm, so the two cannot drift on what "aggregate" meansAuthorization model (keyless admitted subject). A keyless admitted user reaches the UNION of MCP servers/tools granted through ANY of their teams, plus their own direct object-permission grants. It is gated on the un-forgeable, server-only
mcp_admitted_user_subjectmarker (stripped from every validated input, set only post-construction after crypto + live DB reload), so virtual-key and JWT auth are byte-identical. Because ateam_id-less subject skips the centralizedcommon_checksper-team gates, the union enforces them itself, at the one chokepoint both the server and tool paths share:organization_idfalls back to the user's primary org; the user's own direct grants are capped by the user's org.team_idcannot keep access after removal).true_passthroughpath is stripped too — it is never a valid upstream token and can't be captured + replayed as the user.(user_id, server_id)(the config server's DCR client persists via fix(mcp): persist config.yaml DCR clients in a server-scoped store so refresh survives token expiry #33768).common_checksDOES enforce), never to a specific team's shared bucket, so there is no correct per-team attribution. Not a bypass of the user's own limits.QA runbook
Run a proxy with a master key and a database; walk the flow from the PR below this one to obtain a session
access_token, then present it asAuthorization: Bearer <token>toPOST /mcp/. Confirm initialize and tools/list return 200 (admitted as the user); a tampered token returns 401 with theerror="invalid_token"challenge; arefresh_tokenpresented at/mcp/returns 401; and an anonymous/mcp/still returns the bareresource_metadatachallenge. For the cross-org grant model, seed two orgs with different MCP ceilings + a user in a team under each, and confirm each team's servers/tools are capped by that team's own org (see the before/after above)Final Attestation
Note
High Risk
Large auth/authorization surface (MCP admission, multi-team grants, org ceilings, credential scrubbing, DCR token single-use); mistakes could leak cross-org access or forward replayable credentials.
Overview
Adds the admission edge for aggregate
/mcp: session-shapedAuthorizationbearers are opened via_admit_gateway_session, the live user is reloaded, and policy is enforced on every call. Virtual-key and JWT paths are unchanged.Keyless admitted users (server-only
mcp_admitted_user_subject, not forgeable from metadata) get MCP access by unioning grants per source—own permissions plus each roster-verified team—with per-team org ceilings on servers and tools, blocked/over-budget teams excluded, and fail-closed org lookup faults for keyless sources. Tool calls can bill the granting team viabilling_auth_for_tool_call; per-teammcp_rpm_limitdescriptors apply across teams.Egress hardening strips session/bridge-shaped credentials from all outbound header paths so gateway admission tokens cannot be replayed upstream.
Supporting changes:
gateway_dcr_flow(stateless register, authorize,POST /authorize/complete, token with Redis-backed single-use claims), shared redirect-uri hygiene, UI session JWTexp, unifiedreturn_toafter sign-in, andOrganizationNotFoundErrorso missing org vs DB outage is distinguishable for org ceilings.Reviewed by Cursor Bugbot for commit ffa0dff. Bugbot is set up for automated code reviews on this repo. Configure here.