From 67d8ba01cf3f3366f4472a915bd264e163c787d5 Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Wed, 7 Oct 2026 17:53:52 +0900 Subject: [PATCH 01/10] fix(teams): let member_add pick one of several users sharing an email The Add Team Member modal sent both user_email and user_id, and the backend rejected any email owned by more than one user even when the user_id named one of them. The backend now accepts that pair, and the modal lists a shared email once and asks for the user ID among its owners before submitting --- .../management_endpoints/team_endpoints.py | 2 + .../test_team_endpoints.py | 28 +++++++++ .../user_search_modal.integration.test.tsx | 43 +++++++++++++ .../common_components/user_search_modal.tsx | 60 +++++++++++++------ 4 files changed, 114 insertions(+), 19 deletions(-) diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index aeb317c8905..a5ac743bd6a 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -3286,6 +3286,8 @@ async def _validate_and_populate_member_user_info( return member if isinstance(users_by_email, list) and len(users_by_email) > 1: + if any(user.user_id == member.user_id for user in users_by_email): + return member raise HTTPException( status_code=400, detail={ diff --git a/tests/unit/proxy/management_endpoints/test_team_endpoints.py b/tests/unit/proxy/management_endpoints/test_team_endpoints.py index 6c5447fce64..6016f61ea14 100644 --- a/tests/unit/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_team_endpoints.py @@ -10661,6 +10661,34 @@ async def test_validate_and_populate_member_user_info_both_provided_match(): ) +def _users_sharing_email(email: str, *user_ids: str) -> list[MagicMock]: + return [MagicMock(user_id=user_id, user_email=email) for user_id in user_ids] + + +@pytest.mark.asyncio +async def test_validate_and_populate_member_user_info_duplicate_email_accepts_matching_user_id(): + member = Member(user_email="dup@example.com", user_id="user-b", role="user") + mock_prisma_client = MagicMock() + mock_prisma_client.get_data = AsyncMock(return_value=_users_sharing_email("dup@example.com", "user-a", "user-b")) + + result = await _validate_and_populate_member_user_info(member=member, prisma_client=mock_prisma_client) + + assert (result.user_id, result.user_email) == ("user-b", "dup@example.com") + + +@pytest.mark.asyncio +async def test_validate_and_populate_member_user_info_duplicate_email_rejects_unrelated_user_id(): + member = Member(user_email="dup@example.com", user_id="user-c", role="user") + mock_prisma_client = MagicMock() + mock_prisma_client.get_data = AsyncMock(return_value=_users_sharing_email("dup@example.com", "user-a", "user-b")) + + with pytest.raises(HTTPException) as exc_info: + await _validate_and_populate_member_user_info(member=member, prisma_client=mock_prisma_client) + + assert exc_info.value.status_code == 400 + assert "Multiple users found with email 'dup@example.com'" in str(exc_info.value.detail) + + @pytest.mark.asyncio async def test_validate_and_populate_member_user_info_only_email_provided(): """ diff --git a/ui/litellm-dashboard/src/components/common_components/user_search_modal.integration.test.tsx b/ui/litellm-dashboard/src/components/common_components/user_search_modal.integration.test.tsx index 5769d8c2f88..f7052243e82 100644 --- a/ui/litellm-dashboard/src/components/common_components/user_search_modal.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/common_components/user_search_modal.integration.test.tsx @@ -205,6 +205,49 @@ describe("UserSearchModal submit payload", () => { }); }); +describe("UserSearchModal users sharing an email", () => { + beforeEach(() => { + vi.mocked(userFilterUICall).mockReset(); + vi.mocked(userFilterUICall).mockResolvedValue([ + { user_id: "u-first", user_email: "shared@example.com" }, + { user_id: "u-second", user_email: "shared@example.com" }, + ] as never); + }); + + it("lists the shared email once and submits the user ID picked from its owners", async () => { + const user = userEvent.setup(); + const onSubmit = vi.fn(); + render(); + const save = screen.getByRole("button", { name: /add member/i }); + + const emailInput = getEmailSearchInput(); + await user.click(emailInput); + await user.type(emailInput, "shared"); + await waitFor(() => expect(userFilterUICall).toHaveBeenCalled(), { timeout: 3000 }); + const emailOptions = await screen.findAllByRole("option"); + expect(emailOptions.map((option) => option.textContent)).toEqual(["shared@example.com"]); + await user.click(emailOptions[0]); + + expect(screen.getByLabelText("User ID")).toHaveValue(""); + expect(screen.getByText(/multiple users share this email/i)).toBeInTheDocument(); + expect(save).toBeDisabled(); + + await user.click(screen.getByLabelText("User ID")); + const idOptions = await screen.findAllByRole("option"); + expect(idOptions.map((option) => option.textContent)).toEqual(["u-first", "u-second"]); + await user.click(screen.getByRole("option", { name: "u-second" })); + await user.click(save); + + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(onSubmit.mock.calls[0][0]).toStrictEqual({ + user_email: "shared@example.com", + user_id: "u-second", + role: "user", + }); + expect(userFilterUICall).toHaveBeenCalledTimes(1); + }); +}); + describe("UserSearchModal search lifecycle", () => { const directory = [ { user_id: "u-jones", user_email: "alice.jones@example.com" }, diff --git a/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx b/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx index 04ac412397f..c1a10176fc0 100644 --- a/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx +++ b/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx @@ -18,12 +18,21 @@ interface User { role?: string; } +type SearchField = "user_email" | "user_id"; + interface UserOption { label: string; value: string; - user: User | null; + users: User[]; } +const groupUsersByField = (users: User[], fieldName: SearchField): UserOption[] => + [...new Set(users.map((user) => `${user[fieldName]}`))].map((value) => ({ + label: value, + value, + users: users.filter((user) => `${user[fieldName]}` === value), + })); + interface Role { label: string; value: string; @@ -70,11 +79,14 @@ const UserSearchModal: React.FC = ({ const selectedUserEmail = form.watch("user_email"); const [userOptions, setUserOptions] = useState([]); const [loading, setLoading] = useState(false); - const [selectedField, setSelectedField] = useState<"user_email" | "user_id">("user_email"); + const [selectedField, setSelectedField] = useState("user_email"); + const [usersSharingEmail, setUsersSharingEmail] = useState([]); const [isSubmitting, setIsSubmitting] = useState(false); const latestSearchRef = useRef(0); + const needsUserIdPick = usersSharingEmail.length > 0 && !selectedUserId; + const hasPickedUser = Boolean(selectedUserId || selectedUserEmail) && !needsUserIdPick; - const fetchUsers = async (searchText: string, fieldName: "user_email" | "user_id"): Promise => { + const fetchUsers = async (searchText: string, fieldName: SearchField): Promise => { const searchId = latestSearchRef.current + 1; latestSearchRef.current = searchId; const isLatestSearch = (): boolean => searchId === latestSearchRef.current; @@ -99,12 +111,7 @@ const UserSearchModal: React.FC = ({ if (!isLatestSearch()) return; const data: User[] = response; - const options: UserOption[] = data.map((user) => ({ - label: fieldName === "user_email" ? `${user.user_email}` : `${user.user_id}`, - value: fieldName === "user_email" ? user.user_email : user.user_id, - user, - })); - setUserOptions(options); + setUserOptions(groupUsersByField(data, fieldName)); } catch (error) { console.error("Error fetching users:", error); } finally { @@ -112,15 +119,22 @@ const UserSearchModal: React.FC = ({ } }; - const handleSearch = (value: string, fieldName: "user_email" | "user_id"): void => { + const handleSearch = (value: string, fieldName: SearchField): void => { setSelectedField(fieldName); void fetchUsers(value, fieldName); }; - const handleSelect = (option: UserOption | null): void => { - if (option?.user == null) return; - form.setValue("user_email", option.user.user_email); - form.setValue("user_id", option.user.user_id); + const handleSelect = (option: UserOption | null, fieldName: SearchField): void => { + const [firstUser, ...otherUsers] = option?.users ?? []; + if (firstUser === undefined) return; + const isAmbiguous = otherUsers.length > 0; + form.setValue("user_email", firstUser.user_email); + form.setValue("user_id", isAmbiguous ? null : firstUser.user_id); + if (fieldName === "user_email") { + setUsersSharingEmail(isAmbiguous ? option?.users ?? [] : []); + return; + } + setUsersSharingEmail((users) => (users.some((user) => user.user_id === firstUser.user_id) ? users : [])); }; const handleSubmit = async (values: FormValues): Promise => { @@ -135,6 +149,7 @@ const UserSearchModal: React.FC = ({ const handleClose = (): void => { form.reset(emptyValues); setUserOptions([]); + setUsersSharingEmail([]); onCancel(); }; @@ -143,7 +158,7 @@ const UserSearchModal: React.FC = ({ }; const renderUserSearch = ( - fieldName: "user_email" | "user_id", + fieldName: SearchField, placeholder: string, controlProps: { id: string; @@ -152,15 +167,17 @@ const UserSearchModal: React.FC = ({ }, testId?: string, ) => { - const items = selectedField === fieldName ? userOptions : []; + const sharedEmailOptions = fieldName === "user_id" ? groupUsersByField(usersSharingEmail, "user_id") : []; + const items = selectedField === fieldName ? userOptions : sharedEmailOptions; const handleValueChange = (value: string | null) => { if (value === null) { form.setValue("user_email", null); form.setValue("user_id", null); + setUsersSharingEmail([]); return; } controlProps.onChange(value); - handleSelect(items.find((option) => option.value === value) ?? null); + handleSelect(items.find((option) => option.value === value) ?? null, fieldName); }; return (
@@ -205,7 +222,12 @@ const UserSearchModal: React.FC = ({
OR
- + {({ id, value, onChange }) => renderUserSearch("user_id", "Search by user ID", { id, value, onChange })} @@ -238,7 +260,7 @@ const UserSearchModal: React.FC = ({
- From d1f7dd1bdf893aa53d614898ee1ab6d304eeab0c Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Wed, 7 Oct 2026 17:53:52 +0900 Subject: [PATCH 02/10] fix(teams): reject re-adding a member already in the team The member_add duplicate check counted a member once per matching field, so re-adding an existing member with both user_id and user_email produced two hits, skipped the single-member error and returned 200. It now counts each requested member once with the same rule the roster write uses. The UI also reads the proxy error envelope so the real reason is shown instead of a generic failure --- .../management_endpoints/team_endpoints.py | 28 +++------------- .../test_team_endpoints.py | 26 +++++++++++++++ .../src/components/networking.test.ts | 33 +++++++++++++++++++ .../src/components/networking.tsx | 3 +- 4 files changed, 65 insertions(+), 25 deletions(-) diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index a5ac743bd6a..c3eebc94965 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -2739,32 +2739,12 @@ def team_member_add_duplication_check( ): """ Check if a member already exists in the team. - This check is done BEFORE we create/fetch the user, so it only prevents - obvious duplicates where both user_id and user_email match exactly. """ + members: Final = data.member if isinstance(data.member, list) else [data.member] + invalid_team_members: Final = tuple( + member for member in members if _member_already_in_team(member, existing_team_row) + ) - invalid_team_members: Final = [] - - def _check_member_duplication(member: Member): - if member.user_id is not None: - for existing_member in existing_team_row.members_with_roles: - if existing_member.user_id == member.user_id: - invalid_team_members.append(member) - - # Check by user_email if provided - if member.user_email is not None: - for existing_member in existing_team_row.members_with_roles: - if existing_member.user_email == member.user_email: - invalid_team_members.append(member) - - # First, populate the invalid_team_members list by checking for duplicates - if isinstance(data.member, Member): - _check_member_duplication(data.member) - elif isinstance(data.member, list): - for m in data.member: - _check_member_duplication(m) - - # Then check the populated list and raise exceptions if needed if isinstance(data.member, list) and len(invalid_team_members) == len(data.member): raise ProxyException( message=f"All users are already in team. Existing members={existing_team_row.members_with_roles}", diff --git a/tests/unit/proxy/management_endpoints/test_team_endpoints.py b/tests/unit/proxy/management_endpoints/test_team_endpoints.py index 6016f61ea14..74960fb573a 100644 --- a/tests/unit/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_team_endpoints.py @@ -1170,6 +1170,32 @@ def test_team_member_add_duplication_check_raises_proxy_exception(): assert "already in team" in str(exc_info.value.message) +def _team_with_member_sharing_email() -> MagicMock: + team = MagicMock(spec=LiteLLM_TeamTable) + team.team_id = "test-team-123" + team.members_with_roles = [Member(user_id="dup-user-1", user_email="shared@example.com", role="user")] + return team + + +@pytest.mark.parametrize( + "member", + [ + Member(user_id="dup-user-1", user_email="shared@example.com", role="user"), + Member(user_id="dup-user-1", role="user"), + Member(user_email="shared@example.com", role="user"), + Member(user_id="dup-user-2", user_email="shared@example.com", role="user"), + ], + ids=["id_and_email", "id_only", "email_only", "other_user_same_email"], +) +def test_team_member_add_duplication_check_rejects_member_already_in_team(member: Member): + data = TeamMemberAddRequest(team_id="test-team-123", member=member) + + with pytest.raises(ProxyException) as exc_info: + team_member_add_duplication_check(data=data, existing_team_row=_team_with_member_sharing_email()) + + assert exc_info.value.type == ProxyErrorTypes.team_member_already_in_team + + def test_team_member_add_duplication_check_allows_new_member(): """ Test that team_member_add_duplication_check allows adding a new member who is not already in the team diff --git a/ui/litellm-dashboard/src/components/networking.test.ts b/ui/litellm-dashboard/src/components/networking.test.ts index 62670467827..54af5c433e8 100644 --- a/ui/litellm-dashboard/src/components/networking.test.ts +++ b/ui/litellm-dashboard/src/components/networking.test.ts @@ -118,6 +118,39 @@ describe("loginCall - storeLoginToken integration", () => { }); }); +describe("teamMemberAddCall error message", () => { + const originalFetch = global.fetch; + + afterEach(() => { + global.fetch = originalFetch; + }); + + const rejectWith = (body: string) => { + global.fetch = vi.fn().mockResolvedValue({ ok: false, text: async () => body }) as any; + }; + + const member: Networking.Member = { user_id: "dup-user-1", role: "user" }; + + it.each([ + ["the proxy error envelope", { error: { message: "User already in team. Member: user_id=dup-user-1" } }], + ["an HTTPException detail", { detail: { error: "User already in team. Member: user_id=dup-user-1" } }], + ])("surfaces the backend message from %s", async (_shape, body) => { + rejectWith(JSON.stringify(body)); + + await expect(Networking.teamMemberAddCall("token", "team-1", member)).rejects.toThrow( + "User already in team. Member: user_id=dup-user-1", + ); + }); + + it("falls back to a generic message when the body is not JSON", async () => { + rejectWith("bad gateway"); + + await expect(Networking.teamMemberAddCall("token", "team-1", member)).rejects.toThrow( + "Failed to add team member", + ); + }); +}); + describe("modelInfoCall", () => { let currentFetch: typeof global.fetch; diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index a7e9387cbe6..91ebbd1c3bb 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -2767,7 +2767,8 @@ export const teamMemberAddCall = async (accessToken: string, teamId: string, for console.warn("Failed to parse error body as JSON:", errorText); } - const rawMessage = parsedError?.detail?.error || "Failed to add team member"; + const rawMessage = + Object.keys(parsedError).length > 0 ? deriveErrorMessage(parsedError) : "Failed to add team member"; const err = new Error(rawMessage); (err as any).raw = parsedError; throw err; From 7b157c31dd6737e39928b7c05c58e19589044147 Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Wed, 7 Oct 2026 18:17:09 +0900 Subject: [PATCH 03/10] fix(teams): say which member blocks a member_add instead of dumping the roster The already-in-team error printed every team member's id and email, which buried the reason and exposed the roster to whoever made the request. It now names the existing member that matched: the user id when it is already on the team, or the member already using the requested email --- .../management_endpoints/team_endpoints.py | 30 ++++++++---- .../test_team_endpoints.py | 48 +++++++++++++++---- .../src/components/networking.test.ts | 6 +-- 3 files changed, 65 insertions(+), 19 deletions(-) diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index c3eebc94965..729ec4f698a 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -2747,14 +2747,14 @@ def team_member_add_duplication_check( if isinstance(data.member, list) and len(invalid_team_members) == len(data.member): raise ProxyException( - message=f"All users are already in team. Existing members={existing_team_row.members_with_roles}", + message="All requested users are already members of this team.", type=ProxyErrorTypes.team_member_already_in_team, param="member", code="400", ) - elif isinstance(data.member, Member) and len(invalid_team_members) == 1: + elif isinstance(data.member, Member) and (existing_member := _matching_team_member(data.member, existing_team_row)): raise ProxyException( - message=f"User already in team. Member: user_id={data.member.user_id}, user_email={data.member.user_email}. Existing members={existing_team_row.members_with_roles}", + message=_already_in_team_message(data.member, existing_member), type=ProxyErrorTypes.team_member_already_in_team, param="member", code="400", @@ -2932,14 +2932,28 @@ def _resolve_member_identity(member: Member, updated_users: Sequence[LiteLLM_Use ) -def _member_already_in_team(member: Member, complete_team_data: LiteLLM_TeamTable) -> bool: - return any( - (member.user_id is not None and existing_member.user_id == member.user_id) - or (member.user_email is not None and existing_member.user_email == member.user_email) - for existing_member in complete_team_data.members_with_roles +def _matching_team_member(member: Member, complete_team_data: LiteLLM_TeamTable) -> Member | None: + return next( + ( + existing_member + for existing_member in complete_team_data.members_with_roles + if (member.user_id is not None and existing_member.user_id == member.user_id) + or (member.user_email is not None and existing_member.user_email == member.user_email) + ), + None, ) +def _member_already_in_team(member: Member, complete_team_data: LiteLLM_TeamTable) -> bool: + return _matching_team_member(member, complete_team_data) is not None + + +def _already_in_team_message(member: Member, existing_member: Member) -> str: + if existing_member.user_id is None or existing_member.user_id == member.user_id: + return f"{member.user_id or member.user_email} is already a member of this team." + return f"{member.user_email} is already used by team member {existing_member.user_id}." + + async def _update_team_members_list( data: TeamMemberAddRequest, complete_team_data: LiteLLM_TeamTable, diff --git a/tests/unit/proxy/management_endpoints/test_team_endpoints.py b/tests/unit/proxy/management_endpoints/test_team_endpoints.py index 74960fb573a..1e65789d7dd 100644 --- a/tests/unit/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_team_endpoints.py @@ -1166,8 +1166,7 @@ def test_team_member_add_duplication_check_raises_proxy_exception(): assert exc_info.value.type == ProxyErrorTypes.team_member_already_in_team assert exc_info.value.param == "member" assert exc_info.value.code == "400" - assert "existing-user-id" in str(exc_info.value.message) - assert "already in team" in str(exc_info.value.message) + assert exc_info.value.message == "existing-user-id is already a member of this team." def _team_with_member_sharing_email() -> MagicMock: @@ -1178,22 +1177,55 @@ def _team_with_member_sharing_email() -> MagicMock: @pytest.mark.parametrize( - "member", + ("member", "expected_message"), [ - Member(user_id="dup-user-1", user_email="shared@example.com", role="user"), - Member(user_id="dup-user-1", role="user"), - Member(user_email="shared@example.com", role="user"), - Member(user_id="dup-user-2", user_email="shared@example.com", role="user"), + ( + Member(user_id="dup-user-1", user_email="shared@example.com", role="user"), + "dup-user-1 is already a member of this team.", + ), + (Member(user_id="dup-user-1", role="user"), "dup-user-1 is already a member of this team."), + ( + Member(user_email="shared@example.com", role="user"), + "shared@example.com is already used by team member dup-user-1.", + ), + ( + Member(user_id="dup-user-2", user_email="shared@example.com", role="user"), + "shared@example.com is already used by team member dup-user-1.", + ), ], ids=["id_and_email", "id_only", "email_only", "other_user_same_email"], ) -def test_team_member_add_duplication_check_rejects_member_already_in_team(member: Member): +def test_team_member_add_duplication_check_rejects_member_already_in_team(member: Member, expected_message: str): data = TeamMemberAddRequest(team_id="test-team-123", member=member) with pytest.raises(ProxyException) as exc_info: team_member_add_duplication_check(data=data, existing_team_row=_team_with_member_sharing_email()) assert exc_info.value.type == ProxyErrorTypes.team_member_already_in_team + assert exc_info.value.message == expected_message + + +def test_team_member_add_duplication_check_names_email_only_member_by_email(): + team = MagicMock(spec=LiteLLM_TeamTable) + team.members_with_roles = [Member(user_email="invited@example.com", role="user")] + data = TeamMemberAddRequest(team_id="test-team-123", member=Member(user_email="invited@example.com", role="user")) + + with pytest.raises(ProxyException) as exc_info: + team_member_add_duplication_check(data=data, existing_team_row=team) + + assert exc_info.value.message == "invited@example.com is already a member of this team." + + +def test_team_member_add_duplication_check_bulk_message_omits_roster(): + data = TeamMemberAddRequest( + team_id="test-team-123", + member=[Member(user_id="dup-user-1", role="user"), Member(user_email="shared@example.com", role="user")], + ) + + with pytest.raises(ProxyException) as exc_info: + team_member_add_duplication_check(data=data, existing_team_row=_team_with_member_sharing_email()) + + assert exc_info.value.message == "All requested users are already members of this team." def test_team_member_add_duplication_check_allows_new_member(): diff --git a/ui/litellm-dashboard/src/components/networking.test.ts b/ui/litellm-dashboard/src/components/networking.test.ts index 54af5c433e8..832d1afe172 100644 --- a/ui/litellm-dashboard/src/components/networking.test.ts +++ b/ui/litellm-dashboard/src/components/networking.test.ts @@ -132,13 +132,13 @@ describe("teamMemberAddCall error message", () => { const member: Networking.Member = { user_id: "dup-user-1", role: "user" }; it.each([ - ["the proxy error envelope", { error: { message: "User already in team. Member: user_id=dup-user-1" } }], - ["an HTTPException detail", { detail: { error: "User already in team. Member: user_id=dup-user-1" } }], + ["the proxy error envelope", { error: { message: "dup-user-1 is already a member of this team." } }], + ["an HTTPException detail", { detail: { error: "dup-user-1 is already a member of this team." } }], ])("surfaces the backend message from %s", async (_shape, body) => { rejectWith(JSON.stringify(body)); await expect(Networking.teamMemberAddCall("token", "team-1", member)).rejects.toThrow( - "User already in team. Member: user_id=dup-user-1", + "dup-user-1 is already a member of this team.", ); }); From d7d18baf8da1aef3f57d5663578ce9eac969e31f Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Thu, 8 Oct 2026 09:55:21 +0900 Subject: [PATCH 04/10] fix(teams): check member_add permissions before the duplicate lookup The already-in-team check ran before the caller's permissions were validated, so any internal user could post an email to a team they do not manage and learn which member uses it. Permissions are now checked first, and the duplicate check only answers callers allowed to add to the team --- .../management_endpoints/team_endpoints.py | 11 +++--- .../test_team_endpoints.py | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index 729ec4f698a..eacd3f8e6cd 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -3414,18 +3414,17 @@ async def team_member_add( complete_team_data: Final = LiteLLM_TeamTable.model_validate(existing_team_row.model_dump()) - team_member_add_duplication_check( - data=data, - existing_team_row=complete_team_data, - ) - - # Validate permissions await _validate_team_member_add_permissions( user_api_key_dict=user_api_key_dict, complete_team_data=complete_team_data, data=data, ) + team_member_add_duplication_check( + data=data, + existing_team_row=complete_team_data, + ) + requested_members: Final = tuple(data.member) if isinstance(data.member, list) else (data.member,) caller_supplied_user_ids = frozenset(member.user_id for member in requested_members if member.user_id is not None) existing_user_ids: Final = await _resolve_existing_member_user_ids( diff --git a/tests/unit/proxy/management_endpoints/test_team_endpoints.py b/tests/unit/proxy/management_endpoints/test_team_endpoints.py index 1e65789d7dd..280597598f4 100644 --- a/tests/unit/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_team_endpoints.py @@ -13743,6 +13743,41 @@ async def test_team_member_add_evicts_the_cached_team_roster(monkeypatch): assert cache.get_cache(key="team_alias:roster-evict") is None +@pytest.mark.asyncio +async def test_team_member_add_rejects_non_admin_before_revealing_existing_members(monkeypatch): + from litellm.proxy._types import TeamMemberAddRequest + from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache + from litellm.proxy.management_endpoints.team_endpoints import team_member_add + + team_row = LiteLLM_TeamTable( + team_id="team-private", + members_with_roles=[Member(user_id="dup-user-1", user_email="shared@example.com", role="user")], + ) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", AsyncMock()) + monkeypatch.setattr("litellm.proxy.proxy_server.user_api_key_cache", UserApiKeyCache()) + monkeypatch.setattr("litellm.proxy.proxy_server.proxy_logging_obj", None) + monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", True) + monkeypatch.setattr("litellm.proxy.proxy_server.litellm_proxy_admin_name", "default_user_id") + + with ( + patch( + "litellm.proxy.management_endpoints.team_endpoints.get_team_object", + new_callable=AsyncMock, + return_value=team_row, + ), + pytest.raises(HTTPException) as exc_info, + ): + await team_member_add( + data=TeamMemberAddRequest( + team_id="team-private", member=Member(user_email="shared@example.com", role="user") + ), + user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id="outsider"), + ) + + assert exc_info.value.status_code == 403 + assert "dup-user-1" not in str(exc_info.value.detail) + + class _RecordingAuditLogger(CustomLogger): def __init__(self) -> None: super().__init__() From 2b48d0781b6df4bc7d061a1ac2238957d8e950b3 Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Thu, 8 Oct 2026 10:02:45 +0900 Subject: [PATCH 05/10] fix(ui): keep shared-email user IDs listed after an ID search closes After picking an email shared by several users, searching the User ID field and then closing it left the dropdown empty, so Add Member stayed disabled until another search. The User ID field now shows search results only while there is a query, and falls back to the users sharing the picked email --- .../user_search_modal.integration.test.tsx | 21 +++++++++++++++++++ .../common_components/user_search_modal.tsx | 5 ++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/ui/litellm-dashboard/src/components/common_components/user_search_modal.integration.test.tsx b/ui/litellm-dashboard/src/components/common_components/user_search_modal.integration.test.tsx index f7052243e82..b7582abcbf3 100644 --- a/ui/litellm-dashboard/src/components/common_components/user_search_modal.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/common_components/user_search_modal.integration.test.tsx @@ -246,6 +246,27 @@ describe("UserSearchModal users sharing an email", () => { }); expect(userFilterUICall).toHaveBeenCalledTimes(1); }); + + it("lists the shared email's user IDs again when the ID dropdown reopens after a search", async () => { + const user = userEvent.setup(); + render(); + + const emailInput = getEmailSearchInput(); + await user.click(emailInput); + await user.type(emailInput, "shared"); + await user.click((await screen.findAllByRole("option"))[0]); + + vi.mocked(userFilterUICall).mockResolvedValue([] as never); + const idInput = screen.getByLabelText("User ID"); + await user.click(idInput); + await user.type(idInput, "zzz"); + await waitFor(() => expect(userFilterUICall).toHaveBeenCalledTimes(2), { timeout: 3000 }); + await user.keyboard("{Escape}"); + await user.click(idInput); + + const idOptions = await screen.findAllByRole("option", {}, { timeout: 3000 }); + expect(idOptions.map((option) => option.textContent)).toEqual(["u-first", "u-second"]); + }); }); describe("UserSearchModal search lifecycle", () => { diff --git a/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx b/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx index c1a10176fc0..385e68bb23e 100644 --- a/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx +++ b/ui/litellm-dashboard/src/components/common_components/user_search_modal.tsx @@ -80,6 +80,7 @@ const UserSearchModal: React.FC = ({ const [userOptions, setUserOptions] = useState([]); const [loading, setLoading] = useState(false); const [selectedField, setSelectedField] = useState("user_email"); + const [searchQuery, setSearchQuery] = useState(""); const [usersSharingEmail, setUsersSharingEmail] = useState([]); const [isSubmitting, setIsSubmitting] = useState(false); const latestSearchRef = useRef(0); @@ -121,6 +122,7 @@ const UserSearchModal: React.FC = ({ const handleSearch = (value: string, fieldName: SearchField): void => { setSelectedField(fieldName); + setSearchQuery(value); void fetchUsers(value, fieldName); }; @@ -149,6 +151,7 @@ const UserSearchModal: React.FC = ({ const handleClose = (): void => { form.reset(emptyValues); setUserOptions([]); + setSearchQuery(""); setUsersSharingEmail([]); onCancel(); }; @@ -168,7 +171,7 @@ const UserSearchModal: React.FC = ({ testId?: string, ) => { const sharedEmailOptions = fieldName === "user_id" ? groupUsersByField(usersSharingEmail, "user_id") : []; - const items = selectedField === fieldName ? userOptions : sharedEmailOptions; + const items = selectedField === fieldName && searchQuery !== "" ? userOptions : sharedEmailOptions; const handleValueChange = (value: string | null) => { if (value === null) { form.setValue("user_email", null); From 46dafe8167b730bb4edf9061e47f0446bd8cf1db Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Thu, 8 Oct 2026 10:04:23 +0900 Subject: [PATCH 06/10] fix(ui): fall back to the generic member_add error for empty error bodies A failed /team/member_add whose body was JSON null threw a TypeError while reading the error, so the toast showed that instead of "Failed to add team member". The message is now derived only from a non-empty JSON object. This also reformats the test file that failed the frontend lint check --- .../src/components/networking.test.ts | 12 +++++++----- ui/litellm-dashboard/src/components/networking.tsx | 5 +++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/ui/litellm-dashboard/src/components/networking.test.ts b/ui/litellm-dashboard/src/components/networking.test.ts index 832d1afe172..2f4a0d2223f 100644 --- a/ui/litellm-dashboard/src/components/networking.test.ts +++ b/ui/litellm-dashboard/src/components/networking.test.ts @@ -142,12 +142,14 @@ describe("teamMemberAddCall error message", () => { ); }); - it("falls back to a generic message when the body is not JSON", async () => { - rejectWith("bad gateway"); + it.each([ + ["is not JSON", "bad gateway"], + ["is JSON null", "null"], + ["is an empty object", "{}"], + ])("falls back to a generic message when the body %s", async (_case, body) => { + rejectWith(body); - await expect(Networking.teamMemberAddCall("token", "team-1", member)).rejects.toThrow( - "Failed to add team member", - ); + await expect(Networking.teamMemberAddCall("token", "team-1", member)).rejects.toThrow("Failed to add team member"); }); }); diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 91ebbd1c3bb..1a95b1afcbc 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -2767,8 +2767,9 @@ export const teamMemberAddCall = async (accessToken: string, teamId: string, for console.warn("Failed to parse error body as JSON:", errorText); } - const rawMessage = - Object.keys(parsedError).length > 0 ? deriveErrorMessage(parsedError) : "Failed to add team member"; + const hasErrorBody = + parsedError !== null && typeof parsedError === "object" && Object.keys(parsedError).length > 0; + const rawMessage = hasErrorBody ? deriveErrorMessage(parsedError) : "Failed to add team member"; const err = new Error(rawMessage); (err as any).raw = parsedError; throw err; From b44087796e9f4c0c6ac9cd1b8074fd7e6438e3c9 Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Thu, 8 Oct 2026 14:40:25 +0900 Subject: [PATCH 07/10] fix(teams): decide member_add identity by user_id and check the resolved request When both user_id and user_email are sent, the user_id now decides: an existing user with an email on record must have that email, and a new user_id cannot take an email another user already owns. Duplicate checks look for the same user_id before an email match, so the error names the requested member instead of whichever roster entry came first. The request is checked for an empty list and repeated users, and checked again against the roster after missing ids and emails are filled in, so nothing is written for a conflicting request and an id-only add whose email is already used no longer returns 200 without adding anyone --- .../management_endpoints/team_endpoints.py | 133 ++++++++++-------- 1 file changed, 76 insertions(+), 57 deletions(-) diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index eacd3f8e6cd..dcb8442f6b5 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -2733,6 +2733,39 @@ def team_call_validation_checks( raise HTTPException(status_code=400, detail={"error": str(e)}) +def _requested_members(data: TeamMemberAddRequest) -> tuple[Member, ...]: + return tuple(data.member) if isinstance(data.member, list) else (data.member,) + + +def _first_repeated(values: Sequence[str | None]) -> str | None: + return next((value for index, value in enumerate(values) if value is not None and value in values[:index]), None) + + +def _validate_no_repeated_user_id(members: Sequence[Member]) -> None: + repeated_user_id: Final = _first_repeated(tuple(member.user_id for member in members)) + if repeated_user_id is not None: + raise HTTPException( + status_code=400, + detail={"error": f"{repeated_user_id} appears more than once in this request."}, + ) + + +def _validate_member_request_shape(members: Sequence[Member]) -> None: + if len(members) == 0: + raise HTTPException(status_code=400, detail={"error": "At least one member is required."}) + _validate_no_repeated_user_id(members) + + +def _validate_resolved_members_are_distinct(members: Sequence[Member]) -> None: + _validate_no_repeated_user_id(members) + repeated_email: Final = _first_repeated(tuple(member.user_email for member in members)) + if repeated_email is not None: + raise HTTPException( + status_code=400, + detail={"error": f"Several requested users share the email {repeated_email}. Add only one of them."}, + ) + + def team_member_add_duplication_check( data: TeamMemberAddRequest, existing_team_row: LiteLLM_TeamTable, @@ -2740,12 +2773,12 @@ def team_member_add_duplication_check( """ Check if a member already exists in the team. """ - members: Final = data.member if isinstance(data.member, list) else [data.member] + members: Final = _requested_members(data) invalid_team_members: Final = tuple( member for member in members if _member_already_in_team(member, existing_team_row) ) - if isinstance(data.member, list) and len(invalid_team_members) == len(data.member): + if isinstance(data.member, list) and members and len(invalid_team_members) == len(members): raise ProxyException( message="All requested users are already members of this team.", type=ProxyErrorTypes.team_member_already_in_team, @@ -2817,7 +2850,7 @@ async def _validate_team_member_add_permissions( # Available-team self-join: caller may add only themselves, only as a # standard user. Enforce that here so the bypass cannot be used as a # privilege-escalation or cross-user-injection primitive. - members: Final = data.member if isinstance(data.member, list) else [data.member] + members: Final = _requested_members(data) caller_user_id: Final = getattr(user_api_key_dict, "user_id", None) for member in members: if getattr(member, "role", "user") != "user": @@ -2869,9 +2902,7 @@ async def _process_team_members( if member_allowed_models is None and team_default_member_models: member_allowed_models = team_default_member_models - requested_members: Final[Sequence[Member]] = ( - (data.member,) if isinstance(data.member, Member) else tuple(data.member) - ) + requested_members: Final = _requested_members(data) for m in requested_members: if _member_already_in_team(m, complete_team_data): continue @@ -2933,13 +2964,14 @@ def _resolve_member_identity(member: Member, updated_users: Sequence[LiteLLM_Use def _matching_team_member(member: Member, complete_team_data: LiteLLM_TeamTable) -> Member | None: + roster: Final = complete_team_data.members_with_roles + same_user: Final = next( + (existing for existing in roster if member.user_id is not None and existing.user_id == member.user_id), None + ) + if same_user is not None: + return same_user return next( - ( - existing_member - for existing_member in complete_team_data.members_with_roles - if (member.user_id is not None and existing_member.user_id == member.user_id) - or (member.user_email is not None and existing_member.user_email == member.user_email) - ), + (existing for existing in roster if member.user_email is not None and existing.user_email == member.user_email), None, ) @@ -2949,9 +2981,9 @@ def _member_already_in_team(member: Member, complete_team_data: LiteLLM_TeamTabl def _already_in_team_message(member: Member, existing_member: Member) -> str: - if existing_member.user_id is None or existing_member.user_id == member.user_id: - return f"{member.user_id or member.user_email} is already a member of this team." - return f"{member.user_email} is already used by team member {existing_member.user_id}." + if member.user_id is not None and existing_member.user_id == member.user_id: + return f"{member.user_id} is already a member of this team." + return f"A member of this team already uses {member.user_email}." async def _update_team_members_list( @@ -2960,9 +2992,7 @@ async def _update_team_members_list( updated_users: list[LiteLLM_UserTable], ) -> None: """Update the team's members_with_roles list.""" - requested_members: Final[Sequence[Member]] = ( - (data.member,) if isinstance(data.member, Member) else tuple(data.member) - ) + requested_members: Final = _requested_members(data) resolved_members: Final = tuple(_resolve_member_identity(m, updated_users) for m in requested_members) # extend() consumes the generator as it appends, so a member already added by this @@ -3251,7 +3281,8 @@ async def _validate_and_populate_member_user_info( Validate and populate user_email/user_id for a member. Logic: - 1. If both user_email and user_id are provided, verify they belong to the same user (use user_email as source of truth) + 1. If both user_email and user_id are provided, the user_id decides: an existing user with an email on record + must have that email, and a new user_id must not take an email another user already has 2. If only user_email is provided, populate user_id from DB 3. If only user_id is provided, populate user_email from DB (if user exists) 4. If only user_id is provided and doesn't exist, allow it to pass with user_email as None (will be upserted later) @@ -3265,43 +3296,23 @@ async def _validate_and_populate_member_user_info( detail={"error": "Either user_id or user_email must be provided"}, ) - # Case 1: Both user_email and user_id provided - verify they match if member.user_email is not None and member.user_id is not None: - # Use user_email as source of truth - # Check for multiple users with same email first - users_by_email = await prisma_client.get_data( - key_val={"user_email": member.user_email}, - table_name="user", - query_type="find_all", + mismatch_error: Final = HTTPException( + status_code=400, + detail={ + "error": ( + f"user_email '{member.user_email}' and user_id '{member.user_id}' do not belong to the same user." + ) + }, ) - - if users_by_email is None or (isinstance(users_by_email, list) and len(users_by_email) == 0): - # User doesn't exist yet - this is fine, will be created later + existing_user: Final = await _user_db(prisma_client).find_unique(where={"user_id": member.user_id}) + if existing_user is not None: + if existing_user.user_email is not None and existing_user.user_email != member.user_email: + raise mismatch_error return member - if isinstance(users_by_email, list) and len(users_by_email) > 1: - if any(user.user_id == member.user_id for user in users_by_email): - return member - raise HTTPException( - status_code=400, - detail={ - "error": f"Multiple users found with email '{member.user_email}'. Please use 'user_id' instead." - }, - ) - - # Get the single user - user_by_email = users_by_email[0] - - # Verify the user_id matches - if user_by_email.user_id != member.user_id: - raise HTTPException( - status_code=400, - detail={ - "error": f"user_email '{member.user_email}' and user_id '{member.user_id}' do not belong to the same user." - }, - ) - - # Both match, return as is + if await _user_db(prisma_client).find_first(where={"user_email": member.user_email}) is not None: + raise mismatch_error return member # Case 2: Only user_email provided - populate user_id from DB @@ -3420,12 +3431,14 @@ async def team_member_add( data=data, ) - team_member_add_duplication_check( - data=data, - existing_team_row=complete_team_data, - ) + requested_members: Final = _requested_members(data) + _validate_member_request_shape(requested_members) + if isinstance(data.member, Member): + team_member_add_duplication_check( + data=data, + existing_team_row=complete_team_data, + ) - requested_members: Final = tuple(data.member) if isinstance(data.member, list) else (data.member,) caller_supplied_user_ids = frozenset(member.user_id for member in requested_members if member.user_id is not None) existing_user_ids: Final = await _resolve_existing_member_user_ids( members=requested_members, @@ -3451,6 +3464,12 @@ async def team_member_add( prisma_client=prisma_client, ) + _validate_resolved_members_are_distinct(requested_members) + team_member_add_duplication_check( + data=data, + existing_team_row=complete_team_data, + ) + pre_existing_user_ids: Final = _pre_existing_user_ids( members=requested_members, caller_supplied_user_ids=caller_supplied_user_ids, From 05b36887b341db3a5527e69737fd87a50dba2911 Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Thu, 8 Oct 2026 14:40:25 +0900 Subject: [PATCH 08/10] test(teams): cover member_add conflicts end to end against a Prisma fake Rejection cases run through the real endpoint with only the Prisma client faked, assert the exact error contract, and check that no write call was made --- .../test_team_endpoints.py | 303 +++++++++++++----- 1 file changed, 225 insertions(+), 78 deletions(-) diff --git a/tests/unit/proxy/management_endpoints/test_team_endpoints.py b/tests/unit/proxy/management_endpoints/test_team_endpoints.py index 280597598f4..ad9d958442a 100644 --- a/tests/unit/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_team_endpoints.py @@ -1169,61 +1169,63 @@ def test_team_member_add_duplication_check_raises_proxy_exception(): assert exc_info.value.message == "existing-user-id is already a member of this team." -def _team_with_member_sharing_email() -> MagicMock: - team = MagicMock(spec=LiteLLM_TeamTable) - team.team_id = "test-team-123" - team.members_with_roles = [Member(user_id="dup-user-1", user_email="shared@example.com", role="user")] - return team +def _team_with_roster(*members: Member) -> LiteLLM_TeamTable: + return LiteLLM_TeamTable(team_id="test-team-123", members_with_roles=list(members)) + + +_DUP_USER_1: Final = Member(user_id="dup-user-1", user_email="shared@example.com", role="user") +_DUP_USER_2: Final = Member(user_id="dup-user-2", user_email="shared@example.com", role="user") @pytest.mark.parametrize( - ("member", "expected_message"), + ("roster", "member", "expected_message"), [ + ((_DUP_USER_1,), _DUP_USER_1, "dup-user-1 is already a member of this team."), + ((_DUP_USER_1,), Member(user_id="dup-user-1", role="user"), "dup-user-1 is already a member of this team."), ( - Member(user_id="dup-user-1", user_email="shared@example.com", role="user"), - "dup-user-1 is already a member of this team.", - ), - (Member(user_id="dup-user-1", role="user"), "dup-user-1 is already a member of this team."), - ( + (_DUP_USER_1,), Member(user_email="shared@example.com", role="user"), - "shared@example.com is already used by team member dup-user-1.", + "A member of this team already uses shared@example.com.", ), + ((_DUP_USER_1,), _DUP_USER_2, "A member of this team already uses shared@example.com."), + ((_DUP_USER_1, _DUP_USER_2), _DUP_USER_2, "dup-user-2 is already a member of this team."), + ((_DUP_USER_2, _DUP_USER_1), _DUP_USER_2, "dup-user-2 is already a member of this team."), ( - Member(user_id="dup-user-2", user_email="shared@example.com", role="user"), - "shared@example.com is already used by team member dup-user-1.", + (Member(user_email="invited@example.com", role="user"),), + Member(user_email="invited@example.com", role="user"), + "A member of this team already uses invited@example.com.", ), ], - ids=["id_and_email", "id_only", "email_only", "other_user_same_email"], + ids=[ + "id_and_email", + "id_only", + "email_only", + "other_user_same_email", + "same_id_after_shared_email_member", + "same_id_before_shared_email_member", + "email_only_roster_entry", + ], ) -def test_team_member_add_duplication_check_rejects_member_already_in_team(member: Member, expected_message: str): - data = TeamMemberAddRequest(team_id="test-team-123", member=member) +def test_team_member_add_duplication_check_names_only_the_requested_member( + roster: tuple[Member, ...], member: Member, expected_message: str +): + data: Final = TeamMemberAddRequest(team_id="test-team-123", member=member) with pytest.raises(ProxyException) as exc_info: - team_member_add_duplication_check(data=data, existing_team_row=_team_with_member_sharing_email()) + team_member_add_duplication_check(data=data, existing_team_row=_team_with_roster(*roster)) assert exc_info.value.type == ProxyErrorTypes.team_member_already_in_team assert exc_info.value.message == expected_message -def test_team_member_add_duplication_check_names_email_only_member_by_email(): - team = MagicMock(spec=LiteLLM_TeamTable) - team.members_with_roles = [Member(user_email="invited@example.com", role="user")] - data = TeamMemberAddRequest(team_id="test-team-123", member=Member(user_email="invited@example.com", role="user")) - - with pytest.raises(ProxyException) as exc_info: - team_member_add_duplication_check(data=data, existing_team_row=team) - - assert exc_info.value.message == "invited@example.com is already a member of this team." - - def test_team_member_add_duplication_check_bulk_message_omits_roster(): - data = TeamMemberAddRequest( + data: Final = TeamMemberAddRequest( team_id="test-team-123", member=[Member(user_id="dup-user-1", role="user"), Member(user_email="shared@example.com", role="user")], ) with pytest.raises(ProxyException) as exc_info: - team_member_add_duplication_check(data=data, existing_team_row=_team_with_member_sharing_email()) + team_member_add_duplication_check(data=data, existing_team_row=_team_with_roster(_DUP_USER_1)) assert exc_info.value.message == "All requested users are already members of this team." @@ -10681,70 +10683,91 @@ async def test_get_team_daily_activity_team_admin_sees_all_spend(mock_db_client) pytest.fail("API keys should not be fetched for team admin users") -@pytest.mark.asyncio -async def test_validate_and_populate_member_user_info_both_provided_match(): - """ - Test _validate_and_populate_member_user_info when both user_email and user_id - are provided and they match the same user in the database. - """ - # Create member with both user_email and user_id - member = Member(user_email="test@example.com", user_id="user-123", role="user") +def _first_email_owner(rows: tuple[MagicMock, ...], where: dict[str, object]) -> MagicMock | None: + criterion: Final = where["user_email"] + if isinstance(criterion, dict): + return next((row for row in rows if (row.user_email or "").lower() == criterion["equals"].lower()), None) + return next((row for row in rows if row.user_email == criterion), None) - # Mock prisma client - mock_prisma_client = MagicMock() - # Mock user object that matches both email and user_id - mock_user = MagicMock() - mock_user.user_id = "user-123" - mock_user.user_email = "test@example.com" - - # Mock get_data to return single user matching email - mock_prisma_client.get_data = AsyncMock(return_value=[mock_user]) - - # Call the function - result = await _validate_and_populate_member_user_info( - member=member, - prisma_client=mock_prisma_client, - ) - - # Verify result matches input (both already provided and match) - assert result.user_email == "test@example.com" - assert result.user_id == "user-123" - - # Verify get_data was called with correct parameters - mock_prisma_client.get_data.assert_called_once_with( - key_val={"user_email": "test@example.com"}, - table_name="user", - query_type="find_all", +def _prisma_with_users(*users: tuple[str, str | None], team: LiteLLM_TeamTable | None = None) -> MagicMock: + rows: Final = tuple(MagicMock(user_id=user_id, user_email=user_email) for user_id, user_email in users) + return MagicMock( + **{ + "db.litellm_usertable.find_unique": AsyncMock( + side_effect=lambda where: next((row for row in rows if row.user_id == where["user_id"]), None) + ), + "db.litellm_usertable.find_first": AsyncMock(side_effect=lambda where: _first_email_owner(rows, where)), + "db.litellm_usertable.find_many": AsyncMock( + side_effect=lambda where: [row for row in rows if row.user_id in where["user_id"]["in"]] + ), + "get_data": AsyncMock( + side_effect=lambda key_val, table_name, query_type: [ + row for row in rows if row.user_email == key_val["user_email"] + ] + ), + "writer_db.litellm_teamtable.find_unique": AsyncMock(return_value=team), + } ) -def _users_sharing_email(email: str, *user_ids: str) -> list[MagicMock]: - return [MagicMock(user_id=user_id, user_email=email) for user_id in user_ids] +_USER_DIRECTORY: Final = ( + ("dup-user-1", "shared@example.com"), + ("dup-user-2", "shared@example.com"), + ("solo-user", "solo@example.com"), + ("no-email-user", None), +) @pytest.mark.asyncio -async def test_validate_and_populate_member_user_info_duplicate_email_accepts_matching_user_id(): - member = Member(user_email="dup@example.com", user_id="user-b", role="user") - mock_prisma_client = MagicMock() - mock_prisma_client.get_data = AsyncMock(return_value=_users_sharing_email("dup@example.com", "user-a", "user-b")) +@pytest.mark.parametrize( + ("user_id", "user_email"), + [ + ("dup-user-2", "shared@example.com"), + ("solo-user", "solo@example.com"), + ("brand-new-user", "unused@example.com"), + ("no-email-user", "first-email@example.com"), + ], + ids=["one_of_shared_email_owners", "single_owner", "new_user_with_unused_email", "existing_user_without_email"], +) +async def test_validate_and_populate_member_user_info_accepts_owner_of_the_email(user_id: str, user_email: str): + member: Final = Member(user_id=user_id, user_email=user_email, role="user") - result = await _validate_and_populate_member_user_info(member=member, prisma_client=mock_prisma_client) + result: Final = await _validate_and_populate_member_user_info( + member=member, prisma_client=_prisma_with_users(*_USER_DIRECTORY) + ) - assert (result.user_id, result.user_email) == ("user-b", "dup@example.com") + assert (result.user_id, result.user_email) == (user_id, user_email) @pytest.mark.asyncio -async def test_validate_and_populate_member_user_info_duplicate_email_rejects_unrelated_user_id(): - member = Member(user_email="dup@example.com", user_id="user-c", role="user") - mock_prisma_client = MagicMock() - mock_prisma_client.get_data = AsyncMock(return_value=_users_sharing_email("dup@example.com", "user-a", "user-b")) +@pytest.mark.parametrize( + ("user_id", "user_email"), + [ + ("brand-new-user", "shared@example.com"), + ("solo-user", "shared@example.com"), + ("dup-user-1", "unused@example.com"), + ("brand-new-user", "solo@example.com"), + ], + ids=[ + "new_user_id_with_shared_email", + "existing_user_id_with_shared_email", + "existing_user_id_with_unused_email", + "new_user_id_with_taken_email", + ], +) +async def test_validate_and_populate_member_user_info_rejects_non_owner_of_the_email( + user_id: str, user_email: str +): + member: Final = Member(user_id=user_id, user_email=user_email, role="user") with pytest.raises(HTTPException) as exc_info: - await _validate_and_populate_member_user_info(member=member, prisma_client=mock_prisma_client) + await _validate_and_populate_member_user_info(member=member, prisma_client=_prisma_with_users(*_USER_DIRECTORY)) assert exc_info.value.status_code == 400 - assert "Multiple users found with email 'dup@example.com'" in str(exc_info.value.detail) + assert exc_info.value.detail == { + "error": f"user_email '{user_email}' and user_id '{user_id}' do not belong to the same user." + } @pytest.mark.asyncio @@ -13778,6 +13801,130 @@ async def test_team_member_add_rejects_non_admin_before_revealing_existing_membe assert "dup-user-1" not in str(exc_info.value.detail) +_WRITE_METHODS: Final = frozenset({"create", "create_many", "upsert", "update", "update_many", "delete", "tx"}) +_MEMBER_ADD_USERS: Final = ( + ("dup-user-1", "shared@example.com"), + ("dup-user-2", "shared@example.com"), + ("dup-user-3", "shared@example.com"), + ("solo-user", "solo@example.com"), +) + + +def _write_calls(prisma: MagicMock) -> tuple[str, ...]: + return tuple(name for name, _, _ in prisma.mock_calls if name.rsplit(".", 1)[-1] in _WRITE_METHODS) + + +def _rejection(error: Exception) -> tuple[str, int | str, str]: + if isinstance(error, ProxyException): + assert (error.type, error.param) == (ProxyErrorTypes.team_member_already_in_team, "member"), error + return ("already_in_team", error.code, error.message) + assert isinstance(error, HTTPException), error + return ("http", error.status_code, error.detail["error"]) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("roster", "member", "expected"), + [ + ( + (_DUP_USER_1,), + Member(user_id="dup-user-2", role="user"), + ("already_in_team", "400", "A member of this team already uses shared@example.com."), + ), + ( + (_DUP_USER_1, _DUP_USER_2), + _DUP_USER_2, + ("already_in_team", "400", "dup-user-2 is already a member of this team."), + ), + ( + (), + Member(user_email="shared@example.com", role="user"), + ("http", 400, "Multiple users found with email 'shared@example.com'. Please use 'user_id' instead."), + ), + ( + (), + Member(user_id="solo-user", user_email="shared@example.com", role="user"), + ("http", 400, "user_email 'shared@example.com' and user_id 'solo-user' do not belong to the same user."), + ), + ((), [], ("http", 400, "At least one member is required.")), + ( + (), + [_DUP_USER_2, Member(user_id="dup-user-3", user_email="shared@example.com", role="user")], + ("http", 400, "Several requested users share the email shared@example.com. Add only one of them."), + ), + ( + (), + [Member(user_id="dup-user-2", role="user"), Member(user_id="dup-user-3", role="user")], + ("http", 400, "Several requested users share the email shared@example.com. Add only one of them."), + ), + ( + (), + [Member(user_id="solo-user", role="user"), Member(user_id="solo-user", role="admin")], + ("http", 400, "solo-user appears more than once in this request."), + ), + ( + (), + [ + Member(user_id="solo-user", user_email="solo@example.com", role="user"), + Member(user_email="solo@example.com", role="user"), + ], + ("http", 400, "solo-user appears more than once in this request."), + ), + ( + (Member(user_id="solo-user", user_email="solo@example.com", role="user"),), + [Member(user_id="solo-user", role="user"), Member(user_email="solo@example.com", role="user")], + ("http", 400, "solo-user appears more than once in this request."), + ), + ( + (Member(user_id="solo-user", role="user"),), + [Member(user_email="solo@example.com", role="user")], + ("already_in_team", "400", "All requested users are already members of this team."), + ), + ], + ids=[ + "row11_user_id_whose_email_a_member_uses", + "row8_same_user_already_in_team_after_shared_email_member", + "row4_email_with_several_owners", + "row6_user_id_that_does_not_own_the_email", + "row15_empty_list", + "row18_two_users_sharing_an_email", + "row18_two_user_ids_that_resolve_to_one_email", + "row19_same_user_id_twice", + "row19_same_user_by_id_and_email_then_email", + "row19_member_already_in_team_by_id_and_by_email", + "row16_all_members_already_in_team_after_resolution", + ], +) +async def test_team_member_add_rejects_without_writing( + monkeypatch, + roster: tuple[Member, ...], + member: Member | list[Member], + expected: tuple[str, int | str, str], +): + from litellm.proxy._types import TeamMemberAddRequest + from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache + from litellm.proxy.management_endpoints.team_endpoints import team_member_add + + team_id: Final = f"team-{uuid.uuid4().hex}" + prisma: Final = _prisma_with_users( + *_MEMBER_ADD_USERS, team=LiteLLM_TeamTable(team_id=team_id, members_with_roles=list(roster)) + ) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", prisma) + monkeypatch.setattr("litellm.proxy.proxy_server.user_api_key_cache", UserApiKeyCache()) + monkeypatch.setattr("litellm.proxy.proxy_server.proxy_logging_obj", None) + monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", True) + monkeypatch.setattr("litellm.proxy.proxy_server.litellm_proxy_admin_name", "default_user_id") + + with pytest.raises((HTTPException, ProxyException)) as exc_info: + await team_member_add( + data=TeamMemberAddRequest(team_id=team_id, member=member), + user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN, user_id="admin-1"), + ) + + assert _rejection(exc_info.value) == expected + assert _write_calls(prisma) == () + + class _RecordingAuditLogger(CustomLogger): def __init__(self) -> None: super().__init__() From 6d6537266deac04be3464a94fc39fd30df288a80 Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Thu, 8 Oct 2026 14:40:26 +0900 Subject: [PATCH 09/10] fix(ui): only show recognized string messages from a failed member_add Unknown error shapes, arrays, non-string or blank messages fall back to "Failed to add team member" instead of raw JSON or [object Object] --- .../src/components/networking.test.ts | 25 +++++++++++++------ .../src/components/networking.tsx | 21 +++++++++++++--- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/ui/litellm-dashboard/src/components/networking.test.ts b/ui/litellm-dashboard/src/components/networking.test.ts index 2f4a0d2223f..a9ddd640ac7 100644 --- a/ui/litellm-dashboard/src/components/networking.test.ts +++ b/ui/litellm-dashboard/src/components/networking.test.ts @@ -119,37 +119,46 @@ describe("loginCall - storeLoginToken integration", () => { }); describe("teamMemberAddCall error message", () => { - const originalFetch = global.fetch; - afterEach(() => { - global.fetch = originalFetch; + vi.restoreAllMocks(); }); const rejectWith = (body: string) => { - global.fetch = vi.fn().mockResolvedValue({ ok: false, text: async () => body }) as any; + vi.spyOn(global, "fetch").mockResolvedValue(new Response(body, { status: 400 })); }; const member: Networking.Member = { user_id: "dup-user-1", role: "user" }; + const addMember = () => Networking.teamMemberAddCall("token", "team-1", member); it.each([ ["the proxy error envelope", { error: { message: "dup-user-1 is already a member of this team." } }], ["an HTTPException detail", { detail: { error: "dup-user-1 is already a member of this team." } }], + ["a plain message field", { message: "dup-user-1 is already a member of this team." }], + ["a plain error string", { error: "dup-user-1 is already a member of this team." }], + ["a plain detail string", { detail: "dup-user-1 is already a member of this team." }], + [ + "a lower-priority field when a higher one is not a string", + { error: { message: 5 }, detail: { error: "dup-user-1 is already a member of this team." } }, + ], ])("surfaces the backend message from %s", async (_shape, body) => { rejectWith(JSON.stringify(body)); - await expect(Networking.teamMemberAddCall("token", "team-1", member)).rejects.toThrow( - "dup-user-1 is already a member of this team.", - ); + await expect(addMember()).rejects.toHaveProperty("message", "dup-user-1 is already a member of this team."); }); it.each([ ["is not JSON", "bad gateway"], ["is JSON null", "null"], ["is an empty object", "{}"], + ["is an object without a known message field", JSON.stringify({ status: 502 })], + ["is an array", JSON.stringify(["unknown"])], + ["has a non-string message", JSON.stringify({ message: { reason: "unknown" } })], + ["has an empty error message", JSON.stringify({ error: { message: "" } })], + ["has a whitespace-only message", JSON.stringify({ message: " " })], ])("falls back to a generic message when the body %s", async (_case, body) => { rejectWith(body); - await expect(Networking.teamMemberAddCall("token", "team-1", member)).rejects.toThrow("Failed to add team member"); + await expect(addMember()).rejects.toHaveProperty("message", "Failed to add team member"); }); }); diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 1a95b1afcbc..1479f7a7092 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -2740,6 +2740,22 @@ export interface Member { temp_budget_expiry?: string | null; } +const nonEmptyString = (value: unknown): string | undefined => + typeof value === "string" && value.trim() !== "" ? value : undefined; + +const objectField = (value: unknown, key: string): unknown => + value !== null && typeof value === "object" && !Array.isArray(value) + ? (value as Record)[key] + : undefined; + +const teamMemberAddErrorMessage = (body: unknown): string => + nonEmptyString(objectField(objectField(body, "error"), "message")) ?? + nonEmptyString(objectField(body, "error")) ?? + nonEmptyString(objectField(body, "message")) ?? + nonEmptyString(objectField(body, "detail")) ?? + nonEmptyString(objectField(objectField(body, "detail"), "error")) ?? + "Failed to add team member"; + export const teamMemberAddCall = async (accessToken: string, teamId: string, formValues: Member) => { try { const url = proxyBaseUrl ? `${proxyBaseUrl}/team/member_add` : `/team/member_add`; @@ -2767,10 +2783,7 @@ export const teamMemberAddCall = async (accessToken: string, teamId: string, for console.warn("Failed to parse error body as JSON:", errorText); } - const hasErrorBody = - parsedError !== null && typeof parsedError === "object" && Object.keys(parsedError).length > 0; - const rawMessage = hasErrorBody ? deriveErrorMessage(parsedError) : "Failed to add team member"; - const err = new Error(rawMessage); + const err = new Error(teamMemberAddErrorMessage(parsedError)); (err as any).raw = parsedError; throw err; } From 3c84b4ddb24ed00c186f210894fa934b265c3775 Mon Sep 17 00:00:00 2001 From: okhs0712 Date: Thu, 8 Oct 2026 15:37:44 +0900 Subject: [PATCH 10/10] fix(teams): stop a user without an email from claiming another user's email An existing user with no email on record could be added with any user_email, so a self-join could store someone else's email on the roster entry and block that user's own join through the email duplicate check. The supplied email is now rejected when another user already owns it, the same check a new user_id gets --- .../management_endpoints/team_endpoints.py | 6 +-- .../test_team_endpoints.py | 38 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index dcb8442f6b5..fb68e6da428 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -3282,7 +3282,7 @@ async def _validate_and_populate_member_user_info( Logic: 1. If both user_email and user_id are provided, the user_id decides: an existing user with an email on record - must have that email, and a new user_id must not take an email another user already has + must have that email, and any other user_id must not take an email another user already has 2. If only user_email is provided, populate user_id from DB 3. If only user_id is provided, populate user_email from DB (if user exists) 4. If only user_id is provided and doesn't exist, allow it to pass with user_email as None (will be upserted later) @@ -3306,8 +3306,8 @@ async def _validate_and_populate_member_user_info( }, ) existing_user: Final = await _user_db(prisma_client).find_unique(where={"user_id": member.user_id}) - if existing_user is not None: - if existing_user.user_email is not None and existing_user.user_email != member.user_email: + if existing_user is not None and existing_user.user_email is not None: + if existing_user.user_email != member.user_email: raise mismatch_error return member diff --git a/tests/unit/proxy/management_endpoints/test_team_endpoints.py b/tests/unit/proxy/management_endpoints/test_team_endpoints.py index ad9d958442a..b1750215b94 100644 --- a/tests/unit/proxy/management_endpoints/test_team_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_team_endpoints.py @@ -10748,12 +10748,14 @@ async def test_validate_and_populate_member_user_info_accepts_owner_of_the_email ("solo-user", "shared@example.com"), ("dup-user-1", "unused@example.com"), ("brand-new-user", "solo@example.com"), + ("no-email-user", "solo@example.com"), ], ids=[ "new_user_id_with_shared_email", "existing_user_id_with_shared_email", "existing_user_id_with_unused_email", "new_user_id_with_taken_email", + "existing_user_without_email_with_taken_email", ], ) async def test_validate_and_populate_member_user_info_rejects_non_owner_of_the_email( @@ -13925,6 +13927,42 @@ async def test_team_member_add_rejects_without_writing( assert _write_calls(prisma) == () +@pytest.mark.asyncio +async def test_team_member_add_self_join_cannot_claim_another_users_email(monkeypatch): + from litellm.proxy._types import TeamMemberAddRequest + from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache + from litellm.proxy.management_endpoints.team_endpoints import team_member_add + + team_id: Final = f"team-{uuid.uuid4().hex}" + prisma: Final = _prisma_with_users( + ("no-email-user", None), + ("victim-user", "victim@example.com"), + team=LiteLLM_TeamTable(team_id=team_id, members_with_roles=[]), + ) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", prisma) + monkeypatch.setattr("litellm.proxy.proxy_server.user_api_key_cache", UserApiKeyCache()) + monkeypatch.setattr("litellm.proxy.proxy_server.proxy_logging_obj", None) + monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", True) + monkeypatch.setattr("litellm.proxy.proxy_server.litellm_proxy_admin_name", "default_user_id") + monkeypatch.setattr("litellm.default_internal_user_params", {"available_teams": [team_id]}) + + with pytest.raises((HTTPException, ProxyException)) as exc_info: + await team_member_add( + data=TeamMemberAddRequest( + team_id=team_id, + member=Member(user_id="no-email-user", user_email="victim@example.com", role="user"), + ), + user_api_key_dict=UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id="no-email-user"), + ) + + assert _rejection(exc_info.value) == ( + "http", + 400, + "user_email 'victim@example.com' and user_id 'no-email-user' do not belong to the same user.", + ) + assert _write_calls(prisma) == () + + class _RecordingAuditLogger(CustomLogger): def __init__(self) -> None: super().__init__()