fix(team_endpoints): auto-add SSO team members to org on move (proxy admin only)#26377
fix(team_endpoints): auto-add SSO team members to org on move (proxy admin only)#26377ishaan-berri wants to merge 5 commits intolitellm_internal_stagingfrom
Conversation
… commit Co-Authored-By: Ishaan Jaff <ishaan@berri.ai>
|
|
Greptile SummaryThis PR fixes a 403 error affecting proxy admins in SSO/Entra setups where org membership tables are empty due to no SCIM sync. It adds an Confidence Score: 5/5Safe to merge — the fix is well-scoped, the security model is preserved, and all remaining findings are minor style suggestions. No P0 or P1 findings. The proxy-admin bypass is correctly gated behind an existing auth check in No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/management_endpoints/team_endpoints.py | Adds _auto_add_team_members_to_organization helper and threads is_proxy_admin through validate_team_org_change and fetch_and_validate_organization; logic is sound, minor style issues (redundant organization_id guard inside loop, duplicate LiteLLM_TeamTable construction). |
| tests/test_litellm/proxy/test_team_org_move.py | New test file covering all key scenarios (proxy-admin bypass, team-admin block/pass, same-org short-circuit, default-user exclusion, error resilience); tests use manual monkey-patching instead of unittest.mock.patch, which is functional but less idiomatic. |
Sequence Diagram
sequenceDiagram
participant Client
participant update_team
participant fetch_and_validate_organization
participant validate_team_org_change
participant _auto_add_team_members_to_organization
participant add_member_to_organization
participant DB
Client->>update_team: PATCH /team/update (organization_id)
update_team->>update_team: check caller is PROXY_ADMIN or dest-org admin
update_team->>fetch_and_validate_organization: organization_id, existing_team_row, user_api_key_dict
fetch_and_validate_organization->>DB: find org + members
fetch_and_validate_organization->>fetch_and_validate_organization: derive is_proxy_admin
fetch_and_validate_organization->>validate_team_org_change: team, org, is_proxy_admin
alt is_proxy_admin = true
validate_team_org_change-->>fetch_and_validate_organization: skip membership check
fetch_and_validate_organization->>_auto_add_team_members_to_organization: team, org, prisma_client
loop each team member not in org
_auto_add_team_members_to_organization->>add_member_to_organization: user_id, INTERNAL_USER
add_member_to_organization->>DB: upsert org membership
DB-->>add_member_to_organization: ok or dup-key caught+debug-logged
end
else is_proxy_admin = false
validate_team_org_change->>validate_team_org_change: all members must be in org
alt missing members
validate_team_org_change-->>Client: 403 HTTPException
end
end
fetch_and_validate_organization-->>update_team: org_row
update_team->>DB: update team.organization_id
update_team-->>Client: 200 updated team
Reviews (2): Last reviewed commit: "style: black formatting for team_endpoin..." | Re-trigger Greptile
| except Exception as e: | ||
| verbose_proxy_logger.debug( | ||
| "_auto_add_team_members_to_organization: skipping user_id=%s - %s", | ||
| member.user_id, | ||
| e, | ||
| ) |
There was a problem hiding this comment.
Overly broad exception silencing
All exceptions from add_member_to_organization are re-raised as ValueError there, and every one of them is caught here and logged only at DEBUG. This means real failures (DB connection loss, unexpected constraint violations, network timeouts) are completely invisible in production logs — the team move silently succeeds while some members are never actually added to the org, leaving a data inconsistency with no operator-visible signal.
Consider logging unexpected exceptions at WARNING or ERROR and reserving DEBUG only for known-benign errors like unique-constraint violations (e.g. check "duplicate" in str(e).lower() before deciding the log level).
Low: Authorization check relaxation for proxy admins onlyThis PR allows proxy admins to bypass the team-member org-membership check when moving teams between organizations, auto-adding missing members as Status: 0 open Posted by Veria AI · 2026-04-24T01:44:17.701Z |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Relevant issues
Fixes SSO/Entra setups where proxy admins get a 403 when moving teams to organizations because the org membership table is always empty (no SCIM sync).
Fixes LIT-2447
Pre-Submission checklist
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 reviewType
🐛 Bug Fix
Changes
Problem: When a proxy admin calls
PATCH /team/updateto assignorganization_id,validate_team_org_changechecks that every team member is already an org member and returns 403 if not. In SSO/Entra setups without SCIM, users are auto-created as team members on login but the org membership table stays empty — so this always 403s.Fix:
validate_team_org_changegets anis_proxy_adminflag. Proxy admins bypass the membership check; team admins keep the original strict check (prevents privilege escalation — a team admin can't inject users into an org they don't control)._auto_add_team_members_to_organizationhelper: when a proxy admin moves a team, any team members not already in the org are silently added asinternal_user. Errors (duplicate key, race condition) are logged at DEBUG and skipped.fetch_and_validate_organizationthreadsuser_api_key_dictthrough so it can deriveis_proxy_adminand call the helper.Security model:
internal_userTests:
tests/test_litellm/proxy/test_team_org_move.py— 9 unit tests covering proxy-admin bypass, team-admin block/pass, same-org short-circuit, default-user exclusion, and error resilience.Screenshots / Proof of Fix
Before — proxy admin gets 403 moving a team with SSO users to an org:
After — team moves successfully, org field populated: