mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
Merge pull request #39206 from BerriAI/litellm_lit_3925_clear_team_key_create
fix: stop a cleared Team field from blocking personal key creation
This commit is contained in:
commit
fc1a5fd7f9
7 changed files with 147 additions and 19 deletions
|
|
@ -1216,6 +1216,13 @@ class GenerateKeyRequest(KeyRequestBase):
|
|||
organization_id: str | None = None
|
||||
project_id: str | None = None
|
||||
|
||||
@field_validator("team_id", mode="before")
|
||||
@classmethod
|
||||
def treat_cleared_team_id_as_unset(cls, v: object) -> object:
|
||||
if v == "":
|
||||
return None
|
||||
return v
|
||||
|
||||
|
||||
class GenerateKeyResponse(KeyRequestBase):
|
||||
key: str
|
||||
|
|
|
|||
|
|
@ -17489,3 +17489,49 @@ async def test_check_project_key_limits_still_rejects_real_model_outside_project
|
|||
|
||||
assert exc_info.value.status_code == 400
|
||||
assert "Model 'gpt-5.4-mini' not in project's allowed models" in exc_info.value.detail["error"]
|
||||
|
||||
|
||||
def test_generate_key_request_blank_team_id_is_personal():
|
||||
"""The UI Team-field clear submits team_id=""; it must count as no team (LIT-3925)."""
|
||||
from litellm.proxy._types import RegenerateKeyRequest
|
||||
from litellm.proxy.management_endpoints.key_management_endpoints import (
|
||||
_is_team_key,
|
||||
)
|
||||
|
||||
cleared = GenerateKeyRequest(team_id="")
|
||||
assert cleared.team_id is None
|
||||
assert _is_team_key(data=cleared) is False
|
||||
assert RegenerateKeyRequest(team_id="").team_id is None
|
||||
assert GenerateKeyRequest(team_id="team-1").team_id == "team-1"
|
||||
|
||||
|
||||
def test_key_generation_check_blank_team_id_uses_personal_permissions(monkeypatch):
|
||||
"""key_generation_check with team_id="" must take the personal-key path instead
|
||||
of failing the team lookup with "Unable to find team object" (LIT-3925)."""
|
||||
from litellm.proxy._types import KeyManagementRoutes
|
||||
from litellm.proxy.management_endpoints.key_management_endpoints import (
|
||||
key_generation_check,
|
||||
)
|
||||
|
||||
monkeypatch.setattr(
|
||||
litellm,
|
||||
"key_generation_settings",
|
||||
{
|
||||
"team_key_generation": {"allowed_team_member_roles": ["admin"]},
|
||||
"personal_key_generation": {"allowed_user_roles": ["proxy_admin", "internal_user"]},
|
||||
},
|
||||
)
|
||||
|
||||
assert (
|
||||
key_generation_check(
|
||||
team_table=None,
|
||||
user_api_key_dict=UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.INTERNAL_USER,
|
||||
api_key="sk-alice",
|
||||
user_id="alice",
|
||||
),
|
||||
data=GenerateKeyRequest(key_alias="personal", team_id=""),
|
||||
route=KeyManagementRoutes.KEY_GENERATE,
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
|
|
|||
|
|
@ -111,17 +111,17 @@ describe("ModelsAndEndpointsPage", () => {
|
|||
// POST /model/new 403s a proxy_admin_viewer, so the form's tab must not render for one.
|
||||
it("hides the Add Model tab for a view-only admin session", () => {
|
||||
mockUseAuthorized.mockReturnValue(VIEW_ONLY_ADMIN);
|
||||
const { getByRole, queryByRole } = renderPage();
|
||||
expect(queryByRole("tab", { name: "Add Model" })).not.toBeInTheDocument();
|
||||
expect(getByRole("tab", { name: "All Models" })).toBeInTheDocument();
|
||||
renderPage();
|
||||
expect(screen.queryByRole("tab", { name: "Add Model" })).not.toBeInTheDocument();
|
||||
expect(screen.getByRole("tab", { name: "All Models" })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Read parity: the Auto-Routers list stays reachable for a view-only admin; only the
|
||||
// create affordance inside it is withheld, which AutoRoutersTabPanel decides.
|
||||
it("keeps the Auto-Routers tab for a view-only admin session", () => {
|
||||
mockUseAuthorized.mockReturnValue(VIEW_ONLY_ADMIN);
|
||||
const { getByRole } = renderPage();
|
||||
expect(getByRole("tab", { name: /Auto-Routers/ })).toBeInTheDocument();
|
||||
renderPage();
|
||||
expect(screen.getByRole("tab", { name: /Auto-Routers/ })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// Auto-routers are excluded from the All Models table, so this tab is their home: the only
|
||||
|
|
|
|||
|
|
@ -101,18 +101,24 @@ vi.mock("./build_complexity_router_config", async (importOriginal) => {
|
|||
});
|
||||
|
||||
// A real TeamDropdown fetches teams and renders an antd Select; the wiring under test is
|
||||
// whether team_id is registered, validated and forwarded, so a plain control stands in.
|
||||
// whether team_id is registered, validated and forwarded, so a plain control stands in. The
|
||||
// clear button mirrors the real dropdown's x, which emits null rather than a string.
|
||||
vi.mock("../common_components/team_dropdown", () => ({
|
||||
default: ({ value, onChange }: { value?: string; onChange?: (next: string) => void }) => (
|
||||
<select
|
||||
data-testid="team-dropdown"
|
||||
value={value ?? ""}
|
||||
onChange={(event) => onChange?.(event.target.value)}
|
||||
aria-label="Select Team"
|
||||
>
|
||||
<option value="">none</option>
|
||||
<option value="team-1">team-1</option>
|
||||
</select>
|
||||
default: ({ value, onChange }: { value?: string; onChange?: (next: string | null) => void }) => (
|
||||
<>
|
||||
<select
|
||||
data-testid="team-dropdown"
|
||||
value={value ?? ""}
|
||||
onChange={(event) => onChange?.(event.target.value)}
|
||||
aria-label="Select Team"
|
||||
>
|
||||
<option value="">none</option>
|
||||
<option value="team-1">team-1</option>
|
||||
</select>
|
||||
<button type="button" data-testid="team-dropdown-clear" onClick={() => onChange?.(null)}>
|
||||
clear team
|
||||
</button>
|
||||
</>
|
||||
),
|
||||
}));
|
||||
|
||||
|
|
@ -354,6 +360,25 @@ describe("AddAutoRouterTab", () => {
|
|||
expect(handleAddAutoRouterSubmit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// The shared dropdown emits null on clear while this form's schema wants a string, so the
|
||||
// form maps null back to "": the user sees the pick-a-team message, not a zod type error.
|
||||
it("treats a team picked and then cleared like no team at all", async () => {
|
||||
const user = userEvent.setup();
|
||||
vi.mocked(getMissingTiersError).mockReturnValue(null);
|
||||
|
||||
renderWithProviders(
|
||||
<AddAutoRouterTab handleOk={vi.fn()} accessToken="token" userRole="Internal User" createScope="team-required" />,
|
||||
);
|
||||
|
||||
await user.type(screen.getByPlaceholderText(/smart_router/i), "team-scoped-router");
|
||||
await user.selectOptions(screen.getByTestId("team-dropdown"), "team-1");
|
||||
await user.click(screen.getByTestId("team-dropdown-clear"));
|
||||
await user.click(screen.getByRole("button", { name: /add auto router/i }));
|
||||
|
||||
expect(await screen.findByText("Please select a team to continue")).toBeInTheDocument();
|
||||
expect(handleAddAutoRouterSubmit).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("defaults a new router to session affinity off, matching the backend field default", async () => {
|
||||
const user = userEvent.setup();
|
||||
vi.mocked(getMissingTiersError).mockReturnValue(null);
|
||||
|
|
|
|||
|
|
@ -548,7 +548,9 @@ const AddAutoRouterTab: React.FC<AddAutoRouterTabProps> = ({
|
|||
"Select the team this auto router belongs to. Only keys for this team will be able to call it.",
|
||||
)}
|
||||
>
|
||||
{({ id, value, onChange }) => <TeamDropdown id={id} value={value} onChange={onChange} />}
|
||||
{({ id, value, onChange }) => (
|
||||
<TeamDropdown id={id} value={value} onChange={(next) => onChange(next ?? "")} />
|
||||
)}
|
||||
</FormField>
|
||||
)}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,48 @@
|
|||
import { render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { chooseSelectOption } from "../../../tests/test-utils";
|
||||
import type { Team } from "../key_team_helpers/key_list";
|
||||
import TeamDropdown from "./team_dropdown";
|
||||
|
||||
const TEAMS = [
|
||||
{ team_id: "team-1", team_alias: "Alpha Team" },
|
||||
{ team_id: "team-2", team_alias: "Beta Team" },
|
||||
] as unknown as Team[];
|
||||
|
||||
vi.mock("@/app/(dashboard)/hooks/teams/useTeams", () => ({
|
||||
useInfiniteTeams: () => ({
|
||||
data: { pages: [{ teams: TEAMS }] },
|
||||
fetchNextPage: vi.fn(),
|
||||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
isLoading: false,
|
||||
}),
|
||||
}));
|
||||
|
||||
describe("TeamDropdown", () => {
|
||||
it("emits the picked team's id and full object", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const onTeamSelect = vi.fn();
|
||||
render(<TeamDropdown onChange={onChange} onTeamSelect={onTeamSelect} />);
|
||||
|
||||
await chooseSelectOption(user, screen.getByRole("combobox"), /^Beta Team/);
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith("team-2");
|
||||
expect(onTeamSelect).toHaveBeenCalledWith(TEAMS[1]);
|
||||
});
|
||||
|
||||
it("emits null, never the empty string, when the selection is cleared", async () => {
|
||||
const user = userEvent.setup();
|
||||
const onChange = vi.fn();
|
||||
const onTeamSelect = vi.fn();
|
||||
render(<TeamDropdown value="team-1" onChange={onChange} onTeamSelect={onTeamSelect} />);
|
||||
|
||||
await user.click(screen.getByRole("button", { name: "Clear" }));
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith(null);
|
||||
expect(onTeamSelect).toHaveBeenCalledWith(null);
|
||||
});
|
||||
});
|
||||
|
|
@ -5,7 +5,7 @@ import { Team } from "../key_team_helpers/key_list";
|
|||
|
||||
interface TeamDropdownProps {
|
||||
value?: string;
|
||||
onChange?: (value: string) => void;
|
||||
onChange?: (value: string | null) => void;
|
||||
/** Callback with the full Team object (or null on clear). */
|
||||
onTeamSelect?: (team: Team | null) => void;
|
||||
disabled?: boolean;
|
||||
|
|
@ -47,7 +47,7 @@ const TeamDropdown: React.FC<TeamDropdownProps> = ({
|
|||
}, [data]);
|
||||
|
||||
const handleChange = (teamId: string) => {
|
||||
onChange?.(teamId);
|
||||
onChange?.(teamId || null);
|
||||
if (onTeamSelect) {
|
||||
onTeamSelect(teamId ? teams.find((t) => t.team_id === teamId) ?? null : null);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue