mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
fix(ui): let the Create Key user picker find users by user_id, not just email (#41687)
* feat(ui): search users by id or email when assigning a key owner Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): label users without an email by user id in key owner picker Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(proxy): freeze merged user-filter where, format create key test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(proxy): suppress module-global patch findings in ui_view_users search test Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(proxy): keep merged user-filter where as a plain dict for prisma serialization Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): forward search param from userFilterUICall to /user/filter/ui Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): mention user ID in the Create Key user picker helper text Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: jesus <jesus@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
devin-ai-integration[bot]
GitHub
parent
dd327156c8
commit
da82ea8e94
7 changed files with 148 additions and 23 deletions
|
|
@ -2741,6 +2741,10 @@ async def _resolve_team_org_filter(
|
|||
async def ui_view_users(
|
||||
user_id: str | None = fastapi.Query(default=None, description="User ID in the request parameters"),
|
||||
user_email: str | None = fastapi.Query(default=None, description="User email in the request parameters"),
|
||||
search: str | None = fastapi.Query(
|
||||
default=None,
|
||||
description="Combined search: matches users whose 'user_id' or 'user_email' contains the value (case-insensitive).",
|
||||
),
|
||||
team_id: str | None = fastapi.Query(
|
||||
default=None,
|
||||
description="Team ID — used when a team admin searches for users to add to their team",
|
||||
|
|
@ -2750,7 +2754,7 @@ async def ui_view_users(
|
|||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
||||
):
|
||||
"""
|
||||
Filter users based on partial match of user_id or email with pagination.
|
||||
Filter users based on partial match of user_id or email, or combined ``search``, with pagination.
|
||||
|
||||
Behaviour depends on the ``scope_user_search_to_org`` UI-setting flag
|
||||
(stored in the ``litellm_uisettings`` table):
|
||||
|
|
@ -2802,9 +2806,15 @@ async def ui_view_users(
|
|||
if org_filter_ids is not None:
|
||||
where_conditions["organization_memberships"] = {"some": {"organization_id": {"in": org_filter_ids}}}
|
||||
|
||||
where: Final[Mapping[str, object]] = { # mutable-ok: prisma serializes `where`, keep it a plain dict
|
||||
key: value
|
||||
for key, value in (*where_conditions.items(), *_user_search_where(search).items())
|
||||
if value is not None
|
||||
}
|
||||
|
||||
# Query users with pagination and filters
|
||||
users: Final = await _user_table(prisma_client).find_many(
|
||||
where=where_conditions,
|
||||
where=where,
|
||||
skip=skip,
|
||||
take=page_size,
|
||||
order={"created_at": "desc"},
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import asyncio
|
|||
import hashlib
|
||||
import json
|
||||
import logging
|
||||
from collections.abc import Mapping, Sequence
|
||||
from datetime import datetime, timezone
|
||||
from types import SimpleNamespace
|
||||
from typing import Final
|
||||
|
|
@ -37,6 +38,7 @@ from litellm.proxy.management_endpoints.internal_user_endpoints import (
|
|||
ui_view_users,
|
||||
)
|
||||
from litellm.proxy.proxy_server import app
|
||||
from litellm.types.proxy.management_endpoints.internal_user_endpoints import InsensitiveContains
|
||||
from tests.test_litellm.proxy.management_endpoints.jwt_key_mapping_doubles import (
|
||||
CascadingJWTMappingTable,
|
||||
JWTMappingRow,
|
||||
|
|
@ -119,6 +121,70 @@ async def test_ui_view_users_proxy_admin_no_org_filter(mocker):
|
|||
)
|
||||
|
||||
|
||||
UserWhereCondition = InsensitiveContains | Sequence[Mapping[str, InsensitiveContains]]
|
||||
|
||||
|
||||
def _matches_user_where(row: LiteLLM_UserTableFiltered, where: Mapping[str, UserWhereCondition]) -> bool:
|
||||
def matches(field: str, condition: UserWhereCondition) -> bool:
|
||||
if not isinstance(condition, Mapping):
|
||||
return any(_matches_user_where(row, branch) for branch in condition)
|
||||
value: Final = {"user_id": row.user_id, "user_email": row.user_email}[field]
|
||||
return value is not None and condition["contains"].lower() in value.lower()
|
||||
|
||||
return all(matches(field, condition) for field, condition in where.items())
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"params, expected_user_ids",
|
||||
[
|
||||
({"search": "SVC"}, ["svc-bot"]),
|
||||
({"search": "ali"}, ["alice-admin"]),
|
||||
({"search": "example.com"}, ["alice-admin"]),
|
||||
({"search": "admin"}, ["alice-admin"]),
|
||||
({"user_email": "svc"}, []),
|
||||
({"user_id": "svc"}, ["svc-bot"]),
|
||||
({"search": "ali", "user_id": "svc"}, []),
|
||||
],
|
||||
)
|
||||
def test_ui_view_users_search_matches_user_id_or_email(
|
||||
mocker: MockerFixture, params: Mapping[str, str], expected_user_ids: list[str]
|
||||
):
|
||||
"""
|
||||
search= returns users whose user_id or user_email contains the value (case-insensitive),
|
||||
including users with no email; user_id=/user_email= keep filtering a single field and AND with search.
|
||||
"""
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
|
||||
users = (
|
||||
LiteLLM_UserTableFiltered(user_id="alice-admin", user_email="alice@example.com"),
|
||||
LiteLLM_UserTableFiltered(user_id="svc-bot", user_email=None),
|
||||
LiteLLM_UserTableFiltered(user_id="bob", user_email="bob@corp.io"),
|
||||
)
|
||||
|
||||
async def mock_find_many(*, where: Mapping[str, UserWhereCondition], **_: object):
|
||||
return [user for user in users if _matches_user_where(user, where)]
|
||||
|
||||
mock_prisma_client = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_usertable.find_many = mock_find_many
|
||||
mocker.patch( # test-quality-ok: endpoint reads settings via module global; same seam as sibling tests
|
||||
"litellm.proxy.ui_crud_endpoints.proxy_setting_endpoints.get_ui_settings_cached",
|
||||
return_value={},
|
||||
)
|
||||
mocker.patch( # test-quality-ok: endpoint reads prisma_client via module global; same seam as sibling tests
|
||||
"litellm.proxy.proxy_server.prisma_client", mock_prisma_client
|
||||
)
|
||||
app.dependency_overrides[user_api_key_auth] = lambda: UserAPIKeyAuth(
|
||||
user_id="admin", user_role=LitellmUserRoles.PROXY_ADMIN
|
||||
)
|
||||
try:
|
||||
response = client.get("/user/filter/ui", params=params)
|
||||
finally:
|
||||
app.dependency_overrides.pop(user_api_key_auth, None)
|
||||
|
||||
assert response.status_code == 200, response.text
|
||||
assert [user["user_id"] for user in response.json()] == expected_user_ids
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ui_view_users_org_admin_filtered_by_org(mocker):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -913,3 +913,26 @@ describe("fetchMemoryList search serialization", () => {
|
|||
expect(lastParams(mockFetch).has("search")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("userFilterUICall", () => {
|
||||
let currentFetch: typeof global.fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
currentFetch = global.fetch;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
global.fetch = currentFetch;
|
||||
});
|
||||
|
||||
it("forwards the search param to /user/filter/ui", async () => {
|
||||
const mockFetch = vi.fn().mockResolvedValue({ ok: true, text: async () => "[]" } as any);
|
||||
global.fetch = mockFetch as any;
|
||||
|
||||
await Networking.userFilterUICall("sk-test", new URLSearchParams({ search: "svc" }));
|
||||
|
||||
const parsed = new URL(mockFetch.mock.calls[0][0] as string, "http://localhost");
|
||||
expect(parsed.pathname).toContain("/user/filter/ui");
|
||||
expect(parsed.searchParams.get("search")).toBe("svc");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1983,6 +1983,7 @@ export const userFilterUICall = async (accessToken: string, params: URLSearchPar
|
|||
user_email: params.get("user_email") || undefined,
|
||||
user_id: params.get("user_id") || undefined,
|
||||
team_id: params.get("team_id") || undefined,
|
||||
search: params.get("search") || undefined,
|
||||
},
|
||||
});
|
||||
} catch (error) {
|
||||
|
|
|
|||
|
|
@ -204,7 +204,8 @@ const openModal = async (props: Partial<React.ComponentProps<typeof CreateKey>>
|
|||
return view;
|
||||
};
|
||||
|
||||
const userSearchInput = (): Promise<HTMLElement> => screen.findByPlaceholderText("Type email to search for users");
|
||||
const userSearchInput = (): Promise<HTMLElement> =>
|
||||
screen.findByPlaceholderText("Type email or user ID to search for users");
|
||||
|
||||
const openSection = async (name: RegExp) => {
|
||||
await userEvent.click(await screen.findByRole("button", { name }));
|
||||
|
|
@ -619,7 +620,7 @@ describe("CreateKey", () => {
|
|||
|
||||
it("mounts the user search control only once Another User is chosen", async () => {
|
||||
await openModal();
|
||||
expect(screen.queryByPlaceholderText("Type email to search for users")).not.toBeInTheDocument();
|
||||
expect(screen.queryByPlaceholderText("Type email or user ID to search for users")).not.toBeInTheDocument();
|
||||
|
||||
await userEvent.click(screen.getByRole("radio", { name: "Another User" }));
|
||||
|
||||
|
|
@ -946,18 +947,18 @@ describe("CreateKey", () => {
|
|||
|
||||
expect(vi.mocked(userFilterUICall)).toHaveBeenCalledTimes(1);
|
||||
const params = vi.mocked(userFilterUICall).mock.calls[0][1] as URLSearchParams;
|
||||
expect(params.get("user_email")).toBe("alice");
|
||||
expect(params.get("search")).toBe("alice");
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps the current search's users when an abandoned search answers last", async () => {
|
||||
const answers = new Map<string, (users: { user_id: string; user_email: string }[]) => void>();
|
||||
const answers = new Map<string, (users: { user_id: string; user_email: string | null }[]) => void>();
|
||||
vi.mocked(userFilterUICall).mockImplementation(
|
||||
(_accessToken, params) =>
|
||||
new Promise((resolve) => {
|
||||
answers.set(params.get("user_email") ?? "", resolve);
|
||||
answers.set(params.get("search") ?? "", resolve);
|
||||
}) as never,
|
||||
);
|
||||
|
||||
|
|
@ -984,12 +985,36 @@ describe("CreateKey", () => {
|
|||
expect(screen.getByRole("option", { name: "alice.smith@example.com (u-smith)" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("stops searching once the box is cleared and the abandoned search answers", async () => {
|
||||
const answers = new Map<string, (users: { user_id: string; user_email: string }[]) => void>();
|
||||
it("labels a user with no email by their user id", async () => {
|
||||
const answers = new Map<string, (users: { user_id: string; user_email: string | null }[]) => void>();
|
||||
vi.mocked(userFilterUICall).mockImplementation(
|
||||
(_accessToken, params) =>
|
||||
new Promise((resolve) => {
|
||||
answers.set(params.get("user_email") ?? "", resolve);
|
||||
answers.set(params.get("search") ?? "", resolve);
|
||||
}) as never,
|
||||
);
|
||||
|
||||
const user = userEvent.setup();
|
||||
renderCreateKey({ autoOpenCreate: true, prefillData: { owned_by: "another_user" } });
|
||||
const search = await userSearchInput();
|
||||
|
||||
await user.type(search, "svc");
|
||||
await waitFor(() => expect(answers.has("svc")).toBe(true), { timeout: 3000 });
|
||||
|
||||
await act(async () => {
|
||||
answers.get("svc")?.([{ user_id: "svc-bot", user_email: null }]);
|
||||
});
|
||||
|
||||
expect(await screen.findByRole("option", { name: "svc-bot" })).toBeInTheDocument();
|
||||
expect(screen.queryByRole("option", { name: /null/ })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("stops searching once the box is cleared and the abandoned search answers", async () => {
|
||||
const answers = new Map<string, (users: { user_id: string; user_email: string | null }[]) => void>();
|
||||
vi.mocked(userFilterUICall).mockImplementation(
|
||||
(_accessToken, params) =>
|
||||
new Promise((resolve) => {
|
||||
answers.set(params.get("search") ?? "", resolve);
|
||||
}) as never,
|
||||
);
|
||||
|
||||
|
|
@ -1013,11 +1038,11 @@ describe("CreateKey", () => {
|
|||
});
|
||||
|
||||
it("keeps searching while a newer search is still in flight", async () => {
|
||||
const answers = new Map<string, (users: { user_id: string; user_email: string }[]) => void>();
|
||||
const answers = new Map<string, (users: { user_id: string; user_email: string | null }[]) => void>();
|
||||
vi.mocked(userFilterUICall).mockImplementation(
|
||||
(_accessToken, params) =>
|
||||
new Promise((resolve) => {
|
||||
answers.set(params.get("user_email") ?? "", resolve);
|
||||
answers.set(params.get("search") ?? "", resolve);
|
||||
}) as never,
|
||||
);
|
||||
|
||||
|
|
@ -1047,12 +1072,12 @@ describe("CreateKey", () => {
|
|||
it("only warns about a failed search when it is the one the box is waiting on", async () => {
|
||||
const answers = new Map<
|
||||
string,
|
||||
{ resolve: (users: { user_id: string; user_email: string }[]) => void; reject: (error: Error) => void }
|
||||
{ resolve: (users: { user_id: string; user_email: string | null }[]) => void; reject: (error: Error) => void }
|
||||
>();
|
||||
vi.mocked(userFilterUICall).mockImplementation(
|
||||
(_accessToken, params) =>
|
||||
new Promise((resolve, reject) => {
|
||||
answers.set(params.get("user_email") ?? "", { resolve, reject });
|
||||
answers.set(params.get("search") ?? "", { resolve, reject });
|
||||
}) as never,
|
||||
);
|
||||
|
||||
|
|
@ -1099,9 +1124,7 @@ describe("CreateKey", () => {
|
|||
];
|
||||
vi.mocked(userFilterUICall).mockImplementation(
|
||||
(_accessToken, params) =>
|
||||
Promise.resolve(
|
||||
directory.filter((entry) => entry.user_email.includes(params.get("user_email") ?? "")),
|
||||
) as never,
|
||||
Promise.resolve(directory.filter((entry) => entry.user_email.includes(params.get("search") ?? ""))) as never,
|
||||
);
|
||||
|
||||
const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime });
|
||||
|
|
|
|||
|
|
@ -161,7 +161,7 @@ interface CreateKeyProps {
|
|||
|
||||
interface User {
|
||||
user_id: string;
|
||||
user_email: string;
|
||||
user_email: string | null;
|
||||
role?: string;
|
||||
}
|
||||
|
||||
|
|
@ -570,7 +570,7 @@ const CreateKey: React.FC<CreateKeyProps> = ({ team, teams, data, addKey, autoOp
|
|||
setUserSearchLoading(true);
|
||||
try {
|
||||
const params = new URLSearchParams();
|
||||
params.append("user_email", searchText); // Always search by email
|
||||
params.append("search", searchText);
|
||||
if (accessToken == null) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -579,7 +579,7 @@ const CreateKey: React.FC<CreateKeyProps> = ({ team, teams, data, addKey, autoOp
|
|||
|
||||
const data: User[] = response;
|
||||
const options: SearchSelectOption[] = data.map((user) => ({
|
||||
label: `${user.user_email} (${user.user_id})`,
|
||||
label: user.user_email ? `${user.user_email} (${user.user_id})` : user.user_id,
|
||||
value: user.user_id,
|
||||
}));
|
||||
|
||||
|
|
@ -729,7 +729,7 @@ const CreateKey: React.FC<CreateKeyProps> = ({ team, teams, data, addKey, autoOp
|
|||
onValueChange={control.onChange}
|
||||
onSearchChange={fetchUsers}
|
||||
isLoading={userSearchLoading}
|
||||
placeholder="Type email to search for users"
|
||||
placeholder="Type email or user ID to search for users"
|
||||
emptyText="No users found"
|
||||
loadingText="Searching..."
|
||||
inputId={control.id}
|
||||
|
|
@ -741,7 +741,7 @@ const CreateKey: React.FC<CreateKeyProps> = ({ team, teams, data, addKey, autoOp
|
|||
Create User
|
||||
</Button>
|
||||
</div>
|
||||
<div className="text-xs text-muted-foreground">Search by email to find users</div>
|
||||
<div className="text-xs text-muted-foreground">Search by email or user ID to find users</div>
|
||||
</div>
|
||||
)}
|
||||
</MountedFormField>
|
||||
|
|
|
|||
4
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
4
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -17325,7 +17325,7 @@ export interface paths {
|
|||
};
|
||||
/**
|
||||
* Ui View Users
|
||||
* @description Filter users based on partial match of user_id or email with pagination.
|
||||
* @description Filter users based on partial match of user_id or email, or combined ``search``, with pagination.
|
||||
*
|
||||
* Behaviour depends on the ``scope_user_search_to_org`` UI-setting flag
|
||||
* (stored in the ``litellm_uisettings`` table):
|
||||
|
|
@ -64338,6 +64338,8 @@ export interface operations {
|
|||
user_id?: string | null;
|
||||
/** @description User email in the request parameters */
|
||||
user_email?: string | null;
|
||||
/** @description Combined search: matches users whose 'user_id' or 'user_email' contains the value (case-insensitive). */
|
||||
search?: string | null;
|
||||
/** @description Team ID — used when a team admin searches for users to add to their team */
|
||||
team_id?: string | null;
|
||||
/** @description Page number for pagination */
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue