Repository navigation
test(e2e): cover MCP OAuth SSO and cold restart persistence - #41909
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
…nexpected consent Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
…ear tool name Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
… the stored token once per phase Co-Authored-By: bot_apk <apk@cognition.ai>
…py_path_e2e Co-Authored-By: bot_apk <apk@cognition.ai> # Conflicts: # tests/e2e/mcp/oauth_chat_client.py
|
bugbot run |
|
@veria-ai Please review the current tip 742a3ad. All four OAuth cases passed with zero skips in https://github.com/BerriAI/litellm/actions/runs/35467606351. Scope is LIT-3467 E2E tests, necessary helpers, CI and documentation. (aside) |
|
bugbot run |
|
@veria-ai @greptileai Please review 90687ae. The reconnect guard now records Linear requests across the browser context and checks them even after a fast callback. A real Chromium probe reproduced the old false pass and now rejects it; permitted initial consent and gateway-only reconnect still pass. Full lint passed; fresh live CI is pending. (aside) |
|
bugbot run |
|
@veria-ai @greptileai Please refresh review at d5ac850 after integrating main and its shared E2E lockout fix #42008. No new production diff; the only accompanying adjustment adds isolated Python execution to our diagnostic reporter test. All 147 affected tests and full lint passed locally. (aside) |
|
bugbot run |
|
@veria-ai Please review the latest PR tip, including the integration of #42008. Review only; no implementation changes requested. (aside) |
|
@greptileai Please review the latest PR tip after the main-branch integration. (aside) |
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 d5ac850. Configure here.
TLDR
Problem this solves:
How it solves it:
User Flow
Before: an engineer who breaks the MCP OAuth login, or the reuse of the stored upstream token, still gets a green PR, because no CI job walks the real journey
http://<gateway>/mcp, logs in through gateway SSO, approves Linear's real consent screen, and then calls tools/list and tools/callAfter: the same PR gets a red
MCP OAuth happy path / oauthcheck that names the failing phaseMCP OAuth happy pathworkflow starts and waits for ane2e-changedapproval from a LiteLLM engineerPOST http://127.0.0.1:<port>/v1/mcp/server, then connects an MCP SDK client tohttp://127.0.0.1:<port>/mcpusing gateway SSO, and tohttp://127.0.0.1:<port>/<alias>/mcpusing a JWT inx-litellm-api-key<alias>-list_teamsand expects real team data withisError=falseGET http://127.0.0.1:<port>/v1/mcp/server/{server_id}/user-credentialsshows oneoauth2credential for that user, the gateway restarts, and a fresh client reconnects and gets the same team data with no second Linear consentThe aggregate client uses normal SDK authentication without injecting a gateway header. The per-server initial consent deliberately uses
x-litellm-api-keyand is labeled accordingly; its reconnect uses only the gateway JWT. This does not claim an Authorization-only per-server OAuth dance succeedsRelevant issues
The Linear login is provisioned as
E2E_LINEAR_STORAGE_STATE_B64in the protectede2e-changedenvironment. This PR has no provisioning dependency on LIT-4973. Renew an expired session with the existing capture helper and update that same secretLIT-7737 retains the dependency/Python compatibility matrix. LIT-4506 owns broader isolation/failure regressions and LIT-3559 owns broader auth coverage. No production code, dependency constraints or lockfiles change
Affected release
Linear ticket
Resolves LIT-3467
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@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
This PR only adds tests, so the proof is the new live job running against a real gateway, a real Keycloak login and real Linear consent, with no mocks, plus a probe showing the job turns red when the gateway stops reading back the stored upstream token. The branch has merged current main (
974e4f109c), which already carries the MCP SDK 2 gateway upgrade and the shared E2E lockout fix from #42008, so the After run exercises that gatewayBefore (974e4f1)
git ls-tree --name-only 974e4f109c tests/e2e/mcp/test_mcp_oauth_happy_path_e2e.py tests/e2e/mcp/oauth_gateway.py .github/workflows/test-mcp-oauth-e2e.ymlprints nothing: the happy path test, its owned gateway helper and the workflow do not existgit ls-tree --name-only 974e4f109c tests/e2e/mcp/ | grep -i oauthlists onlyoauth_chat_client.pyandtest_mcp_chat_completion_oauth_e2e.py, which cover the explicit-header route and no gateway restartAfter (d5ac850)
MCP OAuth happy path / oauthjob ran on the pull request at this commit after ane2e-changedapproval: https://github.com/BerriAI/litellm/actions/runs/35472707002tests/e2e/mcp/test_mcp_oauth_happy_path_e2e.pywithE2E_REQUIRED_TEST_COUNT: 4and finished in about two minutes with every step successfultests/e2e/mcp/test_mcp_oauth_happy_path_e2e.py: 4 collected, 0 skippedwith no failed cases<alias>-list_teams, the one-credential ownership check, the upstream bearer comparison at the forwarding edge, and a fresh-client reconnect after a verified cold gateway restart that contacted nolinear.apphostuv run --no-sync pytest tests/code_coverage_tests/test_e2e_changed_gate.py -qreports46 passed, and fromtests/e2e,uv run --no-sync pytest test_provider_edge.py test_idp.py test_proxy_client.py coverage_registry/test_collector.py -qreports369 passedbuildkite/e2e-tests, which is not required, was still running7651d6b550, 43 commits past the merge base, none touching this PR's files): https://github.com/BerriAI/litellm/actions/runs/35474368195, again4 collected, 0 skippedProbe: the job turns red on a broken gateway
To show the User Flow's last step, a throwaway branch off this tip made the gateway's stored-credential read return nothing (
_read_credentialinlitellm/proxy/_experimental/mcp_server/outbound_credentials/per_user_oauth_store.pyreturningNone, every other file unchanged), and the workflow was dispatched on it with the samee2e-changedapproval: https://github.com/BerriAI/litellm/actions/runs/35473850324oauthjob went red about a minute after the pytest step started, and the step "Report JUnit results and reject skipped or missing cases" ended withevery selected file must execute a passing test, with no failures or errorstests/e2e/mcp/test_mcp_oauth_happy_path_e2e.py: 4 collected, 0 skippedfollowed by fourfailed:cases, each withoauth_failure_phase: callaggregate_ssocases reportoauth_exception_type: AssertionErrorwith frames ending attest_mcp_oauth_happy_path_e2e.py:182:test_consent_list_call_and_cold_restartandtest_mcp_oauth_happy_path_e2e.py:56:assert_tool_result, so the first tools/call after consent came back as an error; the twoexplicit_header_jwtcases reportoauth_exception_type: ExceptionGroupwith frames ending atoauth_chat_client.py:304:_list_and_call90687ae597closed Bugbot's reconnect-guard finding: the browser helper now records every request across the browser context and fails the reconnect when anylinear.apphost was contacted, so a fast upstream auto-approve can no longer pass as a consent-free reconnect.d5ac850febadds-Ito the gate test's reporter subprocess for the test-quality rule that landed on mainEarlier runs on this branch:
b7bab56d4dpassed (https://github.com/BerriAI/litellm/actions/runs/35465677503),df7e1fe69dpassed (https://github.com/BerriAI/litellm/actions/runs/35462954636), and4e8a4d4b61failed all four cases within about a minute of the test starting (https://github.com/BerriAI/litellm/actions/runs/35463907681). That failure's private pytest log was removed on exit and the public log carried no assertion text, and the only source change from the passing run was the chat-test constant restore, which the happy-path test does not import. Theb7bab56d4dcommit publishes only failure phase, exception class and code locations for future failuresActual selection against main now includes only the five access-control/JWT canary files; the unchanged OAuth chat test is not selected. No skip gate was weakened and no new exclusion remains for that existing test. A real loopback probe verified gateway header injection and off-origin stripping, including redirects
Management APIs prove credential existence and canonical ownership. A narrow read-only DB inspection after consent and after restart proves encryption and enables exact bearer comparison; the API does not expose plaintext tokens. Observation remains at the live forwarding boundary, not inside Linear
Type
✅ Test
Caveats (if any)
Medium
oauthjob is not a required check and waits for ane2e-changedapproval, like every e2e job on this repo today4e8a4d4b61failed all four variants before the failure-location reporter existed and never reproducedLow
E2E_LINEAR_STORAGE_STATE_B64, so this adds no new dependencytests/e2e/mcp/linear_session_capture.pyand update that secrettest_mcp_chat_completion_oauth_e2e.pydid not run live at this tip, since the selector picks changed test files rather than helpersexplicit_header_jwtvariants exercise the same transportOAuthObservation.gateway_tokenis a JWT the gateway never receives on theaggregate_ssoroute, so its leak check is inert there; the stored-token equality check still covers the forwarded bearerstored_oauthreads the persisted credential through the proxy'sdecrypt_value_helper, so a refactor of that helper fails this test rather than the product/team/member_addleaves a stale team roster cache for up to 60 seconds; the test orders calls around itQA runbook
Shared setup: use the existing database/Prisma and Keycloak setup, a real E2E license, a permanent test salt, and a captured test-account Linear session. Build and stage the dashboard at the tested commit. See
tests/e2e/CONTRIBUTING.mdunder "MCP OAuth happy path" for the exact invocation. The dedicated workflow provisions this environment and requires four non-skipped passestests/e2e/mcp/test_mcp_oauth_happy_path_e2e.py::TestMcpOauthHappyPath::test_consent_list_call_and_cold_restart[direct-aggregate_sso]- an SDK client on the aggregate/mcproute logs in with gateway SSO, consents once at Linear, and still gets team data after a cold gateway restart/mcp, complete gateway SSO and the server card's real upstream consent, then finish connectingisError=falseand valid nonempty teamstests/e2e/mcp/test_mcp_oauth_happy_path_e2e.py::TestMcpOauthHappyPath::test_consent_list_call_and_cold_restart[direct-explicit_header_jwt]- the same journey on the per-server route with the JWT sent inx-litellm-api-key, reconnecting after the restart with only the gateway JWT/<alias>/mcp, with the JWT configured inx-litellm-api-keyfrom client creationtests/e2e/mcp/test_mcp_oauth_happy_path_e2e.py::TestMcpOauthHappyPath::test_consent_list_call_and_cold_restart[observed-aggregate_sso]- the aggregate journey through the live forwarding edge, proving Linear receives the stored upstream token and never the gateway tokentests/e2e/mcp/test_mcp_oauth_happy_path_e2e.py::TestMcpOauthHappyPath::test_consent_list_call_and_cold_restart[observed-explicit_header_jwt]- the explicit JWT journey through the same observer, with the same bearer proof before and after the restartFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/9c61b6dde4e64e83a6f5fe8af173a9dd
Open in Devin Desktop: https://app.devin.ai/desktop/session/9c61b6dde4e64e83a6f5fe8af173a9dd?variant=devin
Note
Low Risk
Test and CI harness changes only; no production gateway or auth logic is modified in this diff.
Overview
Adds live MCP OAuth acceptance for gateway-managed Linear authorization-code flows, with a dedicated protected CI job instead of folding the scenario into the generic changed-e2e selector.
New scenario (
test_mcp_oauth_happy_path_e2e.py) — one parametrized test runs four variants: aggregate gateway SSO vs explicitx-litellm-api-keyJWT, each direct to Linear or observed through the live provider edge (bearer must match decrypted stored upstream token, no gateway leak). Flow covers consent (captured Playwright session), immediatetools/list+list_teams, credential ownership APIs, cold restart of an owned source-built gateway (oauth_gateway.py, no Redis), and reconnect without a second upstream consent. Opt-in viamcp_oauth_live/E2E_MCP_OAUTH_LIVE; tests usingoauth_gatewayskip the shared-proxy liveness gate.Harness & CI —
test-mcp-oauth-e2e.ymlprovisions DB/Keycloak, builds consent UI, materializesE2E_LINEAR_STORAGE_STATE, runs pytest with--reruns 0andE2E_REQUIRED_TEST_COUNT=4.assert_tests_ran.pyfails on wrong count or skips; on OAuth failures it prints only sanitizedoauth_failure_phase,oauth_exception_type, andoauth_frame(conftest attaches these; gate test asserts no token leakage). The happy-path file is excluded fromselect_tests.pyso it only runs in this workflow. Registry addsmcp.call_tool.oauth.persists_across_processes;oauth_chat_client, models,proxy_client.update_team, and provider-edge request observation support the new paths.Reviewed by Cursor Bugbot for commit d5ac850. Bugbot is set up for automated code reviews on this repo. Configure here.