From bd8511dd5106c449035296e5c48f7cd78d2157fd Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Wed, 29 Jul 2026 23:58:41 -0700 Subject: [PATCH] refactor: extract user_is_scim_deactivated as one shared predicate The SCIM-deactivation check was spelled out inline at five call sites (the standard auth builder, both MCP admission arms, and both bridge-refresh revalidation paths). Each one re-derived the same three conditions, and cache warming, which resolves a user through get_user_object exactly like those five do, shipped without the check at all; a sixth caller getting it wrong is what a copy-pasted predicate guarantees eventually. The predicate now lives beside get_user_object in auth_checks.py, since the resolver is what hands back a live-looking row for a deactivated user and every caller owes the check afterwards. It returns a bool rather than raising, so each caller keeps its own reaction: the standard builder raises, the MCP arms return 401, the bridge returns a status, and the warming refresher raises a ProxyException before it can spend against a deactivated owner's key. Only an explicit scim_active of False deactivates, so a missing user or absent metadata still fails open exactly as before at every site. --- .../mcp_server/auth/user_api_key_auth_mcp.py | 14 ++++++++------ .../mcp_server/bridge_token_flow.py | 6 ++++-- litellm/proxy/auth/auth_checks.py | 16 ++++++++++++++++ litellm/proxy/auth/user_api_key_auth.py | 7 ++----- .../complexity_router/cache_warming/refresher.py | 9 +++++++++ .../cache_warming/test_refresher.py | 15 +++++++++++++++ 6 files changed, 54 insertions(+), 13 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py index a27d6b92843..660e89e7f69 100644 --- a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py +++ b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py @@ -834,7 +834,11 @@ class MCPRequestHandler: missing user and a real outage look identical (the cause survives only as ``__context__``). ``_raise_503_if_db_unavailable`` walks the cause chain so an outage stays a retryable 503 while any other failure fails closed as 401, not an opaque 500; the object-permission load shares that boundary.""" - from litellm.proxy.auth.auth_checks import get_object_permission, get_user_object + from litellm.proxy.auth.auth_checks import ( + get_object_permission, + get_user_object, + user_is_scim_deactivated, + ) from litellm.proxy.proxy_server import prisma_client, user_api_key_cache if prisma_client is None: @@ -863,7 +867,7 @@ class MCPRequestHandler: raise HTTPException(status_code=401, detail="Invalid or expired credential") from None if user_object is None: raise HTTPException(status_code=401, detail="Invalid or expired credential") - if isinstance(user_object.metadata, dict) and user_object.metadata.get("scim_active") is False: + if user_is_scim_deactivated(user_object): raise HTTPException(status_code=401, detail="Invalid or expired credential") admitted = UserAPIKeyAuth( user_id=user_object.user_id, @@ -1004,7 +1008,7 @@ class MCPRequestHandler: keeping parity with how the standard pipeline treats the same lookup failure.""" if key_object.user_id is None: return - from litellm.proxy.auth.auth_checks import get_user_object + from litellm.proxy.auth.auth_checks import get_user_object, user_is_scim_deactivated from litellm.proxy.proxy_server import prisma_client, user_api_key_cache try: @@ -1017,9 +1021,7 @@ class MCPRequestHandler: except Exception as e: # noqa: BLE001 # mirror the builder's fail-open user lookup; DB errors are of any type verbose_logger.debug(f"bridge admission: user lookup failed, skipping SCIM gate: {e}") user_object = None - if user_object is None or not isinstance(user_object.metadata, dict): - return - if user_object.metadata.get("scim_active") is False: + if user_is_scim_deactivated(user_object): raise HTTPException(status_code=401, detail="Invalid or expired credential") @staticmethod diff --git a/litellm/proxy/_experimental/mcp_server/bridge_token_flow.py b/litellm/proxy/_experimental/mcp_server/bridge_token_flow.py index 19048e2eb7c..cb317329b77 100644 --- a/litellm/proxy/_experimental/mcp_server/bridge_token_flow.py +++ b/litellm/proxy/_experimental/mcp_server/bridge_token_flow.py @@ -197,6 +197,7 @@ async def _reload_active_user_by_id(user_id: str) -> "_KeyResolutionFailure | No ) from litellm.proxy.auth.auth_checks import ( # noqa: PLC0415 # inline import avoids a module-load circular import get_user_object, + user_is_scim_deactivated, ) from litellm.proxy.db.exception_handler import ( # noqa: PLC0415 # inline import avoids a module-load circular import PrismaDBExceptionHandler, @@ -224,7 +225,7 @@ async def _reload_active_user_by_id(user_id: str) -> "_KeyResolutionFailure | No return "no_active_key" if user_object is None: return "no_active_key" - if isinstance(user_object.metadata, dict) and user_object.metadata.get("scim_active") is False: + if user_is_scim_deactivated(user_object): return "no_active_key" return None @@ -239,6 +240,7 @@ async def _key_owner_scim_deactivated(key: "UserAPIKeyAuth") -> bool: return False from litellm.proxy.auth.auth_checks import ( # noqa: PLC0415 # inline import avoids a module-load circular import get_user_object, + user_is_scim_deactivated, ) from litellm.proxy.proxy_server import ( # noqa: PLC0415 # inline import avoids a module-load circular import prisma_client, @@ -257,7 +259,7 @@ async def _key_owner_scim_deactivated(key: "UserAPIKeyAuth") -> bool: except Exception as exc: # noqa: BLE001 # fail open: a missing owner (get_user_object's wrapped ValueError) or a DB blip must not revoke a live key verbose_logger.debug("refresh: key-owner SCIM lookup failed, not revoking (%s)", type(exc).__name__) return False - return owner is not None and isinstance(owner.metadata, dict) and owner.metadata.get("scim_active") is False + return user_is_scim_deactivated(owner) async def _revalidate_active_subject(identity: "EnvelopeIdentity") -> "_KeyResolutionFailure | None": diff --git a/litellm/proxy/auth/auth_checks.py b/litellm/proxy/auth/auth_checks.py index b971bb6e718..0b52701573c 100644 --- a/litellm/proxy/auth/auth_checks.py +++ b/litellm/proxy/auth/auth_checks.py @@ -1614,6 +1614,22 @@ async def _get_fuzzy_user_object( return response +def user_is_scim_deactivated(user_object: Optional[LiteLLM_UserTable]) -> bool: + """Whether a user has been explicitly deactivated via SCIM, so keys they own must not be usable. + + Lives beside get_user_object because the resolver returns a live-looking row for a deactivated user and + every caller owes this check afterwards; it was previously spelled out inline at five call sites, which is + how a sixth caller (background cache warming) ended up without it. Returns a value rather than raising so + each caller keeps its own reaction: the primary auth path raises, the MCP paths return 401, the bridge + returns a status. Only an explicit False deactivates -- a missing user or absent metadata is not a + deactivation, so a lookup failure cannot revoke a live key.""" + return ( + user_object is not None + and isinstance(user_object.metadata, dict) + and user_object.metadata.get("scim_active") is False + ) + + @log_db_metrics async def get_user_object( user_id: Optional[str], diff --git a/litellm/proxy/auth/user_api_key_auth.py b/litellm/proxy/auth/user_api_key_auth.py index b268d2f8840..5a6a374a46e 100644 --- a/litellm/proxy/auth/user_api_key_auth.py +++ b/litellm/proxy/auth/user_api_key_auth.py @@ -54,6 +54,7 @@ from litellm.proxy.auth.auth_checks import ( get_user_object, is_valid_fallback_model, resolve_and_validate_end_user_id, + user_is_scim_deactivated, ) from litellm.proxy.auth.auth_exception_handler import UserAPIKeyAuthExceptionHandler from litellm.proxy.auth.auth_utils import ( @@ -1745,11 +1746,7 @@ async def _user_api_key_auth_builder( ) user_obj = None - if ( - user_obj is not None - and isinstance(user_obj.metadata, dict) - and user_obj.metadata.get("scim_active") is False - ): + if user_is_scim_deactivated(user_obj): raise Exception( f"User={valid_token.user_id} has been deactivated via SCIM. Keys owned by this user cannot be used." ) diff --git a/litellm/router_strategy/complexity_router/cache_warming/refresher.py b/litellm/router_strategy/complexity_router/cache_warming/refresher.py index 1161e422a75..30295889288 100644 --- a/litellm/router_strategy/complexity_router/cache_warming/refresher.py +++ b/litellm/router_strategy/complexity_router/cache_warming/refresher.py @@ -733,11 +733,13 @@ class CacheWarmingRefresher: those gates unenforced while the cost callback still charged the same scopes. These stay cache-first, so they carry the same in-memory staleness a real request on another pod carries; only the key, which is the credential warming spends against, is read past the local tier.""" + from litellm.proxy._types import ProxyErrorTypes, ProxyException from litellm.proxy.auth.auth_checks import ( get_end_user_object, get_project_object, get_team_object, get_user_object, + user_is_scim_deactivated, ) from litellm.proxy.auth.user_api_key_auth import ( _reserve_budget_after_common_checks, # pyright: ignore[reportPrivateUsage] # the flow owner; reserve_budget_for_request alone drops the operator settings @@ -758,6 +760,13 @@ class CacheWarmingRefresher: if principal.user_id is not None else None ) + if user_is_scim_deactivated(user): + raise ProxyException( + message=f"User={principal.user_id} has been deactivated via SCIM. Keys owned by this user cannot be used.", + type=ProxyErrorTypes.auth_error, + param="user_id", + code=401, + ) end_user = ( await get_end_user_object( end_user_id=principal.end_user_id, prisma_client=prisma_client, user_api_key_cache=cache diff --git a/tests/test_litellm/router_strategy/complexity_router/cache_warming/test_refresher.py b/tests/test_litellm/router_strategy/complexity_router/cache_warming/test_refresher.py index 4fb1aa88524..b46271135fb 100644 --- a/tests/test_litellm/router_strategy/complexity_router/cache_warming/test_refresher.py +++ b/tests/test_litellm/router_strategy/complexity_router/cache_warming/test_refresher.py @@ -381,3 +381,18 @@ async def test_a_replay_never_falls_back_to_a_group_warming_did_not_validate(): await tick(llm_router) assert llm_router.completion_calls, "expected a replay" assert all(call["disable_fallbacks"] is True for call in llm_router.completion_calls) + + +def test_scim_deactivation_is_one_predicate_shared_with_the_auth_paths(): + """The gate sat inline at five call sites and warming became the sixth caller without it, so it lives + beside get_user_object now and every path calls the same predicate.""" + from litellm.proxy._types import LiteLLM_UserTable + from litellm.proxy.auth.auth_checks import user_is_scim_deactivated + + def user(metadata): + return LiteLLM_UserTable(user_id="u1", max_budget=None, spend=0.0, metadata=metadata) + + assert user_is_scim_deactivated(user({"scim_active": False})) is True + assert user_is_scim_deactivated(user({"scim_active": True})) is False + assert user_is_scim_deactivated(user({})) is False + assert user_is_scim_deactivated(None) is False