mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
fix(scim): apply path-less group PATCH ops instead of storing them under an empty metadata key (#43978)
* fix(scim): apply path-less group PATCH ops instead of storing them under an empty metadata key A path-less add/replace op (RFC 7644 3.5.2, what Okta Push Groups sends on a rename) carries a partial Group resource. Each of its attributes now applies as if sent with that path, so displayName updates the team alias and externalId and members get their usual handling, and the pushed attributes merge into the scim_data snapshot the PUT path already writes. A path-less remove or a path-less op without an object value is rejected with a 400. Any group PATCH drops an empty metadata key an earlier push left behind, and the Admin UI metadata form skips an empty key so an affected team can save its settings. * fix(scim): let a later path op win over an earlier path-less value in the group snapshot * fix(scim): type the stored team metadata before the JSON object check * test(scim): run the real group transformation in the path-less replace test * test(scim): assert the renamed group comes back from the path-less replace * test(scim): audit the path-less group PATCH on the live proxy --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
This commit is contained in:
devin-ai-integration[bot]
GitHub
parent
4a847ac936
commit
0238ec9721
7 changed files with 989 additions and 49 deletions
|
|
@ -10,6 +10,7 @@ from copy import deepcopy
|
|||
from dataclasses import dataclass
|
||||
from functools import partial
|
||||
from itertools import chain
|
||||
from types import MappingProxyType
|
||||
from typing import TYPE_CHECKING, Final, NamedTuple, Protocol, overload
|
||||
|
||||
from fastapi import (
|
||||
|
|
@ -2708,6 +2709,75 @@ async def delete_group(
|
|||
raise handle_exception_on_proxy(e)
|
||||
|
||||
|
||||
GROUP_PATCH_READ_ONLY_ATTRIBUTES: Final = frozenset({"id", "schemas", "meta"})
|
||||
_NO_FIELDS: Final[Mapping[str, object]] = MappingProxyType({})
|
||||
|
||||
|
||||
def _pathless_group_resource(op: SCIMPatchOperation) -> Mapping[str, object] | None:
|
||||
"""The partial Group resource a path-less op carries, or None when the op names a path.
|
||||
|
||||
RFC 7644 Section 3.5.2 lets ``add`` and ``replace`` omit ``path`` and send the
|
||||
attributes to apply as an object (what Okta Push Groups does on a rename);
|
||||
``remove`` always needs a path (Section 3.5.2.2).
|
||||
"""
|
||||
if op.path:
|
||||
return None
|
||||
resource: Final = _json_object_fields(op.value)
|
||||
if op.op != "remove" and resource is not None:
|
||||
return resource
|
||||
detail: Final[_ScimErrorDetail] = {
|
||||
"error": (
|
||||
"A remove operation requires a 'path' (RFC 7644 Section 3.5.2.2)"
|
||||
if op.op == "remove"
|
||||
else f"A {op.op} operation without a 'path' requires an object 'value' (RFC 7644 Section 3.5.2)"
|
||||
)
|
||||
}
|
||||
raise HTTPException(status_code=400, detail=detail)
|
||||
|
||||
|
||||
def _group_patch_attribute_values(op: SCIMPatchOperation) -> tuple[tuple[str, object], ...]:
|
||||
"""The (attribute, value) pairs an operation applies, one per key of a path-less value."""
|
||||
resource: Final = _pathless_group_resource(op)
|
||||
if resource is None:
|
||||
return (((op.path or "").lower(), op.value),)
|
||||
return tuple(
|
||||
(key.lower(), value)
|
||||
for key, value in resource.items()
|
||||
if key and key.lower() not in GROUP_PATCH_READ_ONLY_ATTRIBUTES
|
||||
)
|
||||
|
||||
|
||||
def _replaces_members(op: SCIMPatchOperation) -> bool:
|
||||
if op.op != "replace":
|
||||
return False
|
||||
return any(attribute.startswith("members") for attribute, _ in _group_patch_attribute_values(op))
|
||||
|
||||
|
||||
def _patched_group_snapshot(
|
||||
existing_snapshot: Mapping[str, object],
|
||||
pathless_resources: Sequence[Mapping[str, object]],
|
||||
mirrored_values: Sequence[tuple[str, object | None]],
|
||||
) -> dict[str, object]:
|
||||
"""The ``scim_data`` snapshot after a PATCH: the path-less resources merged over the
|
||||
existing snapshot in operation order (``members`` live in members_with_roles), then
|
||||
each attribute in ``mirrored_values`` set to what the whole operation list left on
|
||||
the team, so a later path op wins over an earlier path-less value; ``None`` drops it.
|
||||
"""
|
||||
pathless_items: Final = (
|
||||
(key, value)
|
||||
for key, value in chain.from_iterable(resource.items() for resource in pathless_resources)
|
||||
if key.lower() != "members"
|
||||
)
|
||||
mirrored_keys: Final = frozenset(key.lower() for key, _ in mirrored_values)
|
||||
kept: Final = (
|
||||
(key, value)
|
||||
for key, value in chain(existing_snapshot.items(), pathless_items)
|
||||
if key.lower() not in mirrored_keys
|
||||
)
|
||||
refreshed: Final = ((key, value) for key, value in mirrored_values if value is not None)
|
||||
return dict(chain(kept, refreshed))
|
||||
|
||||
|
||||
async def _process_group_patch_operations(
|
||||
patch_ops: SCIMPatchOp, existing_team: LiteLLM_TeamTable, prisma_client: PrismaClient
|
||||
) -> tuple[dict[str, object], set[str], set[str] | None]:
|
||||
|
|
@ -2725,11 +2795,24 @@ async def _process_group_patch_operations(
|
|||
conditional on what the id turns out to be and leave members we should never
|
||||
have admitted - the phantom users this endpoint used to create for nested
|
||||
groups - impossible to clean up.
|
||||
|
||||
A path-less op carries a partial Group resource: each attribute applies as if
|
||||
sent with that path, and its attributes other than ``members`` (the roster
|
||||
lives in members_with_roles) are merged in operation order into the
|
||||
``scim_data`` snapshot the PUT path writes, whose displayName and externalId
|
||||
then mirror what the whole operation list left on the team. An empty metadata
|
||||
key left behind by an earlier path-less op (stored whole under ``""``) is
|
||||
dropped.
|
||||
"""
|
||||
update_data: Final[dict[str, object]] = {}
|
||||
stored_metadata: Final[dict[str, object] | None] = existing_team.metadata
|
||||
existing_metadata: Final = _json_object_fields(stored_metadata) or _NO_FIELDS
|
||||
pathless_resources: Final = tuple(
|
||||
resource for resource in map(_pathless_group_resource, patch_ops.Operations) if resource is not None
|
||||
)
|
||||
|
||||
# Create a fresh copy of existing metadata to avoid Prisma issues
|
||||
metadata: Final = {**(existing_team.metadata or {}), SCIM_MANAGED_TEAM_METADATA_KEY: True}
|
||||
kept_metadata_items: Final = ((key, value) for key, value in existing_metadata.items() if key)
|
||||
metadata: Final = dict(chain(kept_metadata_items, ((SCIM_MANAGED_TEAM_METADATA_KEY, True),)))
|
||||
|
||||
# Track member changes. members_with_roles is the source of truth for team
|
||||
# membership; the legacy `members` column is not populated by team creation
|
||||
|
|
@ -2739,58 +2822,69 @@ async def _process_group_patch_operations(
|
|||
current_members: Final = set(await _get_team_member_user_ids_from_team(existing_team))
|
||||
final_members = current_members.copy()
|
||||
|
||||
# Process each patch operation
|
||||
for op in patch_ops.Operations:
|
||||
path = (op.path or "").lower()
|
||||
value = op.value
|
||||
op_type = op.op
|
||||
for attribute, value in _group_patch_attribute_values(op):
|
||||
op_type = op.op
|
||||
|
||||
if path == "displayname":
|
||||
if op_type == "remove":
|
||||
update_data["team_alias"] = None
|
||||
else:
|
||||
update_data["team_alias"] = str(value)
|
||||
elif path == "externalid":
|
||||
if op_type == "remove":
|
||||
metadata.pop("externalId", None)
|
||||
else:
|
||||
metadata["externalId"] = str(value)
|
||||
elif path.startswith("members"):
|
||||
# Handle member operations
|
||||
patched_members = (
|
||||
_parse_member_entries(value)
|
||||
if value is not None
|
||||
else tuple(
|
||||
SCIMMember(value=member_id) for member_id in _extract_ids_from_path_filter(op.path, "members")
|
||||
if attribute == "displayname":
|
||||
if op_type == "remove":
|
||||
update_data["team_alias"] = None
|
||||
else:
|
||||
update_data["team_alias"] = str(value)
|
||||
elif attribute == "externalid":
|
||||
if op_type == "remove":
|
||||
metadata.pop("externalId", None)
|
||||
else:
|
||||
metadata["externalId"] = str(value)
|
||||
elif attribute.startswith("members"):
|
||||
patched_members = (
|
||||
_parse_member_entries(value)
|
||||
if value is not None
|
||||
else tuple(
|
||||
SCIMMember(value=member_id) for member_id in _extract_ids_from_path_filter(op.path, "members")
|
||||
)
|
||||
)
|
||||
|
||||
if op_type == "remove":
|
||||
final_members = final_members - await _member_ids_to_drop(
|
||||
patched_members, frozenset(final_members), prisma_client
|
||||
)
|
||||
else:
|
||||
member_result = await _resolve_group_member_ids(
|
||||
members=patched_members,
|
||||
created_via="scim_group_patch",
|
||||
prisma_client=prisma_client,
|
||||
)
|
||||
if op_type == "replace":
|
||||
final_members = set(member_result.all_member_ids)
|
||||
elif op_type == "add":
|
||||
final_members = final_members | set(member_result.all_member_ids)
|
||||
elif op_type == "remove":
|
||||
metadata.pop(attribute, None)
|
||||
else:
|
||||
metadata[attribute] = value
|
||||
|
||||
if pathless_resources:
|
||||
applied_attributes: Final = frozenset(
|
||||
attribute for attribute, _ in chain.from_iterable(map(_group_patch_attribute_values, patch_ops.Operations))
|
||||
)
|
||||
mirrored_values: Final = tuple(
|
||||
(snapshot_key, final_value)
|
||||
for attribute, snapshot_key, final_value in (
|
||||
("displayname", "displayName", update_data.get("team_alias")),
|
||||
("externalid", "externalId", metadata.get("externalId")),
|
||||
)
|
||||
|
||||
if op_type == "remove":
|
||||
final_members = final_members - await _member_ids_to_drop(
|
||||
patched_members, frozenset(final_members), prisma_client
|
||||
)
|
||||
else:
|
||||
member_result = await _resolve_group_member_ids(
|
||||
members=patched_members,
|
||||
created_via="scim_group_patch",
|
||||
prisma_client=prisma_client,
|
||||
)
|
||||
if op_type == "replace":
|
||||
final_members = set(member_result.all_member_ids)
|
||||
elif op_type == "add":
|
||||
final_members = final_members | set(member_result.all_member_ids)
|
||||
else:
|
||||
# Handle other generic metadata
|
||||
if op_type == "remove":
|
||||
metadata.pop(path, None)
|
||||
else:
|
||||
metadata[path] = value
|
||||
if attribute in applied_attributes
|
||||
)
|
||||
metadata[SCIM_TEAM_DATA_METADATA_KEY] = _patched_group_snapshot(
|
||||
existing_snapshot=_json_object_fields(existing_metadata.get(SCIM_TEAM_DATA_METADATA_KEY)) or _NO_FIELDS,
|
||||
pathless_resources=pathless_resources,
|
||||
mirrored_values=mirrored_values,
|
||||
)
|
||||
|
||||
update_data["metadata"] = metadata
|
||||
|
||||
member_replace_present: Final = any(
|
||||
op.op == "replace" and (op.path or "").lower().startswith("members") for op in patch_ops.Operations
|
||||
)
|
||||
member_replace_present: Final = any(map(_replaces_members, patch_ops.Operations))
|
||||
replace_target: Final = set(final_members) if member_replace_present else None
|
||||
|
||||
return update_data, final_members, replace_target
|
||||
|
|
|
|||
|
|
@ -9,5 +9,6 @@
|
|||
"tests/e2e/ui/tests/integrationCritical/mcpUserEnvVars.spec.ts::clearing credentials for a server deleted underneath the modal reports the failure without losing the page",
|
||||
"tests/e2e/ui/tests/integrationCritical/costOptimizationModelGroups.spec.ts::cache leakage by model merges a deployment's resolved and requested model names into its model group",
|
||||
"tests/e2e/ui/tests/integrationCritical/logsDrawerCredentialCanary.spec.ts::the Logs drawer renders the stored request without the deployment api_key",
|
||||
"tests/e2e/ui/tests/integrationCritical/toolPoliciesUserColumn.spec.ts::the Tool Policies page names the user behind the key that discovered a tool"
|
||||
"tests/e2e/ui/tests/integrationCritical/toolPoliciesUserColumn.spec.ts::the Tool Policies page names the user behind the key that discovered a tool",
|
||||
"tests/e2e/ui/tests/integrationCritical/teamMetadataEmptyKey.spec.ts::the team settings form skips a metadata row with an empty key and saving drops the key"
|
||||
]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,90 @@
|
|||
import {
|
||||
test,
|
||||
expect,
|
||||
APIRequestContext,
|
||||
Page as PlaywrightPage,
|
||||
} from "@playwright/test";
|
||||
import { randomUUID } from "node:crypto";
|
||||
|
||||
const master = process.env.LITELLM_MASTER_KEY ?? "sk-integration-master";
|
||||
const headers = { Authorization: `Bearer ${master}` };
|
||||
|
||||
async function createTeamCarryingAnEmptyMetadataKey(
|
||||
request: APIRequestContext,
|
||||
): Promise<string> {
|
||||
const created = await request.post("/team/new", {
|
||||
headers,
|
||||
data: {
|
||||
team_alias: `int_empty_key_${randomUUID().replace(/-/g, "").slice(0, 12)}`,
|
||||
},
|
||||
});
|
||||
expect(created.ok(), await created.text()).toBe(true);
|
||||
const teamId = (await created.json()).team_id as string;
|
||||
const seeded = await request.post("/team/update", {
|
||||
headers,
|
||||
data: {
|
||||
team_id: teamId,
|
||||
metadata: { "": { displayName: "stale" }, env: "staging" },
|
||||
},
|
||||
});
|
||||
expect(seeded.ok(), await seeded.text()).toBe(true);
|
||||
return teamId;
|
||||
}
|
||||
|
||||
async function teamMetadata(
|
||||
request: APIRequestContext,
|
||||
teamId: string,
|
||||
): Promise<Record<string, unknown>> {
|
||||
const response = await request.get(`/team/info?team_id=${teamId}`, {
|
||||
headers,
|
||||
});
|
||||
expect(response.ok(), await response.text()).toBe(true);
|
||||
const json = await response.json();
|
||||
return (json.team_info?.metadata ?? {}) as Record<string, unknown>;
|
||||
}
|
||||
|
||||
async function loginAsAdmin(page: PlaywrightPage): Promise<void> {
|
||||
await page.goto("/ui/login");
|
||||
await page.getByPlaceholder("Enter your username").fill("admin");
|
||||
await page.getByPlaceholder("Enter your password").fill(master);
|
||||
await page.getByRole("button", { name: "Login", exact: true }).click();
|
||||
await expect(page).toHaveURL(
|
||||
(url) => url.pathname.startsWith("/ui") && !url.pathname.includes("login"),
|
||||
);
|
||||
}
|
||||
|
||||
test("the team settings form skips a metadata row with an empty key and saving drops the key", async ({
|
||||
page,
|
||||
request,
|
||||
}) => {
|
||||
const teamId = await createTeamCarryingAnEmptyMetadataKey(request);
|
||||
try {
|
||||
expect(Object.keys(await teamMetadata(request, teamId))).toContain("");
|
||||
await loginAsAdmin(page);
|
||||
await page.goto(`/ui/models-and-endpoints?team=${teamId}`);
|
||||
await page.getByRole("tab", { name: "Settings" }).click();
|
||||
await page.getByRole("button", { name: /edit settings/i }).click();
|
||||
await expect(page.getByLabel(/Team Name/)).toBeVisible();
|
||||
const keys = page.getByPlaceholder("Key", { exact: true });
|
||||
await expect(keys).toHaveCount(1);
|
||||
await expect(keys.first()).toHaveValue("env");
|
||||
await expect(
|
||||
page.getByPlaceholder("Value", { exact: true }).first(),
|
||||
).toHaveValue("staging");
|
||||
await page.getByRole("button", { name: "Save Changes" }).click();
|
||||
await expect
|
||||
.poll(async () => {
|
||||
const metadata = await teamMetadata(request, teamId);
|
||||
return { hasEmptyKey: "" in metadata, env: metadata.env };
|
||||
})
|
||||
.toEqual({ hasEmptyKey: false, env: "staging" });
|
||||
} finally {
|
||||
const removed = await request.post("/team/delete", {
|
||||
headers,
|
||||
data: { team_ids: [teamId] },
|
||||
});
|
||||
expect(removed.ok() || removed.status() === 404, await removed.text()).toBe(
|
||||
true,
|
||||
);
|
||||
}
|
||||
});
|
||||
549
tests/integration/management/test_scim_group_pathless_patch.py
Normal file
549
tests/integration/management/test_scim_group_pathless_patch.py
Normal file
|
|
@ -0,0 +1,549 @@
|
|||
import os
|
||||
import signal
|
||||
import threading
|
||||
import uuid
|
||||
from collections.abc import Callable, Mapping, Sequence
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from dataclasses import dataclass
|
||||
from itertools import chain
|
||||
from pathlib import Path
|
||||
from types import MappingProxyType
|
||||
from typing import Final
|
||||
|
||||
import httpx
|
||||
import psutil
|
||||
import pytest
|
||||
from integration._support.client import Gateway, Scenario, eventually, object_value, string_value
|
||||
from integration._support.process import owned_proxy_process
|
||||
from integration.authorization._guardrail_opt_out import upstream_hits
|
||||
from pydantic import JsonValue
|
||||
|
||||
PATCH_OP_SCHEMA: Final = "urn:ietf:params:scim:api:messages:2.0:PatchOp"
|
||||
GROUP_SCHEMA: Final = "urn:ietf:params:scim:schemas:core:2.0:Group"
|
||||
BURST_TEAMS: Final = 10
|
||||
BURST_REQUESTS_PER_TEAM: Final = 3
|
||||
KILL_AFTER_RESPONSES: Final = 5
|
||||
|
||||
|
||||
def patch_group(
|
||||
candidate: Gateway, team: str, operations: Sequence[JsonValue], *, key: str | None = None
|
||||
) -> httpx.Response:
|
||||
return candidate.request(
|
||||
"PATCH",
|
||||
f"/scim/v2/Groups/{team}",
|
||||
{"schemas": [PATCH_OP_SCHEMA], "Operations": list(operations)},
|
||||
key=key,
|
||||
)
|
||||
|
||||
|
||||
def pathless(op: str, value: JsonValue) -> dict[str, JsonValue]:
|
||||
return {"op": op, "value": value}
|
||||
|
||||
|
||||
def pathed(op: str, path: str, value: JsonValue | None = None) -> dict[str, JsonValue]:
|
||||
return {"op": op, "path": path, **({} if value is None else {"value": value})}
|
||||
|
||||
|
||||
def team_info(candidate: Gateway, team: str) -> dict[str, JsonValue]:
|
||||
return object_value(candidate.get("/team/info", {"team_id": team})["team_info"])
|
||||
|
||||
|
||||
def team_metadata(candidate: Gateway, team: str) -> dict[str, JsonValue]:
|
||||
return object_value(team_info(candidate, team).get("metadata") or {})
|
||||
|
||||
|
||||
def team_alias(candidate: Gateway, team: str) -> JsonValue:
|
||||
return team_info(candidate, team).get("team_alias")
|
||||
|
||||
|
||||
def alias_and_metadata(candidate: Gateway, team: str) -> tuple[JsonValue, dict[str, JsonValue]]:
|
||||
info: Final = team_info(candidate, team)
|
||||
return info.get("team_alias"), object_value(info.get("metadata") or {})
|
||||
|
||||
|
||||
def member_ids(candidate: Gateway, team: str) -> frozenset[str]:
|
||||
members: Final = team_info(candidate, team).get("members_with_roles") or []
|
||||
assert isinstance(members, list), members
|
||||
return frozenset(string_value(object_value(member)["user_id"]) for member in members)
|
||||
|
||||
|
||||
def group_member_ids(candidate: Gateway, team: str) -> frozenset[str]:
|
||||
members: Final = candidate.get(f"/scim/v2/Groups/{team}").get("members") or []
|
||||
assert isinstance(members, list), members
|
||||
return frozenset(string_value(object_value(member)["value"]) for member in members)
|
||||
|
||||
|
||||
def scim_data(metadata: Mapping[str, JsonValue]) -> dict[str, JsonValue]:
|
||||
return object_value(metadata["scim_data"])
|
||||
|
||||
|
||||
def scim_group(scenario: Scenario, members: Sequence[str]) -> str:
|
||||
created: Final = scenario.gateway.request(
|
||||
"POST",
|
||||
"/scim/v2/Groups",
|
||||
{
|
||||
"schemas": [GROUP_SCHEMA],
|
||||
"displayName": f"integration-{uuid.uuid4().hex}",
|
||||
"members": [{"value": member} for member in members],
|
||||
},
|
||||
)
|
||||
assert created.status_code == 201, created.text
|
||||
team: Final = string_value(object_value(created.json())["id"])
|
||||
scenario.cleanups.callback(scenario.delete_team, team)
|
||||
return team
|
||||
|
||||
|
||||
def model_names(candidate: Gateway) -> frozenset[str]:
|
||||
entries: Final = candidate.get("/model/info")["data"]
|
||||
assert isinstance(entries, list), entries
|
||||
return frozenset(string_value(object_value(entry)["model_name"]) for entry in entries)
|
||||
|
||||
|
||||
def test_pathless_replace_renames_the_team_and_keeps_the_resource_under_scim_data(gateway: Gateway) -> None:
|
||||
renamed: Final = f"okta-renamed-{uuid.uuid4().hex}"
|
||||
external: Final = f"ext-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
response: Final = patch_group(
|
||||
gateway, team, [pathless("replace", {"id": team, "displayName": renamed, "externalId": external})]
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
assert response.json()["displayName"] == renamed, response.text
|
||||
assert team_alias(gateway, team) == renamed
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert set(metadata) == {"externalId", "scim_data", "scim_managed"}, metadata
|
||||
assert metadata["externalId"] == external and metadata["scim_managed"] is True, metadata
|
||||
assert scim_data(metadata) == {"id": team, "displayName": renamed, "externalId": external}, metadata
|
||||
|
||||
|
||||
def test_pathless_replace_with_members_is_an_absolute_roster(gateway: Gateway) -> None:
|
||||
renamed: Final = f"roster-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
first: Final = scenario.user(user_role="internal_user")
|
||||
second: Final = scenario.user(user_role="internal_user")
|
||||
team: Final = scim_group(scenario, [first])
|
||||
assert member_ids(gateway, team) == {first}
|
||||
response: Final = patch_group(
|
||||
gateway, team, [pathless("replace", {"displayName": renamed, "members": [{"value": second}]})]
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
assert member_ids(gateway, team) == {second}
|
||||
assert group_member_ids(gateway, team) == {second}
|
||||
assert team_alias(gateway, team) == renamed
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert "" not in metadata, metadata
|
||||
assert "members" not in scim_data(metadata), metadata
|
||||
|
||||
|
||||
def test_pathless_add_applies_the_attribute(gateway: Gateway) -> None:
|
||||
external: Final = f"ext-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
alias: Final = team_alias(gateway, team)
|
||||
response: Final = patch_group(gateway, team, [pathless("add", {"externalId": external})])
|
||||
assert response.status_code == 200, response.text
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert "" not in metadata, metadata
|
||||
assert metadata["externalId"] == external, metadata
|
||||
assert scim_data(metadata) == {"externalId": external}, metadata
|
||||
assert team_alias(gateway, team) == alias
|
||||
|
||||
|
||||
def test_pathed_operations_are_unchanged(gateway: Gateway) -> None:
|
||||
renamed: Final = f"pathed-{uuid.uuid4().hex}"
|
||||
external: Final = f"ext-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
first: Final = scenario.user(user_role="internal_user")
|
||||
second: Final = scenario.user(user_role="internal_user")
|
||||
team: Final = scim_group(scenario, [first])
|
||||
response: Final = patch_group(
|
||||
gateway,
|
||||
team,
|
||||
[
|
||||
pathed("replace", "displayName", renamed),
|
||||
pathed("replace", "externalId", external),
|
||||
pathed("add", "members", [{"value": second}]),
|
||||
pathed("remove", f'members[value eq "{first}"]'),
|
||||
],
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
assert response.json()["displayName"] == renamed, response.text
|
||||
assert team_alias(gateway, team) == renamed
|
||||
assert member_ids(gateway, team) == {second}
|
||||
assert group_member_ids(gateway, team) == {second}
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert metadata["externalId"] == external, metadata
|
||||
assert "" not in metadata, metadata
|
||||
|
||||
|
||||
def test_pathless_patch_merges_into_the_put_snapshot(gateway: Gateway) -> None:
|
||||
put_alias: Final = f"put-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
member: Final = scenario.user(user_role="internal_user")
|
||||
team: Final = scim_group(scenario, [member])
|
||||
put: Final = gateway.request(
|
||||
"PUT",
|
||||
f"/scim/v2/Groups/{team}",
|
||||
{
|
||||
"schemas": [GROUP_SCHEMA],
|
||||
"id": team,
|
||||
"displayName": put_alias,
|
||||
"externalId": "ext-v1",
|
||||
"members": [{"value": member}],
|
||||
},
|
||||
)
|
||||
assert put.status_code == 200, put.text
|
||||
assert scim_data(team_metadata(gateway, team))["externalId"] == "ext-v1"
|
||||
response: Final = patch_group(gateway, team, [pathless("add", {"externalId": "ext-v2"})])
|
||||
assert response.status_code == 200, response.text
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
snapshot: Final = scim_data(metadata)
|
||||
assert "" not in metadata, metadata
|
||||
assert metadata["externalId"] == "ext-v2", metadata
|
||||
assert snapshot["displayName"] == put_alias and snapshot["externalId"] == "ext-v2", snapshot
|
||||
assert team_alias(gateway, team) == put_alias
|
||||
assert member_ids(gateway, team) == {member}
|
||||
|
||||
|
||||
def test_group_patch_drops_the_empty_key_left_by_an_earlier_push(gateway: Gateway) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
gateway.post("/team/update", {"team_id": team, "metadata": {"": {"displayName": "stale"}, "env": "staging"}})
|
||||
assert "" in team_metadata(gateway, team)
|
||||
response: Final = patch_group(gateway, team, [pathed("replace", "externalId", "ext-after")])
|
||||
assert response.status_code == 200, response.text
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert "" not in metadata, metadata
|
||||
assert metadata["env"] == "staging" and metadata["externalId"] == "ext-after", metadata
|
||||
|
||||
|
||||
def test_later_path_op_wins_over_the_pathless_value(gateway: Gateway) -> None:
|
||||
first: Final = f"first-{uuid.uuid4().hex}"
|
||||
second: Final = f"second-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
response: Final = patch_group(
|
||||
gateway, team, [pathless("replace", {"displayName": first}), pathed("replace", "displayName", second)]
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
assert response.json()["displayName"] == second, response.text
|
||||
assert team_alias(gateway, team) == second
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert "" not in metadata, metadata
|
||||
assert scim_data(metadata)["displayName"] == second, metadata
|
||||
|
||||
|
||||
def test_read_only_attributes_do_not_become_metadata_keys(gateway: Gateway) -> None:
|
||||
renamed: Final = f"readonly-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
response: Final = patch_group(
|
||||
gateway,
|
||||
team,
|
||||
[
|
||||
pathless(
|
||||
"replace",
|
||||
{
|
||||
"id": team,
|
||||
"schemas": [GROUP_SCHEMA],
|
||||
"meta": {"resourceType": "Group"},
|
||||
"displayName": renamed,
|
||||
},
|
||||
)
|
||||
],
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert set(metadata) == {"scim_data", "scim_managed"}, metadata
|
||||
assert team_alias(gateway, team) == renamed
|
||||
|
||||
|
||||
def test_pathless_rename_is_visible_from_the_peer_proxy(gateway: Gateway, peer: Gateway) -> None:
|
||||
renamed: Final = f"peer-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
response: Final = patch_group(gateway, team, [pathless("replace", {"displayName": renamed})])
|
||||
assert response.status_code == 200, response.text
|
||||
group: Final = eventually(
|
||||
lambda: peer.get(f"/scim/v2/Groups/{team}"), lambda observed: observed.get("displayName") == renamed
|
||||
)
|
||||
assert group["displayName"] == renamed, group
|
||||
assert team_alias(peer, team) == renamed
|
||||
assert "" not in team_metadata(peer, team)
|
||||
|
||||
|
||||
def test_pathless_remove_is_rejected(gateway: Gateway) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
before: Final = alias_and_metadata(gateway, team)
|
||||
response: Final = patch_group(gateway, team, [pathless("remove", {"externalId": "ext-gone"})])
|
||||
assert response.status_code == 400, response.text
|
||||
assert "RFC 7644 Section 3.5.2.2" in response.text, response.text
|
||||
assert alias_and_metadata(gateway, team) == before
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"operation",
|
||||
[
|
||||
{"op": "replace", "value": "new-name"},
|
||||
{"op": "replace", "value": 7},
|
||||
{"op": "replace", "value": ["new-name"]},
|
||||
{"op": "replace", "value": ""},
|
||||
{"op": "replace", "value": "x" * 5120},
|
||||
{"op": "replace", "value": None},
|
||||
{"op": "replace"},
|
||||
{"op": "add", "value": "new-name"},
|
||||
],
|
||||
ids=["string", "int", "list", "empty-string", "5kb-string", "null", "missing", "add-string"],
|
||||
)
|
||||
def test_pathless_op_without_an_object_value_is_rejected(gateway: Gateway, operation: dict[str, JsonValue]) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
before: Final = alias_and_metadata(gateway, team)
|
||||
response: Final = patch_group(gateway, team, [operation])
|
||||
assert response.status_code == 400, response.text
|
||||
assert "RFC 7644 Section 3.5.2" in response.text, response.text
|
||||
assert alias_and_metadata(gateway, team) == before
|
||||
|
||||
|
||||
def test_pathless_empty_object_changes_nothing_but_marks_the_team(gateway: Gateway) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
alias: Final = team_alias(gateway, team)
|
||||
response: Final = patch_group(gateway, team, [pathless("replace", {})])
|
||||
assert response.status_code == 200, response.text
|
||||
assert team_alias(gateway, team) == alias
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert metadata == {"scim_managed": True, "scim_data": {}}, metadata
|
||||
|
||||
|
||||
def test_duplicate_pathless_ops_are_idempotent(gateway: Gateway) -> None:
|
||||
external: Final = f"ext-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
response: Final = patch_group(gateway, team, [pathless("add", {"externalId": external})] * 2)
|
||||
assert response.status_code == 200, response.text
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert metadata == {"scim_managed": True, "externalId": external, "scim_data": {"externalId": external}}, (
|
||||
metadata
|
||||
)
|
||||
|
||||
|
||||
def test_unauthenticated_patch_changes_nothing(gateway: Gateway) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
before: Final = alias_and_metadata(gateway, team)
|
||||
response: Final = patch_group(
|
||||
gateway, team, [pathless("replace", {"displayName": "intruder"})], key=f"sk-{uuid.uuid4().hex}"
|
||||
)
|
||||
assert response.status_code == 401, response.text
|
||||
assert alias_and_metadata(gateway, team) == before
|
||||
|
||||
|
||||
def test_unknown_group_is_404(gateway: Gateway) -> None:
|
||||
response: Final = patch_group(
|
||||
gateway, f"missing-{uuid.uuid4().hex}", [pathless("replace", {"displayName": "ghost"})]
|
||||
)
|
||||
assert response.status_code == 404, response.text
|
||||
|
||||
|
||||
def test_malformed_patch_body_is_rejected(gateway: Gateway) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
before: Final = alias_and_metadata(gateway, team)
|
||||
response: Final = gateway.request(
|
||||
"PATCH", f"/scim/v2/Groups/{team}", {"schemas": [PATCH_OP_SCHEMA], "Operations": "nope"}
|
||||
)
|
||||
assert response.status_code in (400, 422), response.text
|
||||
assert alias_and_metadata(gateway, team) == before
|
||||
|
||||
|
||||
def test_pathless_and_pathed_scalar_coercion_agree(gateway: Gateway) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
via_pathless: Final = scenario.team()
|
||||
via_path: Final = scenario.team()
|
||||
first: Final = patch_group(gateway, via_pathless, [pathless("replace", {"displayName": 7})])
|
||||
second: Final = patch_group(gateway, via_path, [pathed("replace", "displayName", 7)])
|
||||
assert first.status_code == second.status_code == 200, (first.text, second.text)
|
||||
assert team_alias(gateway, via_pathless) == team_alias(gateway, via_path), (first.text, second.text)
|
||||
assert "" not in team_metadata(gateway, via_pathless)
|
||||
|
||||
|
||||
def _patched_state(
|
||||
candidate: Gateway, team: str, operations: Sequence[JsonValue]
|
||||
) -> tuple[JsonValue, dict[str, JsonValue]]:
|
||||
response: Final = patch_group(candidate, team, operations)
|
||||
assert response.status_code == 200, response.text
|
||||
return alias_and_metadata(candidate, team)
|
||||
|
||||
|
||||
def test_repeated_pathless_patch_is_idempotent(gateway: Gateway) -> None:
|
||||
renamed: Final = f"repeat-{uuid.uuid4().hex}"
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
operations: Final = [pathless("replace", {"displayName": renamed, "externalId": "ext-repeat"})]
|
||||
states: Final = tuple(_patched_state(gateway, team, operations) for _ in range(3))
|
||||
assert all(state == states[0] for state in states), states
|
||||
assert states[0][0] == renamed, states
|
||||
assert "" not in states[0][1], states
|
||||
|
||||
|
||||
def test_concurrent_pathless_renames_converge(gateway: Gateway, peer: Gateway) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
team: Final = scenario.team()
|
||||
aliases: Final = tuple(f"race-{index}-{uuid.uuid4().hex}" for index in range(10))
|
||||
candidates: Final = (gateway, peer)
|
||||
with ThreadPoolExecutor(max_workers=len(aliases)) as pool:
|
||||
responses: Final = tuple(
|
||||
pool.map(
|
||||
lambda indexed: patch_group(
|
||||
candidates[indexed[0] % 2], team, [pathless("replace", {"displayName": indexed[1]})]
|
||||
),
|
||||
enumerate(aliases),
|
||||
)
|
||||
)
|
||||
assert all(response.status_code == 200 for response in responses), [
|
||||
response.text for response in responses if response.status_code != 200
|
||||
]
|
||||
alias: Final = team_alias(gateway, team)
|
||||
assert alias in aliases, alias
|
||||
metadata: Final = team_metadata(gateway, team)
|
||||
assert "" not in metadata, metadata
|
||||
assert scim_data(metadata)["displayName"] == alias, metadata
|
||||
|
||||
|
||||
def test_team_key_keeps_serving_after_the_rename(gateway: Gateway, peer: Gateway) -> None:
|
||||
with gateway.scenario() as scenario:
|
||||
model: Final = scenario.model()
|
||||
team: Final = scenario.team()
|
||||
key: Final = scenario.key(team_id=team, models=[model])
|
||||
response: Final = patch_group(
|
||||
gateway, team, [pathless("replace", {"displayName": f"serving-{uuid.uuid4().hex}"})]
|
||||
)
|
||||
assert response.status_code == 200, response.text
|
||||
eventually(lambda: model_names(peer), lambda names: model in names, seconds=60)
|
||||
for candidate, marker in ((gateway, f"primary-{uuid.uuid4().hex}"), (peer, f"peer-{uuid.uuid4().hex}")):
|
||||
completion: Final = candidate.chat(model, key=key, text=marker)
|
||||
assert completion["choices"], completion
|
||||
assert upstream_hits(gateway, marker) == 1, marker
|
||||
|
||||
|
||||
@dataclass(frozen=True, slots=True)
|
||||
class _Attempt:
|
||||
team: str
|
||||
alias: str
|
||||
response: httpx.Response | None
|
||||
|
||||
|
||||
class _KillSwitch:
|
||||
def __init__(self, after: int, action: Callable[[], None]) -> None:
|
||||
self._after: Final = after
|
||||
self._action: Final = action
|
||||
self._lock: Final = threading.Lock()
|
||||
self._responses = 0
|
||||
|
||||
def tick(self) -> None:
|
||||
with self._lock:
|
||||
self._responses += 1
|
||||
if self._responses == self._after:
|
||||
self._action()
|
||||
|
||||
|
||||
def _burst_operations(alias: str, index: int) -> Sequence[JsonValue]:
|
||||
shapes: Final = (
|
||||
[pathless("replace", {"displayName": alias, "externalId": f"ext-{index}"})],
|
||||
[pathless("add", {"displayName": alias})],
|
||||
[pathed("replace", "displayName", alias)],
|
||||
)
|
||||
return shapes[index % len(shapes)]
|
||||
|
||||
|
||||
def _attempt(candidate: Gateway, team: str, index: int, switch: _KillSwitch) -> _Attempt:
|
||||
alias: Final = f"burst-{index}-{uuid.uuid4().hex}"
|
||||
try:
|
||||
return _Attempt(team, alias, patch_group(candidate, team, _burst_operations(alias, index)))
|
||||
except httpx.TransportError:
|
||||
return _Attempt(team, alias, None)
|
||||
finally:
|
||||
switch.tick()
|
||||
|
||||
|
||||
def _team_attempts(candidate: Gateway, team: str, offset: int, switch: _KillSwitch) -> tuple[_Attempt, ...]:
|
||||
return tuple(_attempt(candidate, team, offset + index, switch) for index in range(BURST_REQUESTS_PER_TEAM))
|
||||
|
||||
|
||||
def _burst(candidate: Gateway, teams: Sequence[str], disruption: Callable[[], None]) -> tuple[_Attempt, ...]:
|
||||
switch: Final = _KillSwitch(KILL_AFTER_RESPONSES, disruption)
|
||||
with ThreadPoolExecutor(max_workers=len(teams)) as pool:
|
||||
per_team: Final = tuple(
|
||||
pool.submit(_team_attempts, candidate, team, index * BURST_REQUESTS_PER_TEAM, switch)
|
||||
for index, team in enumerate(teams)
|
||||
)
|
||||
return tuple(chain.from_iterable(future.result() for future in per_team))
|
||||
|
||||
|
||||
def _burst_teams(scenario: Scenario) -> Mapping[str, str]:
|
||||
origins: Final = tuple(f"origin-{uuid.uuid4().hex}" for _ in range(BURST_TEAMS))
|
||||
return MappingProxyType({scenario.team(team_alias=origin): origin for origin in origins})
|
||||
|
||||
|
||||
def _assert_team_reflected(candidate: Gateway, team: str, origin: str, sent: Sequence[_Attempt]) -> None:
|
||||
aliases: Final = frozenset(attempt.alias for attempt in sent)
|
||||
alias, metadata = alias_and_metadata(candidate, team)
|
||||
assert "" not in metadata, metadata
|
||||
if alias == origin:
|
||||
assert all(attempt.response is None for attempt in sent), (team, sent)
|
||||
assert "scim_managed" not in metadata, metadata
|
||||
return
|
||||
assert alias in aliases, (alias, aliases)
|
||||
assert metadata["scim_managed"] is True, metadata
|
||||
if sent[-1].response is not None:
|
||||
assert alias == sent[-1].alias, (alias, sent[-1].alias)
|
||||
snapshot: Final = metadata.get("scim_data")
|
||||
if snapshot is not None:
|
||||
assert object_value(snapshot).get("displayName") in aliases, snapshot
|
||||
|
||||
|
||||
def _assert_burst_reflected(candidate: Gateway, teams: Mapping[str, str], attempts: Sequence[_Attempt]) -> None:
|
||||
answered: Final = tuple(attempt for attempt in attempts if attempt.response is not None)
|
||||
assert answered, "The whole burst failed to reach the proxy"
|
||||
for attempt in answered:
|
||||
assert attempt.response is not None and attempt.response.status_code == 200, attempt.response
|
||||
assert attempt.response.json()["displayName"] == attempt.alias, attempt.response.text
|
||||
for team, origin in teams.items():
|
||||
_assert_team_reflected(candidate, team, origin, tuple(attempt for attempt in attempts if attempt.team == team))
|
||||
|
||||
|
||||
def _patch_status(candidate: Gateway, team: str) -> int | None:
|
||||
operations: Final = [pathless("replace", {"displayName": f"probe-{uuid.uuid4().hex}"})]
|
||||
try:
|
||||
return patch_group(candidate, team, operations).status_code
|
||||
except httpx.TransportError:
|
||||
return None
|
||||
|
||||
|
||||
def _serving_workers(root: int) -> tuple[psutil.Process, ...]:
|
||||
return tuple(child for child in psutil.Process(root).children() if child.children())
|
||||
|
||||
|
||||
@pytest.mark.timeout(240)
|
||||
def test_worker_kill_mid_burst_keeps_serving_and_leaves_no_empty_key(gateway: Gateway, tmp_path: Path) -> None:
|
||||
with owned_proxy_process(gateway, tmp_path, {}, workers=2) as owned, gateway.scenario() as scenario:
|
||||
teams: Final = _burst_teams(scenario)
|
||||
workers: Final = eventually(
|
||||
lambda: _serving_workers(owned.process.pid), lambda children: len(children) >= 2, seconds=30
|
||||
)
|
||||
attempts: Final = _burst(owned.gateway, tuple(teams), lambda: os.kill(workers[0].pid, signal.SIGKILL))
|
||||
assert not workers[0].is_running() or workers[0].status() == psutil.STATUS_ZOMBIE, workers[0]
|
||||
probe: Final = scenario.team()
|
||||
eventually(lambda: _patch_status(owned.gateway, probe), lambda status_code: status_code == 200, seconds=60)
|
||||
_assert_burst_reflected(owned.gateway, teams, attempts)
|
||||
|
||||
|
||||
@pytest.mark.timeout(300)
|
||||
def test_rolling_restart_mid_burst_drains_without_empty_keys(gateway: Gateway, tmp_path: Path) -> None:
|
||||
with gateway.scenario() as scenario, owned_proxy_process(gateway, tmp_path, {}, workers=2) as replacement:
|
||||
teams: Final = _burst_teams(scenario)
|
||||
with owned_proxy_process(gateway, tmp_path, {}, workers=2) as retiring:
|
||||
attempts: Final = _burst(retiring.gateway, tuple(teams), retiring.process.terminate)
|
||||
_assert_burst_reflected(replacement.gateway, teams, attempts)
|
||||
|
|
@ -6167,3 +6167,203 @@ async def test_merge_placeholder_refuses_rows_that_are_not_a_lone_placeholder(
|
|||
assert reason in str(exc_info.value.message)
|
||||
team_member_add_mock.assert_not_awaited()
|
||||
prisma_client.db.litellm_usertable.delete.assert_not_awaited()
|
||||
|
||||
|
||||
class _PatchedTeamRow:
|
||||
def __init__(self, team: LiteLLM_TeamTable) -> None:
|
||||
self.team = team
|
||||
self.written: dict[str, object] = {}
|
||||
|
||||
async def find_unique(self, *, where: dict[str, object]) -> LiteLLM_TeamTable:
|
||||
return self.team
|
||||
|
||||
async def update(self, *, where: dict[str, object], data: dict[str, object]) -> LiteLLM_TeamTable:
|
||||
self.written = data
|
||||
self.team = LiteLLM_TeamTable(**{**self.team.model_dump(), **data, "metadata": json.loads(str(data["metadata"]))})
|
||||
return self.team
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_patch_group_pathless_replace_applies_attributes_and_drops_empty_key(mocker, monkeypatch):
|
||||
"""Okta Push Groups renames a group with a path-less ``replace`` whose value is a
|
||||
partial Group resource. Each attribute must apply as if sent with its own path and
|
||||
the resource must land in the ``scim_data`` snapshot, never whole under an empty
|
||||
metadata key, and an empty key an earlier push left behind must be dropped so the
|
||||
team saves from the Admin UI again."""
|
||||
from litellm.proxy import proxy_server
|
||||
|
||||
group_id = "team-1"
|
||||
existing_team = LiteLLM_TeamTable(
|
||||
team_id=group_id,
|
||||
team_alias="okta-push-group",
|
||||
members=[],
|
||||
members_with_roles=[Member(user_id="user1", role="user")],
|
||||
metadata={
|
||||
"": {"id": group_id, "displayName": "okta-push-group-stale"},
|
||||
"scim_managed": True,
|
||||
"scim_data": {"id": group_id, "displayName": "okta-push-group", "externalId": "ext-1"},
|
||||
},
|
||||
)
|
||||
patch_ops = SCIMPatchOp(
|
||||
schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
|
||||
Operations=[
|
||||
SCIMPatchOperation(
|
||||
op="replace",
|
||||
value={"id": group_id, "displayName": "okta-push-group-renamed", "externalId": "ext-2"},
|
||||
)
|
||||
],
|
||||
)
|
||||
|
||||
team_rows = _PatchedTeamRow(existing_team)
|
||||
mock_prisma_client = mocker.MagicMock()
|
||||
mock_prisma_client.db = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_teamtable = team_rows
|
||||
mock_prisma_client.db.litellm_usertable = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_usertable.find_unique = AsyncMock(return_value=mocker.MagicMock())
|
||||
mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=())
|
||||
|
||||
monkeypatch.setattr(proxy_server, "prisma_client", mock_prisma_client)
|
||||
mocker.patch("litellm.proxy.management_endpoints.scim.scim_v2.patch_team_membership", AsyncMock())
|
||||
mocker.patch("litellm.proxy.management_endpoints.scim.scim_v2._recompute_scim_member_roles", AsyncMock())
|
||||
|
||||
response = await patch_group(group_id=group_id, patch_ops=patch_ops)
|
||||
|
||||
assert response.id == group_id
|
||||
assert response.displayName == "okta-push-group-renamed"
|
||||
written = team_rows.written
|
||||
assert written["team_alias"] == "okta-push-group-renamed"
|
||||
written_metadata = json.loads(written["metadata"])
|
||||
assert "" not in written_metadata
|
||||
assert written_metadata["externalId"] == "ext-2"
|
||||
assert written_metadata["scim_data"] == {
|
||||
"id": group_id,
|
||||
"displayName": "okta-push-group-renamed",
|
||||
"externalId": "ext-2",
|
||||
}
|
||||
assert written_metadata["scim_managed"] is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_process_group_patch_operations_pathless_replace_members_is_absolute(mocker, monkeypatch):
|
||||
"""A path-less ``replace`` carrying ``members`` declares the whole roster exactly like
|
||||
``replace`` with path ``members``, so it must be reported as the replace target, and the
|
||||
read-only ``id`` it carries must never become a metadata key."""
|
||||
|
||||
async def mock_get_config():
|
||||
return {"litellm_settings": {"scim_upsert_user": True}}
|
||||
|
||||
from litellm.proxy.proxy_server import proxy_config
|
||||
|
||||
monkeypatch.setattr(proxy_config, "get_config", mock_get_config)
|
||||
|
||||
existing_team = LiteLLM_TeamTable(
|
||||
team_id="team-1",
|
||||
team_alias="Team One",
|
||||
members=[],
|
||||
members_with_roles=[Member(user_id="old-user", role="user")],
|
||||
)
|
||||
patch_ops = SCIMPatchOp(
|
||||
schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
|
||||
Operations=[SCIMPatchOperation(op="replace", value={"id": "team-1", "members": [{"value": "new-user"}]})],
|
||||
)
|
||||
|
||||
mock_prisma_client = mocker.MagicMock()
|
||||
mock_prisma_client.db = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_usertable = mocker.MagicMock()
|
||||
mock_prisma_client.db.litellm_usertable.find_many = AsyncMock(return_value=(mocker.MagicMock(user_id="new-user"),))
|
||||
|
||||
update_data, final_members, replace_target = await _process_group_patch_operations(
|
||||
patch_ops=patch_ops,
|
||||
existing_team=existing_team,
|
||||
prisma_client=mock_prisma_client,
|
||||
)
|
||||
|
||||
assert final_members == {"new-user"}
|
||||
assert replace_target == {"new-user"}
|
||||
assert "id" not in update_data["metadata"]
|
||||
assert "" not in update_data["metadata"]
|
||||
assert update_data["metadata"]["scim_data"] == {"id": "team-1"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("later_op", "expected_alias", "expected_external_id", "expected_snapshot"),
|
||||
[
|
||||
(
|
||||
SCIMPatchOperation(op="replace", path="displayName", value="path-wins"),
|
||||
"path-wins",
|
||||
"ext-pathless",
|
||||
{"id": "team-1", "displayName": "path-wins", "externalId": "ext-pathless"},
|
||||
),
|
||||
(
|
||||
SCIMPatchOperation(op="remove", path="displayName"),
|
||||
None,
|
||||
"ext-pathless",
|
||||
{"id": "team-1", "externalId": "ext-pathless"},
|
||||
),
|
||||
(
|
||||
SCIMPatchOperation(op="replace", path="externalId", value="ext-path-wins"),
|
||||
"pathless-name",
|
||||
"ext-path-wins",
|
||||
{"id": "team-1", "displayName": "pathless-name", "externalId": "ext-path-wins"},
|
||||
),
|
||||
],
|
||||
)
|
||||
async def test_process_group_patch_operations_later_path_op_wins_over_pathless_snapshot(
|
||||
mocker, later_op, expected_alias, expected_external_id, expected_snapshot
|
||||
):
|
||||
"""Operations apply in order (RFC 7644 Section 3.5.2), so a path op after a path-less one
|
||||
decides both the team's value and the ``scim_data`` snapshot; the snapshot must never keep
|
||||
the path-less value the later op replaced or removed."""
|
||||
existing_team = LiteLLM_TeamTable(
|
||||
team_id="team-1",
|
||||
team_alias="Team One",
|
||||
members=[],
|
||||
members_with_roles=[],
|
||||
metadata={"scim_managed": True, "scim_data": {"id": "team-1", "displayName": "Team One"}},
|
||||
)
|
||||
patch_ops = SCIMPatchOp(
|
||||
schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
|
||||
Operations=[
|
||||
SCIMPatchOperation(
|
||||
op="replace",
|
||||
value={"id": "team-1", "displayName": "pathless-name", "externalId": "ext-pathless"},
|
||||
),
|
||||
later_op,
|
||||
],
|
||||
)
|
||||
|
||||
update_data, _, _ = await _process_group_patch_operations(
|
||||
patch_ops=patch_ops,
|
||||
existing_team=existing_team,
|
||||
prisma_client=mocker.MagicMock(),
|
||||
)
|
||||
|
||||
assert update_data["team_alias"] == expected_alias
|
||||
assert update_data["metadata"].get("externalId") == expected_external_id
|
||||
assert update_data["metadata"]["scim_data"] == expected_snapshot
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
("op", "value"),
|
||||
[("remove", {"displayName": "okta-push-group"}), ("replace", "okta-push-group-renamed")],
|
||||
)
|
||||
async def test_process_group_patch_operations_rejects_pathless_op_it_cannot_apply(mocker, op, value):
|
||||
"""A path-less ``remove`` has no target and a path-less ``add``/``replace`` needs an
|
||||
object value (RFC 7644 Section 3.5.2); neither may fall through to a metadata write
|
||||
under an empty key."""
|
||||
existing_team = LiteLLM_TeamTable(team_id="team-1", team_alias="Team One", members=[], members_with_roles=[])
|
||||
patch_ops = SCIMPatchOp(
|
||||
schemas=["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
|
||||
Operations=[SCIMPatchOperation(op=op, value=value)],
|
||||
)
|
||||
|
||||
with pytest.raises(HTTPException) as exc:
|
||||
await _process_group_patch_operations(
|
||||
patch_ops=patch_ops,
|
||||
existing_team=existing_team,
|
||||
prisma_client=mocker.MagicMock(),
|
||||
)
|
||||
|
||||
assert exc.value.status_code == 400
|
||||
|
|
|
|||
|
|
@ -52,6 +52,12 @@ describe("metadataObjectToPairs", () => {
|
|||
metadataObjectToPairs({ department: "research", logging: [{ callback_name: "langfuse" }] }, new Set(["logging"])),
|
||||
).toEqual([{ key: "department", value: "research" }]);
|
||||
});
|
||||
|
||||
it("drops an empty key, which the form can never submit, so an API-written one does not block saving", () => {
|
||||
expect(metadataObjectToPairs({ "": { displayName: "okta-push-group" }, scim_managed: true })).toEqual([
|
||||
{ key: "scim_managed", value: "true" },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("metadataPairsToObject", () => {
|
||||
|
|
|
|||
|
|
@ -56,7 +56,7 @@ export function metadataObjectToPairs(
|
|||
excludedKeys: ReadonlySet<string> = new Set(),
|
||||
): MetadataPair[] {
|
||||
return Object.entries(metadata ?? {})
|
||||
.filter(([key]) => !excludedKeys.has(key))
|
||||
.filter(([key]) => key !== "" && !excludedKeys.has(key))
|
||||
.map(([key, value]) => ({ key, value: formatMetadataValue(value) }));
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue