mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
test(proxy): separate the member_add permission gate from the provisioning 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.
This commit is contained in:
parent
b1fd20f4cd
commit
ff4a50c768
3 changed files with 156 additions and 28 deletions
|
|
@ -194,6 +194,33 @@ async def create_scratch_team(
|
|||
return team_id
|
||||
|
||||
|
||||
async def create_scratch_user(
|
||||
prisma,
|
||||
scratch_prefix: str,
|
||||
*,
|
||||
suffix: str,
|
||||
user_email: Optional[str] = None,
|
||||
) -> str:
|
||||
"""Raw-seed a scratch-tagged user row with no key; returns its user_id.
|
||||
|
||||
The passive counterpart to create_scratch_actor: a user that requests are
|
||||
made *about* rather than *by*, so it needs no verification token. The
|
||||
user_id matches Scratch.tag(suffix), so the teardown reclaims it.
|
||||
|
||||
A member named by user_id must already exist for a non-proxy-admin caller
|
||||
— `_validate_member_user_id_provisioning` 403s a user_id with no user row
|
||||
— so an authz matrix targeting a member has to seed one, or every
|
||||
non-proxy-admin row 403s on provisioning and the permission gate under
|
||||
test goes unexercised.
|
||||
"""
|
||||
user_id = f"{scratch_prefix}-{suffix}"
|
||||
data: Dict[str, Any] = {"user_id": user_id, "user_role": "internal_user"}
|
||||
if user_email is not None:
|
||||
data["user_email"] = user_email
|
||||
await prisma.db.litellm_usertable.create(data=data)
|
||||
return user_id
|
||||
|
||||
|
||||
async def create_scratch_org(
|
||||
prisma,
|
||||
scratch_prefix: str,
|
||||
|
|
@ -331,8 +358,16 @@ async def scratch(prisma):
|
|||
await prisma.db.litellm_teamtable.delete_many(
|
||||
where={"team_id": {"startswith": handle.prefix}}
|
||||
)
|
||||
# Inviting a member by user_email allocates the user_id server-side
|
||||
# (a uuid), so a scratch-prefixed email is the only handle on that
|
||||
# row — sweep both, matching the token sweep above.
|
||||
await prisma.db.litellm_usertable.delete_many(
|
||||
where={"user_id": {"startswith": handle.prefix}}
|
||||
where={
|
||||
"OR": [
|
||||
{"user_id": {"startswith": handle.prefix}},
|
||||
{"user_email": {"startswith": handle.prefix}},
|
||||
]
|
||||
}
|
||||
)
|
||||
# F1+F3 seed scratch orgs via create_scratch_org; the world seeder is
|
||||
# the only other writer of LiteLLM_OrganizationTable and uses the
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import litellm
|
|||
import pytest
|
||||
|
||||
from .actors import Actor
|
||||
from .conftest import create_scratch_team
|
||||
from .conftest import create_scratch_team, create_scratch_user
|
||||
|
||||
pytestmark = pytest.mark.asyncio(loop_scope="session")
|
||||
|
||||
|
|
@ -12,6 +12,12 @@ pytestmark = pytest.mark.asyncio(loop_scope="session")
|
|||
# or an org admin of the team's org may add members; everyone else is 403.
|
||||
# Unlike /team/update there is no route gate in front, so the team-admin
|
||||
# branch is reachable (TEAM_ADMIN, an internal_user, is allowed on its team).
|
||||
#
|
||||
# The member being added is seeded as a real user row so this matrix reads the
|
||||
# permission gate alone. Naming a user_id with no user row would 403 every
|
||||
# non-proxy-admin on the downstream provisioning gate instead, which would
|
||||
# leave the matrix green even with _validate_team_member_add_permissions
|
||||
# deleted. That gate gets its own matrix below.
|
||||
_MATRIX = [
|
||||
("alpha/proxy_admin", Actor.PROXY_ADMIN, "alpha", 200),
|
||||
("alpha/org_admin", Actor.ORG_ADMIN, "alpha", 200),
|
||||
|
|
@ -68,7 +74,9 @@ async def test_team_member_add_authz_matrix(
|
|||
):
|
||||
await _seed_target(prisma, world, shape, scratch.prefix)
|
||||
caller = world.keys[actor]
|
||||
new_member_id = scratch.tag("newmember")
|
||||
new_member_id = await create_scratch_user(
|
||||
prisma, scratch.prefix, suffix="newmember"
|
||||
)
|
||||
|
||||
resp = await proxy_client.post(
|
||||
"/team/member_add",
|
||||
|
|
@ -92,6 +100,110 @@ async def test_team_member_add_authz_matrix(
|
|||
assert new_member_id not in _member_ids(row), "denied but member added"
|
||||
|
||||
|
||||
# Naming a user_id with no user row creates that user as a side effect, so
|
||||
# _validate_member_user_id_provisioning restricts it to PROXY_ADMIN — creating
|
||||
# users outright is proxy-admin-only, and member_add must not be a way around
|
||||
# that. Every actor here clears the permission gate on the alpha team (all
|
||||
# three are 200 in the matrix above), so the only thing separating them is the
|
||||
# provisioning gate.
|
||||
_UNPROVISIONED = [
|
||||
("proxy_admin", Actor.PROXY_ADMIN, 200),
|
||||
("team_admin", Actor.TEAM_ADMIN, 403),
|
||||
("org_admin", Actor.ORG_ADMIN, 403),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"actor,expected_status",
|
||||
[(a, s) for (_id, a, s) in _UNPROVISIONED],
|
||||
ids=[s[0] for s in _UNPROVISIONED],
|
||||
)
|
||||
async def test_team_member_add_unprovisioned_user_id_is_proxy_admin_only(
|
||||
actor: Actor,
|
||||
expected_status: int,
|
||||
proxy_client,
|
||||
prisma,
|
||||
scratch,
|
||||
world,
|
||||
):
|
||||
await _seed_target(prisma, world, "alpha", scratch.prefix)
|
||||
caller = world.keys[actor]
|
||||
# Deliberately NOT seeded — create_scratch_user is what the matrix above
|
||||
# calls, and its absence here is the whole scenario.
|
||||
unprovisioned_id = scratch.tag("unprovisioned")
|
||||
assert (
|
||||
await prisma.db.litellm_usertable.find_unique(
|
||||
where={"user_id": unprovisioned_id}
|
||||
)
|
||||
is None
|
||||
), "setup: user must not exist yet"
|
||||
|
||||
resp = await proxy_client.post(
|
||||
"/team/member_add",
|
||||
headers={"Authorization": f"Bearer {caller.cleartext}"},
|
||||
json={
|
||||
"team_id": scratch.prefix,
|
||||
"member": {"user_id": unprovisioned_id, "role": "user"},
|
||||
},
|
||||
)
|
||||
assert (
|
||||
resp.status_code == expected_status
|
||||
), f"{actor.value}: {resp.status_code} {resp.text}"
|
||||
|
||||
row = await prisma.db.litellm_teamtable.find_unique(
|
||||
where={"team_id": scratch.prefix}
|
||||
)
|
||||
assert row is not None
|
||||
created = await prisma.db.litellm_usertable.find_unique(
|
||||
where={"user_id": unprovisioned_id}
|
||||
)
|
||||
if expected_status == 200:
|
||||
assert unprovisioned_id in _member_ids(row)
|
||||
assert created is not None, "proxy admin add did not provision the user"
|
||||
else:
|
||||
assert unprovisioned_id not in _member_ids(row), "denied but member added"
|
||||
# The point of the gate: a denied caller must not leave a user row behind.
|
||||
assert created is None, "denied but user row was created"
|
||||
|
||||
|
||||
async def test_team_member_add_email_invite_open_to_team_admin(
|
||||
proxy_client,
|
||||
prisma,
|
||||
scratch,
|
||||
world,
|
||||
):
|
||||
"""The escape hatch the provisioning 403 names must actually work.
|
||||
|
||||
That message tells a non-proxy-admin to add the member by user_email
|
||||
instead. Inviting an unknown email allocates the user_id server-side, so
|
||||
it stays open to team admins; if it ever closed, the 403 would be sending
|
||||
callers down a dead end.
|
||||
"""
|
||||
await _seed_target(prisma, world, "alpha", scratch.prefix)
|
||||
caller = world.keys[Actor.TEAM_ADMIN]
|
||||
email = f"{scratch.prefix}-invitee@example.com"
|
||||
|
||||
resp = await proxy_client.post(
|
||||
"/team/member_add",
|
||||
headers={"Authorization": f"Bearer {caller.cleartext}"},
|
||||
json={
|
||||
"team_id": scratch.prefix,
|
||||
"member": {"user_email": email, "role": "user"},
|
||||
},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
|
||||
invited = await prisma.db.litellm_usertable.find_first(
|
||||
where={"user_email": email}
|
||||
)
|
||||
assert invited is not None, "invite did not create the user"
|
||||
row = await prisma.db.litellm_teamtable.find_unique(
|
||||
where={"team_id": scratch.prefix}
|
||||
)
|
||||
assert row is not None
|
||||
assert invited.user_id in _member_ids(row)
|
||||
|
||||
|
||||
# Available-team self-join: a non-admin caller may add ITSELF to a team listed
|
||||
# in litellm.default_internal_user_params["available_teams"], but the bypass
|
||||
# must not escalate to role=admin or inject another user.
|
||||
|
|
|
|||
|
|
@ -11,33 +11,14 @@ shape: a payload that silently lands the membership against the WRONG
|
|||
user_id is invisible from response-body alone).
|
||||
"""
|
||||
|
||||
from typing import Any, Dict, Optional
|
||||
|
||||
import pytest
|
||||
|
||||
from .actors import Actor
|
||||
from .conftest import create_scratch_team
|
||||
from .conftest import create_scratch_team, create_scratch_user
|
||||
|
||||
pytestmark = pytest.mark.asyncio(loop_scope="session")
|
||||
|
||||
|
||||
async def _seed_scratch_user(
|
||||
prisma,
|
||||
scratch_prefix: str,
|
||||
*,
|
||||
suffix: str,
|
||||
user_email: Optional[str] = None,
|
||||
) -> str:
|
||||
"""Raw-seed a scratch-prefixed user row; returns user_id. Scratch teardown
|
||||
reclaims by user_id prefix."""
|
||||
user_id = f"{scratch_prefix}-{suffix}"
|
||||
data: Dict[str, Any] = {"user_id": user_id, "user_role": "internal_user"}
|
||||
if user_email is not None:
|
||||
data["user_email"] = user_email
|
||||
await prisma.db.litellm_usertable.create(data=data)
|
||||
return user_id
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Both None → 400 ("Either user_id or user_email must be provided")
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
@ -68,10 +49,10 @@ async def test_member_add_email_id_mismatch_rejected(
|
|||
proxy_client, prisma, scratch, world
|
||||
):
|
||||
email = f"{scratch.prefix}-mismatch@example.com"
|
||||
real_user_id = await _seed_scratch_user(
|
||||
real_user_id = await create_scratch_user(
|
||||
prisma, scratch.prefix, suffix="real", user_email=email
|
||||
)
|
||||
other_user_id = await _seed_scratch_user(prisma, scratch.prefix, suffix="other")
|
||||
other_user_id = await create_scratch_user(prisma, scratch.prefix, suffix="other")
|
||||
assert real_user_id != other_user_id # sanity
|
||||
team_id = await create_scratch_team(prisma, team_id=scratch.tag("team"))
|
||||
seeder = world.keys[Actor.PROXY_ADMIN].cleartext
|
||||
|
|
@ -102,7 +83,7 @@ async def test_member_add_email_only_resolves_user_id(
|
|||
proxy_client, prisma, scratch, world
|
||||
):
|
||||
email = f"{scratch.prefix}-resolve@example.com"
|
||||
user_id = await _seed_scratch_user(
|
||||
user_id = await create_scratch_user(
|
||||
prisma, scratch.prefix, suffix="lookup", user_email=email
|
||||
)
|
||||
team_id = await create_scratch_team(prisma, team_id=scratch.tag("team"))
|
||||
|
|
@ -171,8 +152,8 @@ async def test_member_add_duplicate_email_rejected(
|
|||
proxy_client, prisma, scratch, world
|
||||
):
|
||||
email = f"{scratch.prefix}-dup@example.com"
|
||||
await _seed_scratch_user(prisma, scratch.prefix, suffix="dup1", user_email=email)
|
||||
await _seed_scratch_user(prisma, scratch.prefix, suffix="dup2", user_email=email)
|
||||
await create_scratch_user(prisma, scratch.prefix, suffix="dup1", user_email=email)
|
||||
await create_scratch_user(prisma, scratch.prefix, suffix="dup2", user_email=email)
|
||||
team_id = await create_scratch_team(prisma, team_id=scratch.tag("team"))
|
||||
seeder = world.keys[Actor.PROXY_ADMIN].cleartext
|
||||
resp = await proxy_client.post(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue