Skip to content

test(proxy): separate the member_add permission gate from the provisioning gate - #35507

Merged
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_/team-member-add-authz-tests-3948e1
Aug 1, 2026
Merged

test(proxy): separate the member_add permission gate from the provisioning gate#35507
yuneng-berri merged 1 commit into
litellm_internal_stagingfrom
litellm_/team-member-add-authz-tests-3948e1

Conversation

@yuneng-berri

Copy link
Copy Markdown
Collaborator

TLDR

Problem this solves:

  • member_add authz matrix broke on the new provisioning rule
  • Relaxing its expectations would have made all 18 rows vacuous
  • The new rule had no HTTP-boundary coverage, only helper-level

How it solves it:

  • Seed the member as a real user so the matrix reads the permission gate
  • Add a matrix for the provisioning gate, plus its email escape hatch
  • Share the user seeder the member-info pins kept private

Relevant issues

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Delays in PR merge?

If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).

Screenshots / Proof of Fix

Captured at ff4a50c768 against a local proxy on port 4001, no mocks. Setup is a team admin (an internal_user holding the admin role on their own team) plus the team itself:

curl -sS -X POST $P/user/new -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
  -d '{"user_id": "memberadd-1785620648-teamadmin", "user_role": "internal_user"}'
curl -sS -X POST $P/key/generate -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
  -d '{"user_id": "memberadd-1785620648-teamadmin"}'
curl -sS -X POST $P/team/new -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
  -d '{"team_alias": "memberadd-1785620648-team", "members_with_roles": [{"user_id": "memberadd-1785620648-teamadmin", "role": "admin"}]}'

A. The team admin cannot conjure a user_id that has no user row, and nothing is created behind the refusal. This is the leg the tests had no boundary coverage for.

curl -sS -w '\nHTTP %{http_code}\n' -X POST $P/team/member_add \
  -H "Authorization: Bearer $TA_KEY" -H 'Content-Type: application/json' \
  -d '{"team_id": "ed7d2558-3999-475f-92cd-ff671537efec", "member": {"user_id": "memberadd-1785620648-ghost", "role": "user"}}'
{"detail":{"error":"Only proxy admins can add a user_id that does not exist yet: memberadd-1785620648-ghost. Add the member by user_email to invite a new user, or ask a proxy admin to create the user first."}}
HTTP 403
curl -sS -w '\nHTTP %{http_code}\n' -X GET "$P/user/info?user_id=memberadd-1785620648-ghost" \
  -H "Authorization: Bearer sk-1234"
{"error":{"message":"User memberadd-1785620648-ghost not found","type":"internal_server_error","param":"None","code":"404"}}
HTTP 404

B. The escape hatch that refusal names actually works for the same caller. Inviting by user_email allocates the user_id server-side, so a team admin keeps it:

curl -sS -w '\nHTTP %{http_code}\n' -X POST $P/team/member_add \
  -H "Authorization: Bearer $TA_KEY" -H 'Content-Type: application/json' \
  -d '{"team_id": "ed7d2558-3999-475f-92cd-ff671537efec", "member": {"user_email": "memberadd-1785620648-invitee@example.com", "role": "user"}}'
{
  "team_id": "ed7d2558-3999-475f-92cd-ff671537efec",
  "members_with_roles": [
    {"user_id": "memberadd-1785620648-teamadmin", "user_email": null, "role": "admin"},
    {"user_id": "default_user_id", "user_email": null, "role": "admin"},
    {"user_id": "525c08f9-19e7-422d-9fde-648ee65f23d3", "user_email": "memberadd-1785620648-invitee@example.com", "role": "user"}
  ],
  "updated_users": [
    {"user_id": "525c08f9-19e7-422d-9fde-648ee65f23d3", "user_email": "memberadd-1785620648-invitee@example.com"}
  ]
}
HTTP 200

C. A proxy admin adding that same unprovisioned user_id is still allowed, and does provision the row. The gate is a role split, not a blanket block:

curl -sS -o /dev/null -w 'HTTP %{http_code}\n' -X POST $P/team/member_add \
  -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
  -d '{"team_id": "ed7d2558-3999-475f-92cd-ff671537efec", "member": {"user_id": "memberadd-1785620648-ghost", "role": "user"}}'
curl -sS -X GET "$P/user/info?user_id=memberadd-1785620648-ghost" -H "Authorization: Bearer sk-1234"
HTTP 200
{"user_id": "memberadd-1785620648-ghost", "teams": ["ed7d2558-3999-475f-92cd-ff671537efec"]}

D. The team admin can still add a member that already exists. This is the behavior the matrix rows in question were written to pin, and the reason relaxing them to 403 would have been wrong:

curl -sS -X POST $P/user/new -H "Authorization: Bearer sk-1234" -H 'Content-Type: application/json' \
  -d '{"user_id": "memberadd-1785620648-existing", "user_role": "internal_user"}'
curl -sS -w '\nHTTP %{http_code}\n' -X POST $P/team/member_add \
  -H "Authorization: Bearer $TA_KEY" -H 'Content-Type: application/json' \
  -d '{"team_id": "ed7d2558-3999-475f-92cd-ff671537efec", "member": {"user_id": "memberadd-1785620648-existing", "role": "user"}}'
{"members_with_roles": ["memberadd-1785620648-teamadmin", "default_user_id", "525c08f9-19e7-422d-9fde-648ee65f23d3", "memberadd-1785620648-ghost", "memberadd-1785620648-existing"]}
HTTP 200

Type

✅ Test

Changes

Adding a team member by a user_id with no user row is now proxy-admin-only. The /team/member_add authz matrix targeted a never-seeded user_id, so it started refusing every non-proxy-admin caller and three rows went red. That matrix exists to pin _validate_team_member_add_permissions, which the provisioning rule did not touch, so the member is now seeded as a real user row and the matrix goes back to its original expectations.

Relaxing those three rows to 403 instead would have been the wrong repair. Every non-proxy-admin row in the matrix would then be explainable by the provisioning gate alone; I built that variant and ran it against a tree with _validate_team_member_add_permissions deleted outright, and all 18 rows stayed green. With the member seeded, the same deletion fails 13 of them.

The provisioning rule itself was pinned only by unit tests calling the helper directly, so nothing proved it was wired into the route. It gets its own matrix, over three actors who all clear the permission check on the same team, so the provisioning gate is the only variable between them; the denial legs also assert no user row is left behind, which is the actual claim. One more test covers the user_email invite the 403 message points callers at, so closing that path for non-proxy-admins cannot pass silently.

The user seeder that test_team_member_info_validation.py kept private moves to conftest.py alongside the other create_scratch_* seeders. The scratch teardown now also reclaims users by scratch-prefixed email, since an invite allocates the user_id server-side and the id-prefix sweep cannot see it.

Verified by mutation rather than by the suite passing. Deleting the provisioning gate fails the two new denial rows; deleting the permission gate fails 13 matrix rows; widening the provisioning gate to also reject email-only members fails the escape-hatch test. tests/proxy_behavior is 733 passed, up from 729.

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

…oning gate

Adding a team member by a user_id with no user row is now proxy-admin-only,
so the /team/member_add authz matrix, which targeted a never-seeded user_id,
started 403ing every non-proxy-admin caller. Seed the member as a real user
row so the matrix reads _validate_team_member_add_permissions alone; leaving
it unseeded and relaxing the expectations to 403 would have left all 18 rows
green with that gate deleted outright.

Cover the new gate at the HTTP boundary, where only the helper was pinned
before: a team admin and an org admin both clear the permission check on the
same team and are still refused an unprovisioned user_id, with no user row
left behind. Pin the escape hatch that refusal names too, so closing the
email-invite path for non-proxy-admins cannot pass silently.

Promote the user seeder the member-info pins had kept private to conftest,
and reclaim invited users by their scratch-prefixed email, since an invite
allocates the user_id server-side.
@greptile-apps

greptile-apps Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This test-only PR separates coverage of team-member permission checks from coverage of the user-provisioning restriction.

  • Moves the scratch-user seeder into the shared management-test fixture module.
  • Seeds an existing user in the authorization matrix so provisioning behavior cannot mask permission regressions.
  • Adds HTTP-boundary tests for unprovisioned user IDs and email invitations.
  • Extends scratch cleanup to reclaim users whose IDs were allocated server-side.

Confidence Score: 5/5

The PR appears safe to merge; the test refactor preserves authorization coverage while adding focused provisioning coverage and complete fixture cleanup.

Each parameterized case uses isolated scratch identifiers, the extracted helper preserves the previous seeding behavior, and the new tests verify both HTTP responses and persisted database state.

Important Files Changed

Filename Overview
tests/proxy_behavior/management/conftest.py Adds a reusable scratch-user seeder and safely expands teardown to reclaim email-invited users with server-generated IDs.
tests/proxy_behavior/management/test_team_member_add.py Isolates the existing authorization matrix from provisioning behavior and adds focused boundary tests for both provisioning denial and email invitations.
tests/proxy_behavior/management/test_team_member_info_validation.py Replaces the private user-seeding helper with the equivalent shared fixture helper without changing test behavior.

Reviews (1): Last reviewed commit: "test(proxy): separate the member_add per..." | Re-trigger Greptile

@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@yuneng-berri
yuneng-berri merged commit a9e1ab2 into litellm_internal_staging Aug 1, 2026
76 checks passed
@yuneng-berri
yuneng-berri deleted the litellm_/team-member-add-authz-tests-3948e1 branch August 1, 2026 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants