Repository navigation
Revert "fix(auth): stop the team fallback from widening model access" (#36837) - #36982
Conversation
…36837)" This reverts commit ab2333b. Every Admin UI login mints its session key against the sentinel team_id `litellm-dashboard`, and no LiteLLM_TeamTable row is ever created for it. That lookup is therefore a provably-absent row on every UI request, which #36837 turned into a hard refusal with no override, so the whole dashboard 404s. Reverting restores the token-derived fallback. The model-access widening #36837 closed is reopened and needs a re-land that exempts the UI sentinel team.
Greptile SummaryThe PR restores Admin UI access by reverting the missing-team refusal, but it restores the fallback for every team rather than only the UI sentinel
Confidence Score: 3/5The PR is unsafe to merge until the fallback is restricted to the intended Admin UI sentinel Ordinary missing teams can again be reconstructed from token defaults, and empty model grants are treated as unrestricted access Files Needing Attention: litellm/proxy/auth/user_api_key_auth.py, litellm/proxy/auth/auth_checks.py, tests/test_litellm/proxy/auth/test_user_api_key_auth.py
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/user_api_key_auth.py | Restores unconditional token-derived team fallback, allowing missing ordinary teams to retain permissive authorization state |
| litellm/proxy/auth/auth_checks.py | Removes the exception distinction between definitively absent teams and other lookup failures |
| tests/test_litellm/proxy/auth/test_user_api_key_auth.py | Deletes regression tests that exercised missing-team refusal and restricted model grants |
| tests/test_litellm/proxy/auth/test_auth_checks.py | Removes coverage distinguishing an absent team row from an unreadable lookup |
| tests/proxy_unit_tests/test_user_api_key_auth.py | Removes explicit team model grants from two fixtures to match the restored permissive fallback behavior |
Reviews (1): Last reviewed commit: "Revert "fix(auth): stop the team fallbac..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| team_object = _team_obj_from_token(user_api_key_auth_obj) | ||
| else: | ||
| raise team_result | ||
| team_object = _team_obj_from_token(user_api_key_auth_obj) if user_api_key_auth_obj.team_id is not None else None |
There was a problem hiding this comment.
When an ordinary key references a missing team, token fallback treats empty team_models as unrestricted, allowing models the team did not authorize
How this was verified: The reconstructed team reaches common_checks, where an empty model list grants all-model access
Rule Used: What: Fail any PR which may contains a security in... (source)
Knowledge Base Used: Proxy Authentication and Authorization
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| team_object = _team_obj_from_token(user_api_key_auth_obj) | ||
| else: | ||
| raise team_result | ||
| team_object = _team_obj_from_token(user_api_key_auth_obj) if user_api_key_auth_obj.team_id is not None else None |
There was a problem hiding this comment.
High: Team authorization fails open
A caller whose token references an absent or unreadable team can request models the team was not permitted to use because this fallback reconstructs the team with team_models=[], which the model check interprets as access to every model. Preserve the distinction between a missing team and a degraded database read, and handle the dashboard sentinel explicitly rather than falling back for every lookup error; other fallbacks should require a non-empty token-side grant or an explicit fail-open setting.
PR overviewThis PR reverts the earlier team-fallback change in authentication, restoring construction of a fallback team context from token data when a team lookup is absent or unreadable. One high-impact authorization issue remains open. Tokens referencing a missing or unreadable team can receive unrestricted model access because an empty fallback model list is interpreted as allowing every model, enabling callers to use models their team was not permitted to access. No issues have yet been addressed. Open issues (1)
Fixed/addressed: 0 · PR risk: 8/10 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
652f4cb
into
litellm_internal_staging
2 participants
TLDR
Reverts #36837 (
ab2333b6c4), which locks every user out of the Admin UI.Root cause
Every Admin UI login mints its session key against the sentinel team id
litellm-dashboard— both the password/SSO path(
litellm/proxy/auth/login_utils.py,"team_id": "litellm-dashboard") and theEXPERIMENTAL_UI_LOGINJWT path (ExperimentalUIJWTToken). NoLiteLLM_TeamTablerow is ever created for it; it is a sentinel, and other callsites already special-case it as "not a real team" (
UI_TEAM_IDin the MCP andagent permission handlers).
So on every UI request the team lookup asks the database for
litellm-dashboard, the database answers, and the row is not there. That is theexact state #36837 reclassified:
_get_team_object_from_user_api_key_cacheraises the new
TeamNotFoundError, and_token_can_vouch_for_teamrefuses itoutright, with no setting able to override it. The session key also carries
models=[]and no team grant, so even the second branch would not have vouched.The result is a 404 on every Admin-UI-authenticated request.
Proof
Both legs drive the real
get_team_objectwith a prisma stub whosefind_uniquereturnsNone— the actual absent-row state — and feed whateverthat lookup really raises into the real
_run_centralized_common_checks, withthe token shape the UI login mints. Nothing is hand-constructed or injected.
Before, at
29fe342ead(staging, with #36837)After, with this revert
The lookup still fails in both legs — the difference is only whether the
token-derived fallback is allowed to stand in for the sentinel team.
Tests
Every test file #36837 touched, on this revert:
The revert is the exact inverse of
ab2333b6c4(308 deletions, 1 insertionagainst its 308/1). One conflict had to be resolved by hand: a later commit
added
delete_cache_key_objectsimmediately below whereTeamNotFoundErrorwasinserted. That function is unrelated and is kept.
What this reopens, stated plainly
#36837 closed a real hole: a key whose team is deleted keeps working and reaches
models the team never allowed, because
team_models=[]on the token reads as"every model". This revert reopens that. It is the right trade right now because
the dashboard is down for everyone, but it should not be left open.
The re-land needs the UI sentinel exempted before the refusal is reinstated —
team_id == UI_TEAM_IDmust never reach the absent-team refusal, since it isabsent by design rather than deleted. Worth checking the same question for any
other synthetic team id in the auth path.
Type
🐛 Bug Fix
Caveats (if any)