fix(auth): resolve team object_permission independently in the unresolvable-team fallback - #37960
Conversation
…lvable-team fallback When get_team_object fails for a token's team_id, _user_api_key_auth_builder reconstructs a LiteLLM_TeamTableCachedObj from the token's own cached fields, carrying team_object_permission_id but leaving object_permission unset. That silently dropped any vector-store or MCP restriction the team carried, granting more access than the token's own object_permission_id vouches for. Resolve the object permission by its id directly via get_object_permission, independent of the unreadable team row, matching how every other consumer of a team's object_permission (vector store access checks, MCP tool/server resolvers) already treats an unresolvable team as "no restriction at this level" and re-resolves on its own.
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR makes the authentication fallback construct a consistent team authorization object when the team record cannot be resolved.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/user_api_key_auth.py | Adds a focused helper and wires the resolved permission into the existing unresolvable-team fallback. |
| tests/test_litellm/proxy/auth/test_user_api_key_auth.py | Covers both successful permission resolution and graceful handling when the permission record is unavailable. |
Reviews (2): Last reviewed commit: "fix(auth): trim ticket references and na..." | Re-trigger Greptile
…le review Drop the LIT-5539 ticket id from test names and fixture strings, and shorten both the new helper's docstring and the regression test docstrings to their contracts rather than restating the fix's history.
|
@greptileai please review the current head 563f2a1: trimmed the helper and test docstrings to their contracts and dropped the ticket id from names and fixtures. |
TLDR
Problem this solves:
How it solves it:
User Flow
Before: a token whose team's database row becomes unreadable (deleted, or a transient lookup miss with a stale key-level cache) has its effective access silently re-derived without the team's object-permission restriction, even though the restriction still exists in the database
object_permission.vector_storesrestricted to one store, and a key scoped to that teamteam_object_permission_id(the restriction's id) butteam_object_permissionitself isNone, so any code that trusts that field directly sees no restriction at all instead of the one the id namesAfter: the same fallback re-resolves the restriction by its own id, so the returned auth object accurately reflects it
team_object_permissionis the actual restricted permission (naming the one allowed vector store), matchingteam_object_permission_idRelevant issues
Linear ticket
Resolves LIT-5539
Pre-Submission checklist
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
Setup shared by both runs: a real proxy against real Postgres and real Redis (
enable_redis_auth_cache: trueplusgeneral_settings.coordination_redis, so the auth cache is externally inspectable). A team ("owner-team") owns two vector stores,vs-lit5539-allowedandvs-lit5539-forbidden. A second team ("caller-team") is created withobject_permission.vector_stores: ["vs-lit5539-allowed"], and a key is generated scoped to caller-team. One request through the intact team warms both the key-level cache (which bakes inteam_object_permission_idfrom a live join) and the team-level cache. The team-level cache entry is then evicted directly in Redis and the team's row is deleted from Postgres, while the object_permission row is left intact, reproducing the exact scenario: the team is unresolvable but its object_permission id is still known. A temporary debug log (not part of the diff) placed immediately after the fallback assignment recordsteam_object_permission_idand the resolvedteam_object_permissionon the real auth object built for that requestBefore (490c9f9, the merge base this PR is built on; reproduced by locally reverting just this PR's
object_permission=assignment back to that commit's omission of the kwarg, which is behaviorally identical since both leave it at the pydantic default ofNone)GET /vector_store/list against a key whose team became unresolvable
curl $BASE/vector_store/list -H "Authorization: Bearer $CALLER_KEY"returnsvs-lit5539-allowedonly (total_count: 1)LIT5539_PROOF team_object_permission_id=891dfb2b-19e2-4ccc-b839-3e458aba97ba team_object_permission=None, so the auth object built for this request carries the restriction's id but not the restriction itselfcan_user_access_vector_storeindependently re-resolves the permission by id when the field isNone; a caller that truststeam_object_permissiondirectly (as a sibling code path, or any future consumer, already does for the identical key-level field) would see no restriction at allAfter (e4272c6)
GET /vector_store/list against a key whose team became unresolvable
curl $BASE/vector_store/list -H "Authorization: Bearer $CALLER_KEY"returnsvs-lit5539-allowedonly (total_count: 1)LIT5539_PROOF team_object_permission_id=31160444-2bbe-405f-9c46-d88fd0610b3b team_object_permission=object_permission_id='31160444-2bbe-405f-9c46-d88fd0610b3b' ... vector_stores=['vs-lit5539-allowed'] ..., so the auth object now accurately carries the restriction itself, matching its idvs-lit5539-allowed, nevervs-lit5539-forbidden, in either run, so the observable HTTP behavior is unchanged while the underlying auth object is now correct at the source rather than by accident of one consumer's own defensive re-resolutionType
🐛 Bug Fix
Caveats (if any)
None, so this fix closes the gap at the source rather than changing today's HTTP-observable behavior for that one endpoint; the risk it removes is any other or future consumer trustingteam_object_permissiondirectly, the same way the identical key-level field is already trusted elsewhereFinal Attestation