tiennm99/litellm
1
0
Fork 0
mirror of https://github.com/tiennm99/litellm.git synced 2026-07-30 16:22:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ui): omit allowed_routes from key edit save when unchanged (#27553)

Browse Source 
* fix(ui): omit allowed_routes from key edit save when unchanged

When a team admin opens Edit Settings on a key with key_type=AI APIs and
saves without changing anything, the UI re-sends the existing allowed_routes
value, which the backend's _check_allowed_routes_caller_permission gate
rejects for non-proxy-admins (LIT-2681).

Strip allowed_routes from the patch in handleSubmit when it deep-equals the
original keyData.allowed_routes. The backend treats absence as "leave alone,"
so no-op saves now succeed for non-admins. Admins explicitly editing the
field still send the new value.

* fix(ui): order-insensitive allowed_routes diff + cover null-original case

Address Greptile review:

- Switch the "is allowed_routes unchanged" check to a Set-based comparison so
  a server-side reorder of the array doesn't register as a user edit and
  re-trigger LIT-2681.
- Add two regression tests: (1) keyData.allowed_routes is null and the form
  is untouched — patch should strip the field; (2) server returned routes in
  a different order than the user originally entered — patch should still
  recognize the value as unchanged.

* chore(ui): strip ticket refs and tighten comments in key edit fix

- Remove internal-tracker references from in-code comments
- Tighten the WHY comment in handleSubmit to two lines
- Drop redundant test-block comments — test names already describe the case

* fix(ui): annotate Set<string> generic in allowed_routes diff to fix tsc
This commit is contained in:
ryan-crabbe-berri
2026-05-09 19:01:58 -07:00
committed by GitHub
parent 02edaef50c
commit 0af33fbe70
2 changed files with 180 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/litellm-dashboard/src/components/templates/key_edit_view.test.tsx
+132 -30
View File
@@ -158,8 +158,8 @@ describe("KeyEditView", () => {
const { getByText } = renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -176,8 +176,8 @@ describe("KeyEditView", () => {
const { getByText } = renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -194,8 +194,8 @@ describe("KeyEditView", () => {
const { getByLabelText } = renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -219,7 +219,7 @@ describe("KeyEditView", () => {
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={onCancelMock}
onSubmit={async () => { }}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -241,8 +241,8 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -259,8 +259,8 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -277,8 +277,8 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -295,8 +295,8 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -314,7 +314,7 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onCancel={() => {}}
onSubmit={onSubmitMock}
accessToken={"test-token"}
userID={"test-user"}
@@ -344,8 +344,8 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={keyDataWithManagementRoutes}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -367,8 +367,8 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={keyDataWithInfoRoutes}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={""}
userID={""}
userRole={""}
@@ -385,8 +385,8 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onSubmit={async () => { }}
onCancel={() => {}}
onSubmit={async () => {}}
accessToken={"test-token"}
userID={""}
userRole={""}
@@ -404,7 +404,7 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onCancel={() => {}}
onSubmit={onSubmitMock}
accessToken={"test-token"}
userID={"test-user"}
@@ -434,10 +434,14 @@ describe("KeyEditView", () => {
it("should handle empty allowed routes string on submit", async () => {
const onSubmitMock = vi.fn().mockResolvedValue(undefined);
const keyDataWithRoutes = {
...MOCK_KEY_DATA,
allowed_routes: ["llm_api_routes"],
};
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
keyData={keyDataWithRoutes}
onCancel={() => {}}
onSubmit={onSubmitMock}
accessToken={"test-token"}
userID={"test-user"}
@@ -463,6 +467,101 @@ describe("KeyEditView", () => {
});
});
it("should omit allowed_routes from submit when value is unchanged", async () => {
const onSubmitMock = vi.fn().mockResolvedValue(undefined);
const aiApisKeyData = {
...MOCK_KEY_DATA,
allowed_routes: ["llm_api_routes"],
};
renderWithProviders(
<KeyEditView
keyData={aiApisKeyData}
onCancel={() => {}}
onSubmit={onSubmitMock}
accessToken={"test-token"}
userID={"test-user"}
userRole={"admin"}
premiumUser={false}
/>,
);
await waitFor(() => {
expect(screen.getByRole("button", { name: /save changes/i })).toBeInTheDocument();
});
const submitButton = screen.getByRole("button", { name: /save changes/i });
await userEvent.click(submitButton);
await waitFor(() => {
expect(onSubmitMock).toHaveBeenCalled();
const callArgs = onSubmitMock.mock.calls[0][0];
expect("allowed_routes" in callArgs).toBe(false);
});
});
it("should omit allowed_routes from submit when keyData.allowed_routes is null and form is untouched", async () => {
const onSubmitMock = vi.fn().mockResolvedValue(undefined);
const keyDataNullRoutes = {
...MOCK_KEY_DATA,
allowed_routes: null as unknown as string[],
};
renderWithProviders(
<KeyEditView
keyData={keyDataNullRoutes}
onCancel={() => {}}
onSubmit={onSubmitMock}
accessToken={"test-token"}
userID={"test-user"}
userRole={"admin"}
premiumUser={false}
/>,
);
await waitFor(() => {
expect(screen.getByRole("button", { name: /save changes/i })).toBeInTheDocument();
});
const submitButton = screen.getByRole("button", { name: /save changes/i });
await userEvent.click(submitButton);
await waitFor(() => {
expect(onSubmitMock).toHaveBeenCalled();
const callArgs = onSubmitMock.mock.calls[0][0];
expect("allowed_routes" in callArgs).toBe(false);
});
});
it("should omit allowed_routes from submit when server returned routes in a different order", async () => {
const onSubmitMock = vi.fn().mockResolvedValue(undefined);
const keyDataReordered = {
...MOCK_KEY_DATA,
allowed_routes: ["beta_routes", "alpha_routes"],
};
renderWithProviders(
<KeyEditView
keyData={keyDataReordered}
onCancel={() => {}}
onSubmit={onSubmitMock}
accessToken={"test-token"}
userID={"test-user"}
userRole={"admin"}
premiumUser={false}
/>,
);
await waitFor(() => {
expect(screen.getByRole("button", { name: /save changes/i })).toBeInTheDocument();
});
const submitButton = screen.getByRole("button", { name: /save changes/i });
await userEvent.click(submitButton);
await waitFor(() => {
expect(onSubmitMock).toHaveBeenCalled();
const callArgs = onSubmitMock.mock.calls[0][0];
expect("allowed_routes" in callArgs).toBe(false);
});
});
it("should pass access_group_ids to onSubmit when saving key with access groups", async () => {
const onSubmitMock = vi.fn().mockResolvedValue(undefined);
@@ -554,7 +653,7 @@ describe("KeyEditView", () => {
renderWithProviders(
<KeyEditView
keyData={MOCK_KEY_DATA}
onCancel={() => { }}
onCancel={() => {}}
onSubmit={onSubmitMock}
accessToken={"test-token"}
userID={"test-user"}
@@ -576,10 +675,13 @@ describe("KeyEditView", () => {
});
// Wait for the cancel button to actually be disabled (state update may take a moment)
await waitFor(() => {
const cancelButton = screen.getByRole("button", { name: /cancel/i });
expect(cancelButton).toBeDisabled();
}, { timeout: 3000 });
await waitFor(
() => {
const cancelButton = screen.getByRole("button", { name: /cancel/i });
expect(cancelButton).toBeDisabled();
},
{ timeout: 3000 },
);
// Clean up: resolve the promise to allow the form to complete
if (resolveSubmit) {
ui/litellm-dashboard/src/components/templates/key_edit_view.tsx
+48 -34
View File
@@ -78,7 +78,6 @@ const getKeyTypeFromRoutes = (allowedRoutes: string[] | null | undefined): strin
return "default";
};
export function KeyEditView({
keyData,
onCancel,
@@ -106,7 +105,7 @@ export function KeyEditView({
const [neverExpire, setNeverExpire] = useState<boolean>(!keyData.expires);
const [isKeySaving, setIsKeySaving] = useState(false);
const [budgetLimits, setBudgetLimits] = useState<BudgetWindowEntry[]>(
Array.isArray(keyData.budget_limits) ? keyData.budget_limits : []
Array.isArray(keyData.budget_limits) ? keyData.budget_limits : [],
);
const { data: organizations, isLoading: isOrganizationsLoading } = useOrganizations();
const { data: projects } = useProjects();
@@ -116,9 +115,7 @@ export function KeyEditView({
const projectDisplay = (() => {
if (!keyData.project_id) return null;
const project = projects?.find((p) => p.project_id === keyData.project_id);
return project?.project_alias
? `${project.project_alias} (${keyData.project_id})`
: keyData.project_id;
return project?.project_alias ? `${project.project_alias} (${keyData.project_id})` : keyData.project_id;
})();
useEffect(() => {
@@ -198,9 +195,10 @@ export function KeyEditView({
access_group_ids: keyData.access_group_ids || [],
auto_rotate: keyData.auto_rotate || false,
...(keyData.rotation_interval && { rotation_interval: keyData.rotation_interval }),
allowed_routes: Array.isArray(keyData.allowed_routes) && keyData.allowed_routes.length > 0
? keyData.allowed_routes.join(", ")
: "",
allowed_routes:
Array.isArray(keyData.allowed_routes) && keyData.allowed_routes.length > 0
? keyData.allowed_routes.join(", ")
: "",
};
useEffect(() => {
@@ -226,9 +224,10 @@ export function KeyEditView({
access_group_ids: keyData.access_group_ids || [],
auto_rotate: keyData.auto_rotate || false,
...(keyData.rotation_interval && { rotation_interval: keyData.rotation_interval }),
allowed_routes: Array.isArray(keyData.allowed_routes) && keyData.allowed_routes.length > 0
? keyData.allowed_routes.join(", ")
: "",
allowed_routes:
Array.isArray(keyData.allowed_routes) && keyData.allowed_routes.length > 0
? keyData.allowed_routes.join(", ")
: "",
});
}, [keyData, form]);
@@ -275,12 +274,25 @@ export function KeyEditView({
}
// If it's already an array (shouldn't happen, but handle it), keep as is
// Backend rejects non-empty allowed_routes from non-admins, so re-sending
// an unchanged value 403s a team admin. Set compare tolerates reorder.
const originalRoutesSet = new Set<string>(Array.isArray(keyData.allowed_routes) ? keyData.allowed_routes : []);
const submittedRoutesSet = new Set<string>(Array.isArray(values.allowed_routes) ? values.allowed_routes : []);
const allowedRoutesUnchanged =
originalRoutesSet.size === submittedRoutesSet.size &&
[...submittedRoutesSet].every((r) => originalRoutesSet.has(r));
if (allowedRoutesUnchanged) {
delete values.allowed_routes;
}
if (neverExpire) {
values.duration = null;
}
// Include multi-window budget limits (filter out incomplete entries)
const validWindows = budgetLimits.filter((w) => w.budget_duration && w.max_budget !== null && w.max_budget !== undefined);
const validWindows = budgetLimits.filter(
(w) => w.budget_duration && w.max_budget !== null && w.max_budget !== undefined,
);
values.budget_limits = validWindows.length > 0 ? validWindows : undefined;
await onSubmit(values);
@@ -305,9 +317,13 @@ export function KeyEditView({
{({ getFieldValue, setFieldValue }) => {
const allowedRoutesValue = getFieldValue("allowed_routes") || "";
// Convert string to array for checking
const allowedRoutes = typeof allowedRoutesValue === "string" && allowedRoutesValue.trim() !== ""
? allowedRoutesValue.split(",").map((r: string) => r.trim()).filter((r: string) => r.length > 0)
: [];
const allowedRoutes =
typeof allowedRoutesValue === "string" && allowedRoutesValue.trim() !== ""
? allowedRoutesValue
.split(",")
.map((r: string) => r.trim())
.filter((r: string) => r.length > 0)
: [];
const isDisabled = allowedRoutes.includes("management_routes") || allowedRoutes.includes("info_routes");
const models = getFieldValue("models") || [];
@@ -348,9 +364,13 @@ export function KeyEditView({
{({ getFieldValue, setFieldValue }) => {
const allowedRoutesValue = getFieldValue("allowed_routes") || "";
// Convert string to array for getKeyTypeFromRoutes
const allowedRoutes = typeof allowedRoutesValue === "string" && allowedRoutesValue.trim() !== ""
? allowedRoutesValue.split(",").map((r: string) => r.trim()).filter((r: string) => r.length > 0)
: [];
const allowedRoutes =
typeof allowedRoutesValue === "string" && allowedRoutesValue.trim() !== ""
? allowedRoutesValue
.split(",")
.map((r: string) => r.trim())
.filter((r: string) => r.length > 0)
: [];
const keyTypeValue = getKeyTypeFromRoutes(allowedRoutes);
return (
@@ -415,9 +435,7 @@ export function KeyEditView({
}
name="allowed_routes"
>
<Input
placeholder="Enter allowed routes (comma-separated). Special values: llm_api_routes, management_routes. Examples: llm_api_routes, /chat/completions, /keys/*. Leave empty to allow all routes"
/>
<Input placeholder="Enter allowed routes (comma-separated). Special values: llm_api_routes, management_routes. Examples: llm_api_routes, /chat/completions, /keys/*. Leave empty to allow all routes" />
</Form.Item>
<Form.Item label="Max Budget (USD)" name="max_budget">
@@ -442,10 +460,7 @@ export function KeyEditView({
</span>
}
>
<BudgetWindowsEditor
value={budgetLimits}
onChange={setBudgetLimits}
/>
<BudgetWindowsEditor value={budgetLimits} onChange={setBudgetLimits} />
</Form.Item>
<Form.Item label="TPM Limit" name="tpm_limit">
@@ -579,7 +594,7 @@ export function KeyEditView({
!premiumUser
? "Premium feature - Upgrade to set allowed pass through routes by key"
: Array.isArray(keyData.metadata?.allowed_passthrough_routes) &&
keyData.metadata.allowed_passthrough_routes.length > 0
keyData.metadata.allowed_passthrough_routes.length > 0
? `Current: ${keyData.metadata.allowed_passthrough_routes.join(", ")}`
: "Select or enter allowed pass through routes"
}
@@ -690,14 +705,13 @@ export function KeyEditView({
return team.team_alias?.toLowerCase().includes(input.toLowerCase()) ?? false;
}}
>
{(selectedOrganizationId
? teams?.filter((t) => t.organization_id === selectedOrganizationId)
: teams
)?.map((team) => (
<Select.Option key={team.team_id} value={team.team_id}>
{`${team.team_alias} (${team.team_id})`}
</Select.Option>
))}
{(selectedOrganizationId ? teams?.filter((t) => t.organization_id === selectedOrganizationId) : teams)?.map(
(team) => (
<Select.Option key={team.team_id} value={team.team_id}>
{`${team.team_alias} (${team.team_id})`}
</Select.Option>
),
)}
</Select>
</Form.Item>
{enableProjectsUI && hasProject && (