mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-06 08:16:43 +00:00
fix: allow non-admin access to /guardrails/submissions and fix register membership check
Bug 1: internal users hit route-level 403 on /guardrails/submissions.
The route wasn't in self_managed_routes, so the route allowlist rejected
non-admin callers before our endpoint's team-scoping ran. Added
/guardrails/submissions and /guardrails/submissions/{guardrail_id} to
self_managed_routes.
Bug 2: register_guardrail 403'd non-admins registering for teams in
their user.teams list. It used get_team_membership() which reads the
litellm_teammembership join table, but that row is only created when
the team has a budget (management_helpers/utils.py:225). Switched to
the _get_user_team_ids helper (reads user_obj.teams), making it
consistent with list_guardrail_submissions.
UI: moved the Test Playground tab inside the isAdmin conditional in
guardrails.tsx. Internal users now see only the Submitted Guardrails
tab; admins still see all four.
Tests: added coverage for non-admin register paths (cross-team allowed
and cross-team forbidden).
This commit is contained in:
parent
ab9c875a00
commit
79064a68e4
4 changed files with 71 additions and 23 deletions
|
|
@ -665,6 +665,9 @@ class LiteLLMRoutes(enum.Enum):
|
|||
"/invitation/delete",
|
||||
# Team guardrail submission - requires team-scoped key; endpoint enforces team_id
|
||||
"/guardrails/register",
|
||||
# Team guardrail submissions - endpoint scopes results to caller's teams (non-admin)
|
||||
"/guardrails/submissions",
|
||||
"/guardrails/submissions/{guardrail_id}",
|
||||
] # routes that manage their own allowed/disallowed logic
|
||||
|
||||
## Org Admin Routes ##
|
||||
|
|
|
|||
|
|
@ -615,16 +615,8 @@ async def register_guardrail(
|
|||
# Validate team membership for non-admin users when team differs from key
|
||||
is_admin = user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN
|
||||
if not is_admin and team_id != user_api_key_dict.team_id:
|
||||
from litellm.proxy.auth.auth_checks import get_team_membership
|
||||
from litellm.proxy.proxy_server import user_api_key_cache
|
||||
|
||||
membership = await get_team_membership(
|
||||
user_id=user_api_key_dict.user_id or "",
|
||||
team_id=team_id,
|
||||
prisma_client=prisma_client,
|
||||
user_api_key_cache=user_api_key_cache,
|
||||
)
|
||||
if membership is None:
|
||||
user_team_ids = await _get_user_team_ids(user_api_key_dict)
|
||||
if team_id not in user_team_ids:
|
||||
raise HTTPException(
|
||||
status_code=403,
|
||||
detail=f"You are not a member of team {team_id!r}",
|
||||
|
|
|
|||
|
|
@ -1220,6 +1220,59 @@ async def test_register_guardrail_requires_team_id(mocker):
|
|||
assert "team" in exc_info.value.detail.lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_guardrail_non_admin_cross_team_allowed(mocker):
|
||||
"""Non-admin may register for a team in their user.teams list even if the key's team_id differs."""
|
||||
mock_prisma = mocker.Mock()
|
||||
mock_prisma.db.litellm_guardrailstable.find_unique = AsyncMock(return_value=None)
|
||||
created = mocker.Mock(
|
||||
guardrail_id="g1",
|
||||
guardrail_name=MOCK_REGISTER_REQUEST.guardrail_name,
|
||||
status="pending_review",
|
||||
submitted_at=datetime.now(),
|
||||
)
|
||||
mock_prisma.db.litellm_guardrailstable.create = AsyncMock(return_value=created)
|
||||
mocker.patch("litellm.proxy.proxy_server.prisma_client", mock_prisma)
|
||||
mocker.patch(
|
||||
"litellm.proxy.guardrails.guardrail_endpoints._get_user_team_ids",
|
||||
AsyncMock(return_value=["team-alpha", "team-beta"]),
|
||||
)
|
||||
req = RegisterGuardrailRequest(
|
||||
guardrail_name=MOCK_REGISTER_REQUEST.guardrail_name,
|
||||
team_id="team-beta",
|
||||
litellm_params=MOCK_REGISTER_REQUEST.litellm_params,
|
||||
)
|
||||
user = UserAPIKeyAuth(
|
||||
user_id="u1", user_role=LitellmUserRoles.INTERNAL_USER, team_id="team-alpha"
|
||||
)
|
||||
|
||||
result = await register_guardrail(req, user)
|
||||
|
||||
assert result.guardrail_id == "g1"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_guardrail_non_admin_cross_team_forbidden(mocker):
|
||||
"""Non-admin gets 403 when registering for a team they are not a member of."""
|
||||
mocker.patch("litellm.proxy.proxy_server.prisma_client", mocker.Mock())
|
||||
mocker.patch(
|
||||
"litellm.proxy.guardrails.guardrail_endpoints._get_user_team_ids",
|
||||
AsyncMock(return_value=["team-alpha"]),
|
||||
)
|
||||
req = RegisterGuardrailRequest(
|
||||
guardrail_name=MOCK_REGISTER_REQUEST.guardrail_name,
|
||||
team_id="team-other",
|
||||
litellm_params=MOCK_REGISTER_REQUEST.litellm_params,
|
||||
)
|
||||
user = UserAPIKeyAuth(
|
||||
user_id="u1", user_role=LitellmUserRoles.INTERNAL_USER, team_id="team-alpha"
|
||||
)
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await register_guardrail(req, user)
|
||||
assert exc_info.value.status_code == 403
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_register_guardrail_duplicate_name(mocker):
|
||||
"""Register returns 400 when guardrail_name already exists."""
|
||||
|
|
|
|||
|
|
@ -234,21 +234,21 @@ const GuardrailsPanel: React.FC<GuardrailsPanelProps> = ({ accessToken, userRole
|
|||
</>
|
||||
),
|
||||
},
|
||||
{
|
||||
key: "playground",
|
||||
label: "Test Playground",
|
||||
disabled: !accessToken,
|
||||
children: (
|
||||
<GuardrailTestPlayground
|
||||
guardrailsList={guardrailsList}
|
||||
isLoading={isLoading}
|
||||
accessToken={accessToken}
|
||||
onClose={() => {}}
|
||||
/>
|
||||
),
|
||||
},
|
||||
]
|
||||
: []),
|
||||
{
|
||||
key: "playground",
|
||||
label: "Test Playground",
|
||||
disabled: !accessToken,
|
||||
children: (
|
||||
<GuardrailTestPlayground
|
||||
guardrailsList={guardrailsList}
|
||||
isLoading={isLoading}
|
||||
accessToken={accessToken}
|
||||
onClose={() => {}}
|
||||
/>
|
||||
),
|
||||
},
|
||||
{
|
||||
key: "submitted",
|
||||
label: "Submitted Guardrails",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue