mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(auth): stop the team fallback from widening model access (#37962)
When get_team_object fails, the centralized auth gate rebuilds the team from the token's own fields. A token whose team row was missing when the key was read carries team_models=[] and team_blocked=False, and the model-access check reads an empty model list as every model, so the rebuilt team grants more than the real team ever did. get_team_object reported a deleted team and a database that would not answer as the same 404, so the fallback could not tell a definitive answer from a degraded read. Raise a TeamNotFoundError subclass, still a 404 with the same detail so every other caller is unaffected, only when the database answers and the row is absent. A team that is provably gone now refuses, and no setting overrides that. Otherwise the grant is merely unknown: a token carrying one may vouch, since replaying a recorded grant cannot widen it, and a token carrying none may not. allow_requests_on_db_unavailable still opts back out there, and is only consulted once the failure is known to be a degraded read. The Admin UI mints every session key against the UI_TEAM_ID sentinel, which by design never has a team row, so every UI request hit the new refusal with no override. Exempt UI_TEAM_ID explicitly so it keeps reconstructing from the token unconditionally, matching how the MCP handler and agent_permission_handler already special-case it. Resolves LIT-5522
This commit is contained in:
parent
15510f0b8b
commit
ba876c98e6
5 changed files with 392 additions and 1 deletions
|
|
@ -19,6 +19,7 @@ from typing import TYPE_CHECKING, Any, Final, Literal, Optional, Protocol, cast
|
|||
|
||||
from fastapi import HTTPException, Request, status
|
||||
from pydantic import BaseModel
|
||||
from typing_extensions import ReadOnly, TypedDict
|
||||
|
||||
import litellm
|
||||
from litellm._logging import verbose_proxy_logger
|
||||
|
|
@ -2512,6 +2513,27 @@ async def delete_cache_key_objects(
|
|||
await publish_auth_cache_invalidation(cache_key=hashed_token)
|
||||
|
||||
|
||||
class _TeamNotFoundDetail(TypedDict):
|
||||
error: ReadOnly[str]
|
||||
|
||||
|
||||
class TeamNotFoundError(HTTPException):
|
||||
"""The team row is provably absent, as opposed to merely unreadable.
|
||||
|
||||
``get_team_object`` reports every failure as a 404, so a deleted team and a
|
||||
database that would not answer are indistinguishable to its callers. Callers
|
||||
that must not treat a degraded read as a definitive answer, such as the
|
||||
authorization fallback in ``user_api_key_auth``, key on this subclass. It
|
||||
stays a 404 carrying the same detail, so every other caller is unaffected.
|
||||
"""
|
||||
|
||||
def __init__(self, team_id: str) -> None:
|
||||
detail: Final[_TeamNotFoundDetail] = {
|
||||
"error": f"Team doesn't exist in db. Team={team_id}. Create team via `/team/new` call."
|
||||
}
|
||||
super().__init__(status_code=404, detail=detail)
|
||||
|
||||
|
||||
@log_db_metrics
|
||||
async def _get_team_db_check(
|
||||
team_id: str, prisma_client: PrismaClient, team_id_upsert: bool | None = None
|
||||
|
|
@ -2557,6 +2579,10 @@ async def _get_team_object_from_user_api_key_cache(
|
|||
)
|
||||
if should_check_db:
|
||||
response = await _get_team_db_check(team_id=team_id, prisma_client=prisma_client, team_id_upsert=team_id_upsert)
|
||||
# The database answered and the row is not there. Distinct from every
|
||||
# other failure here, which leaves the team's grant unknown.
|
||||
if response is None:
|
||||
raise TeamNotFoundError(team_id=team_id)
|
||||
else:
|
||||
response = None
|
||||
|
||||
|
|
@ -2678,6 +2704,8 @@ async def get_team_object(
|
|||
key=key,
|
||||
team_id_upsert=team_id_upsert,
|
||||
)
|
||||
except TeamNotFoundError:
|
||||
raise
|
||||
except Exception:
|
||||
raise HTTPException(
|
||||
status_code=404,
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ from litellm.litellm_core_utils.dot_notation_indexing import get_nested_value
|
|||
from litellm.proxy._types import *
|
||||
from litellm.proxy.auth.auth_checks import (
|
||||
ExperimentalUIJWTToken,
|
||||
TeamNotFoundError,
|
||||
_cache_key_object,
|
||||
_can_object_call_model,
|
||||
_check_end_user_budget,
|
||||
|
|
@ -87,6 +88,7 @@ from litellm.proxy.common_utils.http_parsing_utils import (
|
|||
)
|
||||
from litellm.proxy.common_utils.realtime_utils import _realtime_request_body
|
||||
from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache
|
||||
from litellm.proxy.db.exception_handler import PrismaDBExceptionHandler
|
||||
from litellm.proxy.litellm_pre_call_utils import LiteLLMProxyRequestSetup
|
||||
from litellm.proxy.utils import (
|
||||
PrismaClient,
|
||||
|
|
@ -2290,6 +2292,36 @@ def _team_obj_from_token(valid_token: UserAPIKeyAuth) -> LiteLLM_TeamTableCached
|
|||
)
|
||||
|
||||
|
||||
def _token_can_vouch_for_team(valid_token: UserAPIKeyAuth, lookup_error: BaseException) -> bool:
|
||||
"""Whether the token's own team fields may stand in for a team that failed to
|
||||
resolve, without widening access.
|
||||
|
||||
The UI dashboard mints every session key against the ``UI_TEAM_ID`` sentinel,
|
||||
which by design never has a team row, so a failed lookup for it is not a
|
||||
degraded read to be treated with suspicion; it always vouches, exactly as it
|
||||
always safely has (these keys are restricted elsewhere to UI-only routes).
|
||||
|
||||
For every other team, a team that is provably gone is a definitive answer,
|
||||
not a degraded read, so nothing may stand in for it and no setting may
|
||||
override that.
|
||||
|
||||
Otherwise the team's grant is merely unknown. A token carrying one may vouch,
|
||||
since replaying a recorded grant cannot widen it and denying every team key
|
||||
while the row is briefly unreadable would trade the widening for an outage. A
|
||||
token carrying none may not: ``team_models=[]`` reads as every model and
|
||||
``team_blocked=False`` as unblocked. ``allow_requests_on_db_unavailable`` opts
|
||||
back out, and is only consulted here because the failure is known by this
|
||||
point to be a degraded read.
|
||||
"""
|
||||
if valid_token.team_id == UI_TEAM_ID:
|
||||
return True
|
||||
if isinstance(lookup_error, TeamNotFoundError):
|
||||
return False
|
||||
if valid_token.team_models:
|
||||
return True
|
||||
return PrismaDBExceptionHandler.should_allow_request_on_db_unavailable()
|
||||
|
||||
|
||||
@tracer.wrap()
|
||||
async def _run_centralized_common_checks(
|
||||
user_api_key_auth_obj: UserAPIKeyAuth,
|
||||
|
|
@ -2494,7 +2526,12 @@ async def _run_centralized_common_checks(
|
|||
if isinstance(team_result, BaseException):
|
||||
# Token-derived fallback only valid when a team_id is set;
|
||||
# _team_obj_from_token asserts that precondition.
|
||||
team_object = _team_obj_from_token(user_api_key_auth_obj) if user_api_key_auth_obj.team_id is not None else None
|
||||
if user_api_key_auth_obj.team_id is None:
|
||||
team_object = None
|
||||
elif _token_can_vouch_for_team(user_api_key_auth_obj, team_result):
|
||||
team_object = _team_obj_from_token(user_api_key_auth_obj)
|
||||
else:
|
||||
raise team_result
|
||||
else:
|
||||
team_object = team_result
|
||||
|
||||
|
|
|
|||
|
|
@ -154,6 +154,7 @@ async def test_team_object_has_object_permission_id():
|
|||
token=hashed_key,
|
||||
last_refreshed_at=time.time(),
|
||||
team_object_permission_id=permission_id,
|
||||
team_models=["gpt-4o"],
|
||||
)
|
||||
user_api_key_cache.set_cache(key=hashed_key, value=valid_token)
|
||||
|
||||
|
|
@ -242,6 +243,7 @@ async def test_aaauser_personal_budgets(key_ownership):
|
|||
user_id=_user_id,
|
||||
team_id="my-special-team",
|
||||
team_max_budget=100,
|
||||
team_models=["gpt-4o"],
|
||||
spend=20,
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -2455,6 +2455,53 @@ async def test_get_team_object_raises_404_when_not_found():
|
|||
assert "Team doesn't exist in db" in str(exc_info.value.detail)
|
||||
|
||||
|
||||
def _mock_prisma_for_team_lookup(find_unique):
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
mock_prisma_client = MagicMock()
|
||||
mock_prisma_client.db.litellm_teamtable.find_unique = find_unique
|
||||
return mock_prisma_client
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_team_object_distinguishes_absent_team_from_unreadable_row():
|
||||
"""A deleted team and a database that would not answer both surface as a 404,
|
||||
which leaves callers unable to tell a definitive answer from a degraded read.
|
||||
Only the row being positively absent raises the subclass; anything else keeps
|
||||
the plain 404 so every existing caller is unaffected."""
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
from fastapi import HTTPException
|
||||
|
||||
from litellm.proxy.auth.auth_checks import TeamNotFoundError, get_team_object
|
||||
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.async_get_cache = AsyncMock(return_value=None)
|
||||
|
||||
# The database answered, and the row is not there.
|
||||
with pytest.raises(TeamNotFoundError) as absent_info:
|
||||
await get_team_object(
|
||||
team_id="absent-team-lit5522",
|
||||
prisma_client=_mock_prisma_for_team_lookup(AsyncMock(return_value=None)),
|
||||
user_api_key_cache=mock_cache,
|
||||
check_db_only=True,
|
||||
)
|
||||
assert absent_info.value.status_code == 404
|
||||
assert "Team doesn't exist in db" in str(absent_info.value.detail)
|
||||
|
||||
# The database did not answer. Same status and detail, but not the subclass,
|
||||
# so a caller keying on it does not read this as proof the team is gone.
|
||||
with pytest.raises(HTTPException) as unreadable_info:
|
||||
await get_team_object(
|
||||
team_id="unreadable-team-lit5522",
|
||||
prisma_client=_mock_prisma_for_team_lookup(AsyncMock(side_effect=ConnectionError("db unreachable"))),
|
||||
user_api_key_cache=mock_cache,
|
||||
check_db_only=True,
|
||||
)
|
||||
assert unreadable_info.value.status_code == 404
|
||||
assert not isinstance(unreadable_info.value, TeamNotFoundError)
|
||||
|
||||
|
||||
# Reject Client-Side Metadata Tags Tests
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -4693,6 +4693,283 @@ async def test_centralized_common_checks_team_404_does_not_zero_other_contexts()
|
|||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_centralized_common_checks_unresolvable_team_without_grant_is_refused():
|
||||
"""The store restricts the team to gpt-4o-mini and the read of it fails, so the
|
||||
only surviving team record is the token's own, which carries ``team_models=[]``
|
||||
and reads as every model. The request must be refused with the original lookup
|
||||
error. Pre-fix it was served."""
|
||||
import litellm.proxy.proxy_server as _proxy_server_mod
|
||||
from fastapi import HTTPException, Request
|
||||
from starlette.datastructures import URL
|
||||
|
||||
token = UserAPIKeyAuth(
|
||||
api_key="sk-test",
|
||||
team_id="restricted-team",
|
||||
models=[],
|
||||
team_models=[],
|
||||
)
|
||||
request = Request(scope={"type": "http"})
|
||||
request._url = URL(url="/chat/completions")
|
||||
request._body = json.dumps({"model": "gpt-4.1"}).encode()
|
||||
|
||||
team_read_failure = HTTPException(
|
||||
status_code=404,
|
||||
detail={"error": "Team doesn't exist in db. Team=restricted-team."},
|
||||
)
|
||||
|
||||
attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None)
|
||||
originals = {a: getattr(_proxy_server_mod, a, None) for a in attrs}
|
||||
try:
|
||||
for k, v in attrs.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
with patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.get_team_object",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=team_read_failure,
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await _run_centralized_common_checks(
|
||||
user_api_key_auth_obj=token,
|
||||
request=request,
|
||||
request_data={"model": "gpt-4.1"},
|
||||
route="/chat/completions",
|
||||
)
|
||||
assert exc_info.value is team_read_failure
|
||||
finally:
|
||||
for k, v in originals.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("token_team_models", [[], ["gpt-4.1"]])
|
||||
async def test_centralized_common_checks_absent_team_refused_despite_db_unavailable_optout(token_team_models):
|
||||
"""A team that is provably gone is a definitive answer, not a degraded read.
|
||||
``allow_requests_on_db_unavailable`` is a static settings read, so without the
|
||||
absent-versus-unreadable distinction it would hand a deleted team's key the
|
||||
old permissive fallback while the database is perfectly healthy. Refused in
|
||||
both token shapes, including the one whose grant would otherwise vouch.
|
||||
|
||||
Imported from the module under test rather than from ``auth_checks``: other
|
||||
tests in this suite ``importlib.reload`` that module, which rebinds the class
|
||||
and would leave this raising a type the guard has never seen."""
|
||||
import litellm.proxy.proxy_server as _proxy_server_mod
|
||||
from fastapi import HTTPException, Request
|
||||
from starlette.datastructures import URL
|
||||
|
||||
from litellm.proxy.auth.user_api_key_auth import TeamNotFoundError
|
||||
|
||||
token = UserAPIKeyAuth(
|
||||
api_key="sk-test",
|
||||
team_id="deleted-team",
|
||||
models=[],
|
||||
team_models=token_team_models,
|
||||
)
|
||||
request = Request(scope={"type": "http"})
|
||||
request._url = URL(url="/chat/completions")
|
||||
request._body = json.dumps({"model": "gpt-4.1"}).encode()
|
||||
|
||||
team_absent = TeamNotFoundError(team_id="deleted-team")
|
||||
|
||||
attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None)
|
||||
attrs["general_settings"] = {"allow_requests_on_db_unavailable": True}
|
||||
originals = {a: getattr(_proxy_server_mod, a, None) for a in attrs}
|
||||
try:
|
||||
for k, v in attrs.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
with patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.get_team_object",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=team_absent,
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await _run_centralized_common_checks(
|
||||
user_api_key_auth_obj=token,
|
||||
request=request,
|
||||
request_data={"model": "gpt-4.1"},
|
||||
route="/chat/completions",
|
||||
)
|
||||
assert exc_info.value is team_absent
|
||||
finally:
|
||||
for k, v in originals.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_centralized_common_checks_unreadable_team_keeps_db_unavailable_optout():
|
||||
"""The counterpart: an unreadable team leaves the grant unknown rather than
|
||||
answered, so an operator who has accepted degraded authorization during a
|
||||
database fault still gets the fallback. Without this the fix would trade the
|
||||
widening for a lockout with no way out."""
|
||||
import litellm.proxy.proxy_server as _proxy_server_mod
|
||||
from fastapi import HTTPException as _HTTPException
|
||||
from fastapi import Request
|
||||
from starlette.datastructures import URL
|
||||
|
||||
from litellm.proxy._types import LiteLLM_TeamTableCachedObj
|
||||
|
||||
token = UserAPIKeyAuth(api_key="sk-test", team_id="unreadable-team", models=[], team_models=[])
|
||||
request = Request(scope={"type": "http"})
|
||||
request._url = URL(url="/chat/completions")
|
||||
request._body = json.dumps({"model": "gpt-4.1"}).encode()
|
||||
|
||||
received_team_objects: list[LiteLLM_TeamTableCachedObj | None] = []
|
||||
|
||||
async def _capturing_common_checks(*_args, **kwargs) -> bool:
|
||||
received_team_objects.append(kwargs.get("team_object"))
|
||||
return True
|
||||
|
||||
attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None)
|
||||
attrs["general_settings"] = {"allow_requests_on_db_unavailable": True}
|
||||
originals = {a: getattr(_proxy_server_mod, a, None) for a in attrs}
|
||||
try:
|
||||
for k, v in attrs.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.get_team_object",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=_HTTPException(status_code=404, detail={"error": "team unreadable"}),
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.common_checks",
|
||||
_capturing_common_checks,
|
||||
),
|
||||
):
|
||||
await _run_centralized_common_checks(
|
||||
user_api_key_auth_obj=token,
|
||||
request=request,
|
||||
request_data={"model": "gpt-4.1"},
|
||||
route="/chat/completions",
|
||||
)
|
||||
assert len(received_team_objects) == 1
|
||||
received_team_object = received_team_objects[0]
|
||||
assert received_team_object is not None
|
||||
assert received_team_object.team_id == "unreadable-team"
|
||||
finally:
|
||||
for k, v in originals.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"requested_model, is_granted",
|
||||
[("gpt-4o-mini", True), ("gpt-4.1", False)],
|
||||
)
|
||||
async def test_centralized_common_checks_unresolvable_team_with_grant_enforces_it(requested_model, is_granted):
|
||||
"""Mirror of the refusal above: a token that does carry a team model grant keeps
|
||||
the fallback, and the reconstructed team must still enforce that grant rather
|
||||
than wave the request through."""
|
||||
import litellm.proxy.proxy_server as _proxy_server_mod
|
||||
from fastapi import HTTPException, Request
|
||||
from starlette.datastructures import URL
|
||||
|
||||
from litellm.proxy._types import ProxyErrorTypes, ProxyException
|
||||
|
||||
token = UserAPIKeyAuth(
|
||||
api_key="sk-test",
|
||||
team_id="restricted-team",
|
||||
models=[],
|
||||
team_models=["gpt-4o-mini"],
|
||||
)
|
||||
request = Request(scope={"type": "http"})
|
||||
request._url = URL(url="/chat/completions")
|
||||
request._body = json.dumps({"model": requested_model}).encode()
|
||||
|
||||
attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None)
|
||||
originals = {a: getattr(_proxy_server_mod, a, None) for a in attrs}
|
||||
try:
|
||||
for k, v in attrs.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
with patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.get_team_object",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=HTTPException(status_code=404, detail={"error": "team unreadable"}),
|
||||
):
|
||||
if is_granted:
|
||||
await _run_centralized_common_checks(
|
||||
user_api_key_auth_obj=token,
|
||||
request=request,
|
||||
request_data={"model": requested_model},
|
||||
route="/chat/completions",
|
||||
)
|
||||
else:
|
||||
with pytest.raises(ProxyException) as exc_info:
|
||||
await _run_centralized_common_checks(
|
||||
user_api_key_auth_obj=token,
|
||||
request=request,
|
||||
request_data={"model": requested_model},
|
||||
route="/chat/completions",
|
||||
)
|
||||
assert exc_info.value.type == ProxyErrorTypes.team_model_access_denied
|
||||
finally:
|
||||
for k, v in originals.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_centralized_common_checks_ui_sentinel_team_vouches_despite_absent_row():
|
||||
"""The Admin UI mints every session key against the ``UI_TEAM_ID`` sentinel,
|
||||
which by design never has a ``LiteLLM_TeamTable`` row, so ``get_team_object``
|
||||
always raises ``TeamNotFoundError`` for it. That must NOT be read as "team
|
||||
provably gone, refuse" the way it is for a real team_id: PR #36837 made that
|
||||
exact mistake and PR #36982 reverted it because every dashboard request
|
||||
404'd. The sentinel must keep vouching from the token unconditionally."""
|
||||
import litellm.proxy.proxy_server as _proxy_server_mod
|
||||
from fastapi import Request
|
||||
from starlette.datastructures import URL
|
||||
|
||||
from litellm.proxy._types import UI_TEAM_ID, LiteLLM_TeamTableCachedObj
|
||||
from litellm.proxy.auth.user_api_key_auth import TeamNotFoundError
|
||||
|
||||
token = UserAPIKeyAuth(
|
||||
api_key="sk-test",
|
||||
user_id="ui-session-user",
|
||||
team_id=UI_TEAM_ID,
|
||||
models=[],
|
||||
team_models=[],
|
||||
)
|
||||
request = Request(scope={"type": "http"})
|
||||
request._url = URL(url="/user/info")
|
||||
request._body = b"{}"
|
||||
|
||||
received_team_objects: list[LiteLLM_TeamTableCachedObj | None] = []
|
||||
|
||||
async def _capturing_common_checks(*_args, **kwargs) -> bool:
|
||||
received_team_objects.append(kwargs.get("team_object"))
|
||||
return True
|
||||
|
||||
attrs = _proxy_attrs_for_centralized_checks(user_custom_auth=None)
|
||||
originals = {a: getattr(_proxy_server_mod, a, None) for a in attrs}
|
||||
try:
|
||||
for k, v in attrs.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
with (
|
||||
patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.get_team_object",
|
||||
new_callable=AsyncMock,
|
||||
side_effect=TeamNotFoundError(team_id=UI_TEAM_ID),
|
||||
),
|
||||
patch(
|
||||
"litellm.proxy.auth.user_api_key_auth.common_checks",
|
||||
_capturing_common_checks,
|
||||
),
|
||||
):
|
||||
await _run_centralized_common_checks(
|
||||
user_api_key_auth_obj=token,
|
||||
request=request,
|
||||
request_data={},
|
||||
route="/user/info",
|
||||
)
|
||||
assert len(received_team_objects) == 1
|
||||
received_team_object = received_team_objects[0]
|
||||
assert received_team_object is not None
|
||||
assert received_team_object.team_id == UI_TEAM_ID
|
||||
finally:
|
||||
for k, v in originals.items():
|
||||
setattr(_proxy_server_mod, k, v)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_centralized_common_checks_user_http_exception_isolates_to_user_only():
|
||||
"""Per-fetch isolation, mirror of the team case: an HTTPException
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue