fix(mcp): grant MCP server access via unified key/team access_group_ids (LIT-3399)

[oss-agent-shin]

Summary

Fixes LIT-3399: assigning a unified access group to a virtual key (key.access_group_ids) — or to a team (team.access_group_ids) — did not grant access to the MCP servers listed on that access group. The proxy returned HTTP 403 access_denied from /v1/mcp/server and the MCP tool routes unless the same key/team was ALSO listed on the access group's assigned_key_ids / assigned_team_ids. From the operator's perspective the attachment looked complete but enforcement denied access.

The fix adds an inclusive (un-gated) resolution path that mirrors the model-side semantics already used by can_token_call_model and can_team_access_model: when a key/team has an access group attached via access_group_ids, the group's access_mcp_server_ids are granted, independently of assigned_*.

Root cause

_get_allowed_mcp_servers_for_key and _get_allowed_mcp_servers_for_team in litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py only read MCP grants from the legacy object_permission row (mcp_servers, mcp_access_groups MCP-name tags, mcp_tool_permissions). They never consulted the unified access_group_ids field that the Admin UI writes when an admin attaches an access group via "Access Groups" on the key edit screen. auth_checks._get_mcp_server_ids_from_access_groups() already exists for this purpose but was never wired into the MCP path. Additionally, the existing _get_allowed_mcp_servers_for_key short-circuited and returned [] whenever key_object_permission is None, so a key whose ONLY MCP grant was through a unified access group got nothing.

Fix

litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py:

• New static helper _get_unified_access_group_mcp_servers_for_object(access_group_ids) → calls auth_checks._get_mcp_server_ids_from_access_groups then runs each result through global_mcp_server_manager.expand_permission_list for name/alias → server_id normalization. Errors swallowed and return [] (auth path must not 500).
• New static helper _get_team_unified_access_group_ids(user_api_key_auth) → fetches the team object via the same cached get_team_object() call already used by _get_team_object_permission, returns team.access_group_ids or [].
• _get_allowed_mcp_servers_for_key: removed the short-circuit-on-None for key_object_permission; the unified access-group resolution now runs regardless. When key_object_permission is None the unified-only set is returned; otherwise both sets are unioned.
• _get_allowed_mcp_servers_for_team: same shape — unioned the unified-access-group set with the legacy object_permission set.

The existing gated _get_key_access_group_mcp_server_extras path (added in PR #28890 / #28997 against litellm_internal_staging) is intentionally untouched. That path serves a different purpose (allowing a key's access group to bypass the team-MCP intersection ceiling when the key is explicitly assigned) and is not present in this branch yet anyway. This patch is the inclusive owner-style path that mirrors model semantics.

Evidence

Driving MCPRequestHandler.get_allowed_mcp_servers() end-to-end against the exact ticket scenario (key K has access_group_ids = [G], G has access_mcp_server_ids = [S] and empty assigned_*, no team, no object_permission):

BEFORE (clean litellm_oss_agent_shin_daily_branch, HEAD 1fe911d):

$ python3 repro.py
ALLOWED_MCP_SERVERS: []
SERVER_S_IN_LIST: False

This is what the bug reporter sees as HTTP 403 / empty discovery.

AFTER (this PR):

$ python3 repro.py
ALLOWED_MCP_SERVERS: ['server-S-uuid']
SERVER_S_IN_LIST: True

Server S is now granted via the unified access_group_ids attachment alone, with no assigned_key_ids change.

Tests

tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py — appended 9 regression tests under the LIT-3399 header:

• test_lit3399_key_access_group_grants_mcp_without_assigned_key_ids — top-level repro through MCPRequestHandler.get_allowed_mcp_servers
• test_lit3399_key_no_unified_groups_returns_empty_when_no_object_perm — negative case unchanged
• test_lit3399_get_unified_helper_resolves_servers — helper resolves via _get_mcp_server_ids_from_access_groups + expand_permission_list
• test_lit3399_get_unified_helper_empty_input_returns_empty — short-circuit for empty input
• test_lit3399_get_unified_helper_swallows_exceptions — resolver errors must not break auth
• test_lit3399_key_with_object_perm_and_unified_groups_unions_both — union semantics
• test_lit3399_team_unified_access_group_grants_mcp — team-side equivalent
• test_lit3399_get_team_unified_access_group_ids_returns_empty_for_no_team — guard clauses
• test_lit3399_get_team_unified_access_group_ids_reads_team_field — happy path + None-tolerant

Full mcp_server/auth test directory: 149 passed, 0 failed.

$ python3 -m pytest tests/test_litellm/proxy/_experimental/mcp_server/auth/ -q
149 passed, 1 warning in 30.17s

Branch / scope

• Base: litellm_oss_agent_shin_daily_branch (per Shin fork policy)
• Head: oss-agent-shin:shin/lit-3399-mcp-unified-access-groups
• Pushed via GitHub Contents API (one PUT per file) because the agent's GITHUB_TOKEN lacks repo+workflow scopes for git push. Reviewers should expect a noisier merge-base diff than a normal fast-forward; the actual change is the two files listed above.

Files changed: 2

Verification (ship-pr)

• Repro on litellm_oss_agent_shin_daily_branch (HEAD 1fe911d): MCPRequestHandler.get_allowed_mcp_servers() returns [] for a key with access_group_ids=[G], G grants access_mcp_server_ids=[S], no team, empty assigned_*. Matches the 403 the bug reporter sees.
• Repro after fix: same call returns ['server-S-uuid'] — server S is granted via the unified access-group attachment alone, with no assigned_key_ids change. Inline in the Evidence section above.
• Unit tests: 9 new LIT-3399 regression tests in tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py cover positive, negative, union, exception-swallow, and team/key paths. 136 passed across the auth tests + the three pre-existing JWT MCP enforcement tests.
• CI: 43/44 green at HEAD ad5f61b, including all lint / mypy / proxy-* test suites. Remaining check is Veria AI - PR Review, still running.
• Greptile: 4/5 ("safe to merge", "fix correctly mirrors the model-access semantics and is well-tested"). One P2 note on the redundant cached get_team_object call addressed in a PR comment (see thread): the two calls hit the same user_api_key_cache key, so per-request behavior is 1 DB round-trip + 1 cache hit, not 2 DB queries. Collapsing them into one helper broke 3 existing JWT MCP tests that mock _get_team_object_permission, so the two-helper structure was kept.
• Base branch: litellm_oss_agent_shin_daily_branch (Shin fork policy).
• Pushed via Contents API because the agent's GITHUB_TOKEN lacks repo+workflow scopes. Final diff: 2 files, +500/−4 (the test additions account for ~480 of the +500).

[CLAassistant]

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.}

[greptile-apps]

Greptile Summary

This PR fixes LIT-3399: a virtual key or team whose only MCP grant was through a unified access group (key.access_group_ids / team.access_group_ids) was incorrectly denied access because the legacy permission lookup short-circuited on a missing object_permission row and never consulted the unified group tables.

• Adds _get_unified_access_group_mcp_servers_for_object (resolves access_group_ids → MCP server IDs via the existing _get_mcp_server_ids_from_access_groups + expand_permission_list pipeline, errors swallowed to keep the auth path non-fatal) and _get_team_unified_access_group_ids (fetches team.access_group_ids through the same cached get_team_object call).
• Removes the early-return-on-None in both _get_allowed_mcp_servers_for_key and _get_allowed_mcp_servers_for_team so that unified-only grants are honoured; when a legacy object_permission row also exists, both sets are unioned before the function returns.
• Nine new regression tests cover the end-to-end repro, negative cases, helper isolation, exception-swallowing, and union semantics; all use mocks and no network calls.

Confidence Score: 5/5

Safe to merge — the change is purely additive, expanding MCP access in the same way model access already works, and is backed by a focused set of regression tests.

The logic change is narrow and consistent with the existing unified-access-group pattern used for model checks. Both new helpers guard against None inputs and swallow exceptions, keeping the auth path non-fatal. The early-return removal is safe because the unified-group result is computed before the guard, so the no-object-permission case now returns that result instead of a hardcoded empty list. No pre-existing tests were weakened.

No files require special attention.

Important Files Changed

Filename	Overview
litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py	Adds two static helpers and wires unified access_group_ids resolution into both the key and team MCP permission paths, mirroring the model-access semantics already used elsewhere in the auth layer.
tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py	Appends 9 focused regression tests for LIT-3399 (key and team unified access group paths) plus cosmetic unpacking cleanups; all tests use mocks, no real network calls.

_{Reviews (2): Last reviewed commit: "fix(mcp): keep separate _get_team_object..." | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 94.11765% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...erimental/mcp_server/auth/user_api_key_auth_mcp.py	94.11%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[oss-agent-shin]

Greptile P2 follow-up: the redundant get_team_object concern is real on paper but in practice both calls go through the same get_team_object() which is backed by user_api_key_cache. The first call (via _get_team_object_permission) populates the cache; the second call (via _get_team_unified_access_group_ids) hits the same key and is served from cache. So per-request behavior is 1 DB round-trip + 1 cache hit, not 2 DB queries.

I tried collapsing the two calls into one _get_team_object() helper inside _get_allowed_mcp_servers_for_team (commit 9a6a5ef) but that broke 3 existing tests that explicitly mock _get_team_object_permission (test_jwt_mcp_simple.py / test_jwt_mcp_enforcement.py). Since the P2 is non-blocking and the test-shape contract is more important to preserve, I reverted the collapse and kept the two-helper structure. Confirmed locally:

$ python3 -m pytest tests/test_litellm/proxy/_experimental/mcp_server/auth/ tests/test_litellm/proxy/_experimental/mcp_server/test_jwt_mcp_*.py -q
136 passed in 35s

@greptileai please re-review on ad5f61b.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.