mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-01 02:02:20 +00:00
fix(ui): omit unchanged allowed_routes on key update
Non-admin team admins could not save key edit settings because the UI
always submitted the current allowed_routes value in the update payload,
triggering the backend permission check added in PR #25445.
This follows the established precedent from commit 2c41f3c29 (policies
field fix): strip allowed_routes from the update payload when the form
value equals the previously persisted value so non-admin editors don't
trip the backend 'setting allowed_routes' permission check on a no-op
save. Genuine route changes (including clears) still pass through.
Adds normalizeStringList and areStringListsEqual helpers for robust
comparison of the route list values.
Closes #27005
Co-authored-by: Krrish Dholakia <krrish-berri-2@users.noreply.github.com>
This commit is contained in:
parent
de7175d6ab
commit
31f95d9117
2 changed files with 118 additions and 0 deletions
|
|
@ -777,4 +777,92 @@ describe("KeyInfoView", () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("allowed_routes payload normalization", () => {
|
||||
const enterEditMode = async (keyData: KeyResponse) => {
|
||||
vi.mocked(useAuthorized).mockReturnValue({
|
||||
...baseUseAuthorizedMock,
|
||||
userId: "proxy-admin-user",
|
||||
userRole: "proxy_admin",
|
||||
});
|
||||
renderWithProviders(
|
||||
<KeyInfoView
|
||||
keyData={keyData}
|
||||
onClose={() => {}}
|
||||
keyId="test-key-id"
|
||||
onKeyDataUpdate={() => {}}
|
||||
teams={[]}
|
||||
/>,
|
||||
);
|
||||
await userEvent.click(screen.getByRole("tab", { name: /settings/i }));
|
||||
await userEvent.click(screen.getByRole("button", { name: /edit settings/i }));
|
||||
await waitFor(() => expect(editViewMocks.onSubmit).toBeDefined());
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
editViewMocks.onSubmit = undefined;
|
||||
vi.mocked(keyUpdateCall).mockClear();
|
||||
vi.mocked(keyUpdateCall).mockResolvedValue({});
|
||||
});
|
||||
|
||||
it("should drop allowed_routes when the submitted value matches the existing key", async () => {
|
||||
const keyData: KeyResponse = {
|
||||
...MOCK_KEY_DATA,
|
||||
user_id: "proxy-admin-user",
|
||||
allowed_routes: ["management_routes"],
|
||||
} as KeyResponse;
|
||||
|
||||
await enterEditMode(keyData);
|
||||
await editViewMocks.onSubmit!({
|
||||
key: keyData.token,
|
||||
token: keyData.token,
|
||||
allowed_routes: ["management_routes"],
|
||||
});
|
||||
|
||||
expect(keyUpdateCall).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.not.objectContaining({ allowed_routes: expect.anything() }),
|
||||
);
|
||||
});
|
||||
|
||||
it("should drop empty allowed_routes when the key previously had no route override", async () => {
|
||||
const keyData: KeyResponse = {
|
||||
...MOCK_KEY_DATA,
|
||||
user_id: "proxy-admin-user",
|
||||
allowed_routes: [],
|
||||
} as KeyResponse;
|
||||
|
||||
await enterEditMode(keyData);
|
||||
await editViewMocks.onSubmit!({
|
||||
key: keyData.token,
|
||||
token: keyData.token,
|
||||
allowed_routes: [],
|
||||
});
|
||||
|
||||
expect(keyUpdateCall).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.not.objectContaining({ allowed_routes: expect.anything() }),
|
||||
);
|
||||
});
|
||||
|
||||
it("should keep allowed_routes when the user clears an existing route override", async () => {
|
||||
const keyData: KeyResponse = {
|
||||
...MOCK_KEY_DATA,
|
||||
user_id: "proxy-admin-user",
|
||||
allowed_routes: ["management_routes"],
|
||||
} as KeyResponse;
|
||||
|
||||
await enterEditMode(keyData);
|
||||
await editViewMocks.onSubmit!({
|
||||
key: keyData.token,
|
||||
token: keyData.token,
|
||||
allowed_routes: [],
|
||||
});
|
||||
|
||||
expect(keyUpdateCall).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.objectContaining({ allowed_routes: [] }),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -49,6 +49,30 @@ const isEmptyValue = (v: unknown): boolean =>
|
|||
(Array.isArray(v) && v.length === 0) ||
|
||||
(typeof v === "string" && v.trim() === "");
|
||||
|
||||
const normalizeStringList = (value: unknown): string[] => {
|
||||
if (Array.isArray(value)) {
|
||||
return value
|
||||
.map((entry) => (typeof entry === "string" ? entry.trim() : ""))
|
||||
.filter((entry) => entry.length > 0);
|
||||
}
|
||||
if (typeof value === "string") {
|
||||
return value
|
||||
.split(",")
|
||||
.map((entry) => entry.trim())
|
||||
.filter((entry) => entry.length > 0);
|
||||
}
|
||||
return [];
|
||||
};
|
||||
|
||||
const areStringListsEqual = (left: unknown, right: unknown): boolean => {
|
||||
const normalizedLeft = normalizeStringList(left);
|
||||
const normalizedRight = normalizeStringList(right);
|
||||
return (
|
||||
normalizedLeft.length === normalizedRight.length &&
|
||||
normalizedLeft.every((entry, index) => entry === normalizedRight[index])
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* ─────────────────────────────────────────────────────────────────────────
|
||||
* @deprecated
|
||||
|
|
@ -174,6 +198,12 @@ export default function KeyInfoView({
|
|||
}
|
||||
}
|
||||
|
||||
// Strip unchanged allowed_routes so non-admin editors don't trip the
|
||||
// backend "setting allowed_routes" permission check on a no-op save.
|
||||
if (areStringListsEqual(formValues.allowed_routes, currentKeyData.allowed_routes)) {
|
||||
delete formValues.allowed_routes;
|
||||
}
|
||||
|
||||
// Handle max budget empty string
|
||||
formValues.max_budget = mapEmptyStringToNull(formValues.max_budget);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue