fix(mcp): handle OAuth IdP error responses in /callback (LIT-2750) by oss-agent-shin · Pull Request #28743 · BerriAI/litellm

oss-agent-shin · 2026-05-24T02:59:59Z

Summary

Bug: When an IdP rejects an MCP OAuth authorization request, it redirects back to the proxy's /callback with ?error=access_denied&error_description=…&state=… and no code (per RFC 6749 §4.1.2.1). The handler declared code and state as required FastAPI query params, so FastAPI rejected the error response with a 422 / Pydantic validation blob before the handler ran. The user landed on an opaque JSON error page and the MCP loopback client kept waiting on the listener until it timed out. The same 422 also fires when an SSO redirect chain drops the original /authorize query params.
Fix (litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py):
- Make code and state optional, and accept the RFC-defined error, error_description, error_uri params.
- When state decodes to a trusted client redirect_uri, propagate the OAuth error back to that URI with the client's original (un-wrapped) state preserved, so the client's OAuth library can surface the failure (RFC 6749 §4.1.2.1).
- When state is missing/undecryptable or the encoded redirect_uri is no longer trusted, render a 400 HTML page with the (HTML-escaped) error instead of leaking to an attacker-controlled redirect.
- Preserve the existing success path: code + state → 302 to validated client redirect_uri with original state.

Closes LIT-2750 (Linear).

Originally reported in the Slack thread linked from the Linear ticket: a user without all the IdP entitlements got bounced into an MCP OAuth flow that returned 422 instead of an actionable error.

Security considerations

No open-redirect: the error path re-uses the existing _get_validated_client_redirect_uri allowlist (loopback / same-origin / ops-allowlisted). An untrusted redirect_uri encoded in a stale state falls back to the inline HTML page rather than 302-ing the user to an attacker-controlled origin. Regression test: test_idp_error_with_untrusted_redirect_uri_does_not_open_redirect.
No XSS via IdP-supplied fields: error and error_description go through html.escape before being rendered in the HTML fallback. Regression test: test_idp_error_html_escapes_user_controlled_fields.
No 500-on-bad-state: if decode_state_hash raises (e.g. salt key rotated, tampered state), the handler returns 400 HTML instead of propagating the exception. Regression test: test_idp_error_with_undecryptable_state_falls_back_to_html.

Changes

litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py
- Import html as _html.
- Add _render_oauth_error_html(error, description) helper.
- Replace callback(request, code: str, state: str) with optional code/state and new error / error_description / error_uri params; branch by (error present | bare GET | success).
tests/test_litellm/proxy/_experimental/mcp_server/test_callback_oauth_error_responses.py (new)
- 7 regression tests covering the cases above.

Manual testing evidence

Reproduced the original 422 against the unmodified endpoint, then verified each behaviour against the patched endpoint using a fastapi.testclient.TestClient mounted with the discoverable router (LITELLM_SALT_KEY=sk-test-salt-for-LIT-2750).

1. IdP error, no state — was 422 Pydantic, now 400 HTML

status_code: 400
content-type: text/html; charset=utf-8
body[:240]:  <html><body><h2>Authentication failed</h2>
             <p><strong>Error:</strong> access_denied</p>
             <p>User declined access</p>
             <p>You can close this window and try again.</p>
             </body></html>

2. IdP error + trusted (loopback) state — propagates back to client redirect_uri

status_code: 302
location:    http://127.0.0.1:60108/callback?error=access_denied
                                            &error_description=User+declined+access
                                            &state=client-original-state-xyz

The wrapped (encrypted) state passed in is not present in the location header — only the original client state is round-tripped, exactly how the MCP client's OAuth library expects.

3. Bare `GET /callback` (no params) — was 422 Pydantic, now 400 HTML

status_code: 400
body[:240]:  <html><body><h2>Authentication failed</h2>
             <p><strong>Error:</strong> invalid_request</p>
             <p>Missing authorization &#x27;code&#x27; and &#x27;state&#x27; parameters.</p>
             ...

4. Success path — unchanged, still 302 to validated client redirect_uri

status_code: 302
location:    http://127.0.0.1:60108/callback?code=auth-code-abc
                                            &state=client-original-state-xyz

5. Untrusted `redirect_uri` encoded in state — no open redirect

status_code: 400
body[:240]:  <html><body><h2>Authentication failed</h2>
             <p><strong>Error:</strong> access_denied</p>
             ...

The proxy logs the rejection (MCP OAuth: rejecting redirect_uri 'https://attacker.example.com/steal') and surfaces the error inline rather than 302-ing to the attacker URL.

6. HTML escapes IdP-supplied fields — no XSS via `error` / `error_description`

status_code: 400
body[:240]:  <html><body><h2>Authentication failed</h2>
             <p><strong>Error:</strong> &lt;script&gt;alert(1)&lt;/script&gt;</p>
             <p>&lt;img src=x onerror=alert(2)&gt;</p>
             ...

Test plan

$ LITELLM_SALT_KEY=sk-test-salt-for-LIT-2750 \
    python3 -m pytest \
      tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py \
      tests/test_litellm/proxy/_experimental/mcp_server/test_callback_oauth_error_responses.py \
      --tb=short -q

........................................................................ [ 96%]
...                                                                      [100%]
75 passed in 14.67s

7 new tests in test_callback_oauth_error_responses.py cover the bug + each fallback branch.
68 pre-existing tests in test_discoverable_endpoints.py continue to pass (no behaviour change for the success path or /authorize//token endpoints).

Type

Bug fix (non-breaking change which fixes an issue)

No UI surface changes — this is a server-side handler fix.

Agent session: https://litellm-agent-platform.onrender.com/agents/9cbb91a6-e66d-43c5-92ed-68a570429527

[Infra] Promote internal staging to main

chore(ci): promote internal staging to main

Per RFC 6749 section 4.1.2.1, when the IdP rejects an OAuth authorization request it redirects back to the client with ?error=...&error_description=... and no code. The MCP /callback handler declared code and state as required query params, so FastAPI rejected such error responses with a 422 before the handler ran -- stranding the MCP client waiting on the loopback. This change: - Makes code and state optional and accepts the RFC-defined error, error_description, and error_uri params. - When state decodes to a trusted client redirect_uri, propagates the error params back to that URI with the client's original (un-wrapped) state preserved, so the client's OAuth library can surface the failure. - When state is missing/undecryptable or the encoded redirect_uri is no longer trusted, renders a 400 HTML page with the (HTML-escaped) error details instead of leaking to an attacker-controlled redirect. - Preserves the existing success path (code + state -> 302 to validated client redirect_uri with original state). Fixes LIT-2750.

…s (LIT-2750) Adds a new test module covering the LIT-2750 fix: the MCP OAuth /callback endpoint must accept IdP error responses (e.g. ?error=access_denied) per RFC 6749 section 4.1.2.1 instead of returning a 422 because ``code`` is missing. Coverage: - IdP error with no state -> 400 HTML page surfacing the error. - HTML escaping of user-controlled error / error_description fields. - IdP error with a trusted (loopback) state -> 302 propagating error / error_description / original client state to the client. - IdP error with an untrusted redirect_uri encoded in state -> 400 inline (no open-redirect to attacker-controlled origin). - IdP error with an undecryptable state -> 400 HTML fallback. - Bare GET /callback with no params -> 400 HTML (not Pydantic 422). - Success path (code + state) still 302 to validated client redirect_uri with the original (un-wrapped) state preserved.

CLAassistant · 2026-05-24T03:00:07Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

codspeed-hq · 2026-05-24T03:02:34Z

Merging this PR will not alter performance

✅ 16 untouched benchmarks

_{Comparing oss-agent-shin:shin/lit-2750-oauth-callback-error-handling-v2 (5192a4c) with main (06f6cfc)}

greptile-apps · 2026-05-24T03:02:37Z

Greptile Summary

Fixes a 422 returned by the MCP OAuth /callback endpoint when an IdP redirects back with ?error=... and no code, by making code/state optional and implementing the RFC 6749 §4.1.2.1 error-response flow.

discoverable_endpoints.py: adds a _render_oauth_error_html helper and rewrites callback to branch on error/missing/success params — propagating IdP errors to validated loopback redirect URIs, falling back to an HTML 400 page for untrusted or missing state, and HTML-escaping all IdP-supplied fields to prevent XSS.
test_callback_oauth_error_responses.py: 7 new regression tests cover all new branches (error propagation, open-redirect prevention, undecryptable state, XSS escaping, bare GET, and success-path regression).

Confidence Score: 5/5

Safe to merge — changes are scoped to the MCP OAuth callback handler, the success path is preserved, and the new error path re-uses the existing redirect-URI allowlist.

The change is narrowly scoped: it makes two query params optional and adds a well-guarded error branch. Open-redirect risk is mitigated by reusing _get_validated_client_redirect_uri, XSS risk is handled by html.escape, and exception handling falls back to an inline HTML page rather than propagating. All new branches are covered by dedicated regression tests.

No files require special attention.

Important Files Changed

Filename	Overview
litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py	Makes `code`/`state` optional on `/callback` and adds RFC 6749 §4.1.2.1 error-path handling: propagates IdP errors to trusted redirect URIs, falls back to an HTML error page for untrusted/missing state, and HTML-escapes all IdP-supplied fields to prevent XSS.
tests/test_litellm/proxy/_experimental/mcp_server/test_callback_oauth_error_responses.py	New regression test file with 7 unit tests covering IdP error paths, XSS escaping, open-redirect prevention, undecryptable state fallback, bare GET 400, and success path regression; uses only mocks, no real network calls.

_{Reviews (2): Last reviewed commit: "refactor(mcp): drop unused _OAUTH_ERROR_..." | Re-trigger Greptile}

codecov · 2026-05-24T03:02:59Z

Codecov Report

❌ Patch coverage is 96.77419% with 1 line in your changes missing coverage. Please review.

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

📢 Thoughts on this report? Let us know!

The tuple was leftover scaffolding from an earlier draft of the LIT-2750 fix; nothing references it. The explanatory RFC 6749 §4.1.2.1 comment block above the callback handler covers the same intent.

oss-agent-shin · 2026-05-24T03:06:38Z

Addressed the P2: dropped the unused _OAUTH_ERROR_PARAMS constant in 5192a4c (kept the RFC 6749 §4.1.2.1 comment block above the handler since it documents the contract).

@greptileai review

Sameerlite

LGTM, thanks!

shin-berri and others added 5 commits May 13, 2026 22:37

Merge pull request BerriAI#27906 from BerriAI/litellm_internal_staging

e58a561

[Infra] Promote internal staging to main

Merge pull request BerriAI#28100 from BerriAI/litellm_internal_staging

a72414a

[Infra] Promote internal staging to main

Merge pull request BerriAI#28292 from BerriAI/litellm_internal_staging

79b4578

chore(ci): promote internal staging to main

greptile-apps Bot reviewed May 24, 2026

View reviewed changes

Comment thread litellm/proxy/_experimental/mcp_server/discoverable_endpoints.py Outdated

refactor(mcp): drop unused _OAUTH_ERROR_PARAMS constant (Greptile P2)

5192a4c

The tuple was leftover scaffolding from an earlier draft of the LIT-2750 fix; nothing references it. The explanatory RFC 6749 §4.1.2.1 comment block above the callback handler covers the same intent.

Sameerlite approved these changes May 25, 2026

View reviewed changes

Sameerlite changed the base branch from main to litellm_oss_staging_250526 May 25, 2026 06:10

Sameerlite merged commit 2bdfae1 into BerriAI:litellm_oss_staging_250526 May 25, 2026
50 of 53 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(mcp): handle OAuth IdP error responses in /callback (LIT-2750)#28743

fix(mcp): handle OAuth IdP error responses in /callback (LIT-2750)#28743
Sameerlite merged 6 commits into
BerriAI:litellm_oss_staging_250526from
oss-agent-shin:shin/lit-2750-oauth-callback-error-handling-v2

oss-agent-shin commented May 24, 2026

Uh oh!

CLAassistant commented May 24, 2026

Uh oh!

codspeed-hq Bot commented May 24, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented May 24, 2026 •

edited

Loading

Important Files Changed

Uh oh!

Uh oh!

codecov Bot commented May 24, 2026 •

edited

Loading

Uh oh!

oss-agent-shin commented May 24, 2026

Uh oh!

Sameerlite left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

oss-agent-shin commented May 24, 2026

Summary

Security considerations

Changes

Manual testing evidence

1. IdP error, no state — was 422 Pydantic, now 400 HTML

2. IdP error + trusted (loopback) state — propagates back to client redirect_uri

3. Bare GET /callback (no params) — was 422 Pydantic, now 400 HTML

4. Success path — unchanged, still 302 to validated client redirect_uri

5. Untrusted redirect_uri encoded in state — no open redirect

6. HTML escapes IdP-supplied fields — no XSS via error / error_description

Test plan

Type

Uh oh!

CLAassistant commented May 24, 2026

Uh oh!

codspeed-hq Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

greptile-apps Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

codecov Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

oss-agent-shin commented May 24, 2026

Uh oh!

Sameerlite left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

3. Bare `GET /callback` (no params) — was 422 Pydantic, now 400 HTML

5. Untrusted `redirect_uri` encoded in state — no open redirect

6. HTML escapes IdP-supplied fields — no XSS via `error` / `error_description`

codspeed-hq Bot commented May 24, 2026 •

edited

Loading

greptile-apps Bot commented May 24, 2026 •

edited

Loading

codecov Bot commented May 24, 2026 •

edited

Loading