fix(mcp): authorize UI sessions as live users - #34585
Conversation
|
|
Greptile SummaryThe PR refreshes UI-session MCP team membership directly from the database
Confidence Score: 4/5The PR is safe to merge, with non-blocking cleanup needed for the new inline comment and the request-path database cost The database-only lookup refreshes grants as intended, but it bypasses the user cache on UI-session MCP requests and introduces an inline comment prohibited by repository guidance Files Needing Attention: litellm/proxy/_experimental/mcp_server/ui_session_utils.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/ui_session_utils.py | Forces live database-backed team resolution, adding per-request database work and a new inline comment that conflicts with repository guidance |
| tests/test_litellm/proxy/_experimental/mcp_server/test_ui_session_utils.py | Adds focused assertions that database-only resolution is requested and that a subsequent lookup observes revoked team membership |
Reviews (1): Last reviewed commit: "fix(mcp): refresh UI session teams from ..." | Re-trigger Greptile
| # UI-session authorization must observe grants, revocations, and | ||
| # offboarding on the next request. A cached user row can retain a | ||
| # pre-login or pre-reconciliation team list until its management | ||
| # TTL expires, which makes both grants and revocations stale. | ||
| check_db_only=True, |
There was a problem hiding this comment.
This explanatory comment violates the repository-wide instruction against adding code comments unless explicitly requested, creating avoidable policy cleanup for an otherwise self-contained change
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| # pre-login or pre-reconciliation team list until its management | ||
| # TTL expires, which makes both grants and revocations stale. | ||
| check_db_only=True, | ||
| parent_otel_span=user_api_key_auth.parent_otel_span, |
There was a problem hiding this comment.
Database lookup enters request path
Setting check_db_only=True bypasses the user cache for every UI-session MCP discovery and tool call, adding a database query and its latency to each request while increasing database load under concurrent use
Rule Used: What: Avoid creating new database requests or Rout... (source)
Knowledge Base Used: Proxy Authentication and Authorization
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| from litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp import ( | ||
| MCPRequestHandler, | ||
| ) |
Summary
Why
With
require_key_mcp_access_defined: true, cloning alitellm-dashboardUI session into an employee team still applies the ordinary virtual-key ceiling. Because that session key intentionally has no key-level MCP object permission, every team server is hidden even when the employee is correctly provisioned. The DCR bridge already has a server-only admitted-user path that resolves the signed-in employee against live policy; UI discovery should use the same path.This also closes the stale-user-cache portion of #34566 by using database-only user loads at initial admission and team fan-out.
Tests
pytest -q tests/test_litellm/proxy/_experimental/mcp_server/test_ui_session_utils.py tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py(302 passed)Closes the remaining UI-session authorization portion of #34566.