Conversation
…erns Brings organization info and team management endpoints in line with the access-control patterns used elsewhere in the proxy.
…checks [Fix] Align Org and Team Endpoint Permission Checks
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR introduces two access-control helpers — Confidence Score: 5/5Safe to merge; all findings are P2 style/improvement suggestions with no blocking correctness or security issues. The access-control additions are correct and improve security on previously unguarded endpoints. Tests cover the denial paths for the team helper. The only open concern is a P2 N+1 style violation in the deprecated org-info endpoint loop — mitigated by caching in practice. organization_endpoints.py — the
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/organization_endpoints.py | Adds _verify_org_access helper to gate /organization/info and the deprecated POST /organization/info behind proxy-admin or org-admin checks; minor N+1 concern in deprecated endpoint's per-org loop. |
| litellm/proxy/management_endpoints/team_endpoints.py | Adds _verify_team_access helper enforcing proxy-admin/org-admin/team-admin access on update_team and delete_team; correctly delegates org-admin check to the existing _is_user_org_admin_for_team helper. |
| tests/test_litellm/proxy/management_endpoints/test_team_endpoints.py | Adds test_verify_team_access_denies_unauthorized_user and test_update_team_rejects_unauthorized_caller covering the 403 denial paths; other changes are Black-style reformats with no semantic delta. |
Reviews (2): Last reviewed commit: "[Fix] Address Greptile review: POST /org..." | Re-trigger Greptile
| from litellm.proxy.auth.auth_checks import get_user_object | ||
| from litellm.proxy.proxy_server import proxy_logging_obj, user_api_key_cache |
There was a problem hiding this comment.
Inline imports inside helper function
_verify_org_access imports get_user_object, proxy_logging_obj, and user_api_key_cache inside the function body. CLAUDE.md requires module-level imports except where circular imports are unavoidable. The identical pattern already exists in common_utils._is_user_org_admin_for_team (lines 57–62), so the circular-import justification applies here too — but worth aligning by moving them to the module level with a TYPE_CHECKING guard or by grouping them with the existing inline import in info_organization at line 775 where prisma_client is imported inline.
Context Used: CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| async def _verify_team_access( | ||
| team_obj: LiteLLM_TeamTable, | ||
| user_api_key_dict: UserAPIKeyAuth, | ||
| ) -> None: | ||
| """ | ||
| Verify the caller is authorized to manage the given team. | ||
|
|
||
| Access is granted if: | ||
| - Caller is a proxy admin, OR | ||
| - Caller is an org admin for the team's organization, OR | ||
| - Caller is a team admin of this team | ||
|
|
||
| Raises HTTPException(403) otherwise. | ||
| """ | ||
| if user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN: | ||
| return | ||
|
|
||
| if _is_user_team_admin(user_api_key_dict=user_api_key_dict, team_obj=team_obj): | ||
| return | ||
|
|
||
| if await _is_user_org_admin_for_team( | ||
| user_api_key_dict=user_api_key_dict, team_obj=team_obj | ||
| ): | ||
| return | ||
|
|
||
| raise HTTPException( | ||
| status_code=status.HTTP_403_FORBIDDEN, | ||
| detail="You do not have access to this team", | ||
| ) |
There was a problem hiding this comment.
No test covering the 403 denial path of
_verify_team_access
_verify_team_access is now called by both update_team and delete_team, but no test verifies that an unauthorized caller (not proxy admin, not team admin, not org admin) receives a 403. All existing update_team non-admin tests include the caller as a team admin in members_with_roles, so they pass the check incidentally. CLAUDE.md requires at least one test for new features; a test with an empty members_with_roles and a non-admin caller would cover the denial path for both update_team and delete_team.
Context Used: CLAUDE.md (source)
…nline imports, team access denial tests - Add _verify_org_access to deprecated POST /organization/info endpoint - Move get_user_object to module-level import in organization_endpoints.py - Add tests for _verify_team_access 403 denial path
| from litellm._uuid import uuid | ||
| from litellm.proxy._types import * | ||
| from litellm.proxy.auth.auth_checks import can_user_call_model | ||
| from litellm.proxy.auth.auth_checks import can_user_call_model, get_user_object |
| from litellm._uuid import uuid | ||
| from litellm.proxy._types import * | ||
| from litellm.proxy.auth.auth_checks import can_user_call_model | ||
| from litellm.proxy.auth.auth_checks import can_user_call_model, get_user_object |
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Type
🚄 Infrastructure
Changes