mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
Merge pull request #14161 from BerriAI/litellm_dev_09_01_2025_p2
Security fix - prevent proxy_admin_viewer from modifying other user's credentials + remove hardcoded sensitive keys from test repo
This commit is contained in:
Krish Dholakia
GitHub
commit
9a62b9bdb9
6 changed files with 108 additions and 21 deletions
|
|
@ -4,6 +4,9 @@ model_list:
|
|||
model: openai/fake
|
||||
api_key: fake-key
|
||||
api_base: https://exampleopenaiendpoint-production.up.railway.app/
|
||||
- model_name: wildcard_models/*
|
||||
litellm_params:
|
||||
model: openai/*
|
||||
- model_name: gpt-5-mini
|
||||
litellm_params:
|
||||
model: azure/gpt-5-mini
|
||||
|
|
@ -24,4 +27,4 @@ router_settings:
|
|||
|
||||
litellm_settings:
|
||||
callbacks: ["otel"]
|
||||
success_callback: ["braintrust"]
|
||||
success_callback: ["braintrust"]
|
||||
|
|
@ -300,7 +300,7 @@ class RouteChecks:
|
|||
if re.match(pattern, route):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
@staticmethod
|
||||
def _is_wildcard_pattern(pattern: str) -> bool:
|
||||
"""
|
||||
|
|
@ -354,16 +354,22 @@ class RouteChecks:
|
|||
#########################################################
|
||||
if route in allowed_routes:
|
||||
return True
|
||||
|
||||
|
||||
#########################################################
|
||||
# wildcard match route is in allowed_routes
|
||||
# e.g calling /anthropic/v1/messages is allowed if allowed_routes has /anthropic/*
|
||||
#########################################################
|
||||
wildcard_allowed_routes = [route for route in allowed_routes if RouteChecks._is_wildcard_pattern(pattern=route)]
|
||||
wildcard_allowed_routes = [
|
||||
route
|
||||
for route in allowed_routes
|
||||
if RouteChecks._is_wildcard_pattern(pattern=route)
|
||||
]
|
||||
for allowed_route in wildcard_allowed_routes:
|
||||
if RouteChecks._route_matches_wildcard_pattern(route=route, pattern=allowed_route):
|
||||
if RouteChecks._route_matches_wildcard_pattern(
|
||||
route=route, pattern=allowed_route
|
||||
):
|
||||
return True
|
||||
|
||||
|
||||
#########################################################
|
||||
# pattern match route is in allowed_routes
|
||||
# pattern: "/threads/{thread_id}"
|
||||
|
|
@ -375,7 +381,7 @@ class RouteChecks:
|
|||
for allowed_route in allowed_routes
|
||||
):
|
||||
return True
|
||||
|
||||
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
|
|
@ -420,7 +426,7 @@ class RouteChecks:
|
|||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail=f"user not allowed to access this OpenAI routes, role= {_user_role}",
|
||||
)
|
||||
|
||||
|
||||
# Check if this is a write operation on management routes
|
||||
if RouteChecks.check_route_access(
|
||||
route=route, allowed_routes=LiteLLMRoutes.management_routes.value
|
||||
|
|
@ -436,7 +442,28 @@ class RouteChecks:
|
|||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail=f"user not allowed to access this route, role= {_user_role}. Trying to access: {route} and updating invalid param: {param}. only user_email and password can be updated",
|
||||
)
|
||||
elif route in ["/user/new", "/user/delete", "/team/new", "/team/update", "/team/delete", "/model/new", "/model/update", "/model/delete", "/key/generate", "/key/delete", "/key/update", "/key/regenerate", "/key/service-account/generate", "/key/block", "/key/unblock"] or route.startswith("/key/") and route.endswith("/regenerate"):
|
||||
elif (
|
||||
route
|
||||
in [
|
||||
"/user/new",
|
||||
"/user/delete",
|
||||
"/team/new",
|
||||
"/team/update",
|
||||
"/team/delete",
|
||||
"/model/new",
|
||||
"/model/update",
|
||||
"/model/delete",
|
||||
"/key/generate",
|
||||
"/key/delete",
|
||||
"/key/update",
|
||||
"/key/regenerate",
|
||||
"/key/service-account/generate",
|
||||
"/key/block",
|
||||
"/key/unblock",
|
||||
]
|
||||
or route.startswith("/key/")
|
||||
and route.endswith("/regenerate")
|
||||
):
|
||||
# Block write operations for PROXY_ADMIN_VIEW_ONLY
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
|
|
|
|||
|
|
@ -835,6 +835,16 @@ async def _update_single_user_helper(
|
|||
existing_user_row = LiteLLM_UserTable(
|
||||
**existing_user_row.model_dump(exclude_none=True)
|
||||
)
|
||||
if not can_user_call_user_update(
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
user_info=existing_user_row,
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail={
|
||||
"error": "User does not have permission to update this user. Only PROXY_ADMIN can update other users."
|
||||
},
|
||||
)
|
||||
|
||||
existing_metadata = (
|
||||
cast(Dict, getattr(existing_user_row, "metadata", {}) or {})
|
||||
|
|
@ -929,6 +939,20 @@ async def _update_single_user_helper(
|
|||
return response
|
||||
|
||||
|
||||
def can_user_call_user_update(
|
||||
user_api_key_dict: UserAPIKeyAuth,
|
||||
user_info: LiteLLM_UserTable,
|
||||
) -> bool:
|
||||
"""
|
||||
Helper to check if the user has access to the key's info
|
||||
"""
|
||||
if user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value:
|
||||
return True
|
||||
elif user_api_key_dict.user_id == user_info.user_id:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
@router.post(
|
||||
"/user/update",
|
||||
tags=["Internal User management"],
|
||||
|
|
@ -988,6 +1012,7 @@ async def user_update(
|
|||
"""
|
||||
try:
|
||||
verbose_proxy_logger.debug("/user/update: Received data = %s", data)
|
||||
|
||||
response = await _update_single_user_helper(
|
||||
user_request=data,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
|
|
|
|||
|
|
@ -171,14 +171,17 @@ def test_azure_extra_headers(input, call_type, header_value):
|
|||
"api_base, model, expected_endpoint",
|
||||
[
|
||||
(
|
||||
"https://my-endpoint-sweden-berri992.openai.azure.com",
|
||||
os.getenv("AZURE_SWEDEN_API_BASE"),
|
||||
"dall-e-3-test",
|
||||
"https://my-endpoint-sweden-berri992.openai.azure.com/openai/deployments/dall-e-3-test/images/generations?api-version=2023-12-01-preview",
|
||||
os.getenv("AZURE_SWEDEN_API_BASE")
|
||||
+ "/openai/deployments/dall-e-3-test/images/generations?api-version=2023-12-01-preview",
|
||||
),
|
||||
(
|
||||
"https://my-endpoint-sweden-berri992.openai.azure.com/openai/deployments/my-custom-deployment",
|
||||
os.getenv("AZURE_SWEDEN_API_BASE")
|
||||
+ "/openai/deployments/my-custom-deployment",
|
||||
"dall-e-3",
|
||||
"https://my-endpoint-sweden-berri992.openai.azure.com/openai/deployments/my-custom-deployment/images/generations?api-version=2023-12-01-preview",
|
||||
os.getenv("AZURE_SWEDEN_API_BASE")
|
||||
+ "/openai/deployments/my-custom-deployment/images/generations?api-version=2023-12-01-preview",
|
||||
),
|
||||
],
|
||||
)
|
||||
|
|
@ -258,7 +261,7 @@ def test_azure_openai_gpt_4o_naming(monkeypatch):
|
|||
|
||||
client = AzureOpenAI(
|
||||
api_key="test-api-key",
|
||||
base_url="https://my-endpoint-sweden-berri992.openai.azure.com",
|
||||
base_url=os.getenv("AZURE_SWEDEN_API_BASE"),
|
||||
api_version="2023-12-01-preview",
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -21,7 +21,6 @@ def test_aaaasschema_migration_check(schema_setup, monkeypatch):
|
|||
"""Test to check if schema requires migration"""
|
||||
# Set test database URL
|
||||
test_db_url = f"postgresql://{schema_setup.info.user}:@{schema_setup.info.host}:{schema_setup.info.port}/{schema_setup.info.dbname}"
|
||||
# test_db_url = "postgresql://neondb_owner:npg_JiZPS0DAhRn4@ep-delicate-wave-a55cvbuc.us-east-2.aws.neon.tech/neondb?sslmode=require"
|
||||
monkeypatch.setenv("DATABASE_URL", test_db_url)
|
||||
|
||||
deploy_dir = Path("./litellm-proxy-extras/litellm_proxy_extras")
|
||||
|
|
|
|||
|
|
@ -2642,7 +2642,12 @@ async def test_reset_spend_authentication(prisma_client):
|
|||
_response = await new_user(
|
||||
data=NewUserRequest(
|
||||
tpm_limit=20,
|
||||
)
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
api_key="sk-1234",
|
||||
user_id="admin_user_id",
|
||||
),
|
||||
)
|
||||
|
||||
generate_key = "Bearer " + _response.key
|
||||
|
|
@ -2662,7 +2667,12 @@ async def test_reset_spend_authentication(prisma_client):
|
|||
data=NewUserRequest(
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
tpm_limit=20,
|
||||
)
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
api_key="sk-1234",
|
||||
user_id="admin_user_id",
|
||||
),
|
||||
)
|
||||
|
||||
generate_key = "Bearer " + _response.key
|
||||
|
|
@ -2815,7 +2825,12 @@ async def test_update_user_role(prisma_client):
|
|||
key = await new_user(
|
||||
data=NewUserRequest(
|
||||
user_role=LitellmUserRoles.INTERNAL_USER,
|
||||
)
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
api_key="sk-1234",
|
||||
user_id="admin_user_id",
|
||||
),
|
||||
)
|
||||
|
||||
print(key)
|
||||
|
|
@ -2844,7 +2859,12 @@ async def test_update_user_role(prisma_client):
|
|||
await user_update(
|
||||
data=UpdateUserRequest(
|
||||
user_id=key.user_id, user_role=LitellmUserRoles.PROXY_ADMIN
|
||||
)
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
api_key="sk-1234",
|
||||
user_id="admin_user_id",
|
||||
),
|
||||
)
|
||||
|
||||
# await asyncio.sleep(3)
|
||||
|
|
@ -2868,7 +2888,12 @@ async def test_update_user_unit_test(prisma_client):
|
|||
key = await new_user(
|
||||
data=NewUserRequest(
|
||||
user_email=f"test-{uuid.uuid4()}@test.com",
|
||||
)
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
api_key="sk-1234",
|
||||
user_id="admin_user_id",
|
||||
),
|
||||
)
|
||||
|
||||
print(key)
|
||||
|
|
@ -2882,7 +2907,12 @@ async def test_update_user_unit_test(prisma_client):
|
|||
tpm_limit=100,
|
||||
rpm_limit=100,
|
||||
metadata={"very-new-metadata": "something"},
|
||||
)
|
||||
),
|
||||
user_api_key_dict=UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
api_key="sk-1234",
|
||||
user_id="admin_user_id",
|
||||
),
|
||||
)
|
||||
|
||||
print("user_info", user_info)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue