From 53d745a4b62b3be64a0fdeaaf32ce5904c2eadcd Mon Sep 17 00:00:00 2001 From: Yucheng Zhu Date: Tue, 28 Jul 2026 10:35:44 -0700 Subject: [PATCH] chore(otel/v2): sweep remaining tenant-surface leftovers after admin-only refactor Full-diff audit against the admin-only design surfaced leftovers in three layers. Backend: drop dead code the refactor stranded (is_admin_gated_credential_info, is_destination_visible, the CredentialInfo decider-era fields, the unused LLMCallEvent.dynamic_params carrier, the _is_user_org_admin_for_org_id extraction) and correct every comment/docstring still describing the removed team-admin self-service or scoped-read designs. UI: gate the Logging Exporters form rows behind a shared proxy-admin-only LoggingExportersFormItem so non-admin forms no longer render an orphaned label; skip the GET /credentials fetch for roles it would 403 (team/org/key views); make the callbacks table read-only for the admin viewer (no Add/Edit/Delete actions that would 401); reword access tooltips to routing-scope semantics; regenerate schema.d.ts from the corrected endpoint docstrings. Tests: flip the credential-migration non-admin expectation to the route-gate 401, port the visibility tests to access_grants, drop tests of the removed helpers, and update copy assertions and stale rationale to the admin-only contract. --- litellm/integrations/otel/model/metadata.py | 5 -- litellm/models/credentials.py | 20 +++--- .../proxy/credential_endpoints/endpoints.py | 16 ++--- .../management_endpoints/common_utils.py | 29 ++++----- .../logging_exporter_access.py | 51 +++++---------- .../logging_exporter_validation.py | 11 ---- .../management_endpoints/team_endpoints.py | 6 +- .../test_credential_migration_endpoint.py | 11 ++-- .../proxy/auth/test_route_checks.py | 2 +- .../credential_endpoints/test_endpoints.py | 30 +++------ .../test_logging_exporter_access.py | 63 +++++++++---------- .../test_logging_exporter_validation.py | 21 +------ .../LoggingCallbacksTable.tsx | 21 ++++--- ui/litellm-dashboard/src/components/Teams.tsx | 10 +-- .../AccessControlFields.tsx | 6 +- .../LoggingExportersSelect.test.tsx | 14 ++--- .../LoggingExportersSelect.tsx | 27 +++++++- .../org-create/OrgCreateDialog.tsx | 22 ++++--- .../org-settings/OrgSettingsForm.tsx | 22 ++++--- .../organization/organization_view.tsx | 5 +- .../src/components/settings.test.tsx | 2 +- .../src/components/settings.tsx | 10 ++- .../src/components/team/TeamInfo.tsx | 17 ++--- .../components/templates/key_edit_view.tsx | 6 +- .../components/templates/key_info_view.tsx | 9 ++- ui/litellm-dashboard/src/lib/http/schema.d.ts | 25 +++----- ui/litellm-dashboard/src/utils/roles.ts | 5 ++ 27 files changed, 207 insertions(+), 259 deletions(-) diff --git a/litellm/integrations/otel/model/metadata.py b/litellm/integrations/otel/model/metadata.py index 68d4cf5186b..5198c0ea5f3 100644 --- a/litellm/integrations/otel/model/metadata.py +++ b/litellm/integrations/otel/model/metadata.py @@ -194,9 +194,6 @@ class LLMCallEvent: # at ``pre_call``, or when the call closed before any payload materialized (so # there is nothing to stamp on the span). payload: "StandardLoggingPayload | None" - # The ``standard_callback_dynamic_params`` routing the call to a per-tenant - # tracer (its own exporter/endpoint), or ``None`` when the call isn't scoped. - dynamic_params: Any # The admin-resolved OTLP destinations (endpoint + auth headers) for this call's # identity chain, fanned out to. Empty when none are assigned. Read from the # server-only request ContextVar the proxy anchors at auth time, so it is never @@ -217,11 +214,9 @@ class LLMCallEvent: payload = cast("StandardLoggingPayload", raw_payload) if raw_payload else None operation = resolve_operation(as_str(kwargs.get("call_type"))) model = as_str(kwargs.get("model")) or "" - dynamic_params = kwargs.get("standard_callback_dynamic_params") return cls( call_id=_call_id(payload, kwargs), payload=payload, - dynamic_params=dynamic_params, otel_destinations=request_destinations(), is_no_upstream_call=bool(kwargs.get(LITELLM_LOGGING_NO_UPSTREAM_LLM_CALL)), provisional_span_name=f"{operation.value} {model}".strip(), diff --git a/litellm/models/credentials.py b/litellm/models/credentials.py index 7c3cfef3265..5f1c69f35d3 100644 --- a/litellm/models/credentials.py +++ b/litellm/models/credentials.py @@ -32,10 +32,10 @@ class CreateCredentialItem(CredentialBase): class UpdateCredentialItem(BaseModel): """PATCH body for ``/credentials/{name}``. - Both ``credential_values`` and ``credential_info`` are optional so a caller - can patch one without sending the other (team-admins patching access without - knowing the upstream secrets; proxy admins rotating values without touching - access). ``credential_name`` is optional because most patches don't rename. + Both ``credential_values`` and ``credential_info`` are optional so the proxy + admin can patch one without sending the other (rotating values without + touching access, or adjusting access without re-sending secrets). + ``credential_name`` is optional because most patches don't rename. """ credential_name: str | None = None @@ -59,19 +59,17 @@ class CredentialAccess(BaseModel): class CredentialInfo(BaseModel): - """Typed shape of ``credential_info`` for the access-control decider. + """Typed shape of ``credential_info`` as read by the request-time resolver. Existing stored credentials carry arbitrary extra fields (e.g. - ``custom_llm_provider``); ``extra="allow"`` preserves them. The decider - inspects ``model_fields_set`` to learn which fields the caller actually - patched, which is what Pydantic gives us natively without dict-key spelunking. + ``custom_llm_provider``); ``extra="allow"`` preserves them. Only the fields + the resolver consumes are typed: ``credential_type`` selects logging + destinations, and ``access``/``auto_enable`` decide which identities the + destination fires for. """ model_config = ConfigDict(extra="allow") credential_type: str | None = None - description: str | None = None - host: str | None = None - endpoint: str | None = None access: CredentialAccess | None = None auto_enable: bool = False diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index ca171d86925..dd27c8040fd 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -32,7 +32,7 @@ def _require_proxy_admin(user_api_key_dict: UserAPIKeyAuth) -> None: if user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN: raise HTTPException( status_code=403, - detail={"error": "Only the proxy admin can manage logging credentials"}, + detail={"error": "Only the proxy admin can manage credentials"}, ) @@ -271,8 +271,6 @@ async def delete_credential( """ from litellm.proxy.proxy_server import prisma_client - # DELETE stays proxy-admin only. The route gate lets team-admins reach - # /credentials/{name} for PATCH; reject any DELETE that isn't proxy-admin. _require_proxy_admin(user_api_key_dict) try: @@ -334,14 +332,12 @@ def update_db_credential( def _merge_credential_info(into: dict, patch: dict) -> None: """Merge ``patch`` into ``into`` in place, with surgical access subfields. - A prior top-level dict.update let a patch like ``{access: {teams: [...]}}`` + A top-level dict.update would let a patch like ``{access: {teams: [...]}}`` replace the entire stored ``access`` object, wiping ``access.global=true`` - and ``access.orgs`` entries that the decider intentionally protected by - refusing to allow them in the patch (Veria F1: scope tampering). Now - ``access`` is merged subfield-by-subfield, so a non-admin patch carrying - only ``access.teams`` keeps existing ``access.global`` / ``access.orgs`` - intact. The DB write and the in-memory cache sync both call this so the - two stores can't drift. + and ``access.orgs`` entries the caller never mentioned. ``access`` is merged + subfield-by-subfield instead, so a partial patch keeps the untouched + subfields intact. The DB write and the in-memory cache sync both call this + so the two stores can't drift. """ patch_copy = dict(patch) patch_access = patch_copy.pop("access", None) diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index 2fe2203bf4c..8162babef40 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -111,13 +111,15 @@ def _is_user_team_admin(user_api_key_dict: UserAPIKeyAuth, team_obj: LiteLLM_Tea return False -async def _is_user_org_admin_for_org_id(user_api_key_dict: UserAPIKeyAuth, organization_id: str | None) -> bool: - """Check if the caller has the ORG_ADMIN role in the given organization. - - Returns False when ``organization_id`` is falsy or the caller has no user_id, - so the caller can pass an optional org_id directly without branching. +async def _is_user_org_admin_for_team(user_api_key_dict: UserAPIKeyAuth, team_obj: LiteLLM_TeamTable) -> bool: """ - if not organization_id or not user_api_key_dict.user_id: + Check if user is an org admin for the team's organization. + + Returns True if: + - The team belongs to an organization, AND + - The user has org_admin role in that organization + """ + if not team_obj.organization_id or not user_api_key_dict.user_id: return False from litellm.proxy.auth.auth_checks import get_user_object @@ -137,18 +139,11 @@ async def _is_user_org_admin_for_org_id(user_api_key_dict: UserAPIKeyAuth, organ if caller_user is None: return False - return any( - m.organization_id == organization_id and m.user_role == LitellmUserRoles.ORG_ADMIN.value - for m in (caller_user.organization_memberships or []) - ) + for m in caller_user.organization_memberships or []: + if m.organization_id == team_obj.organization_id and m.user_role == LitellmUserRoles.ORG_ADMIN.value: + return True - -async def _is_user_org_admin_for_team(user_api_key_dict: UserAPIKeyAuth, team_obj: LiteLLM_TeamTable) -> bool: - """Check if user is an org admin for the team's organization.""" - return await _is_user_org_admin_for_org_id( - user_api_key_dict=user_api_key_dict, - organization_id=team_obj.organization_id, - ) + return False def _team_member_has_permission( diff --git a/litellm/proxy/management_endpoints/logging_exporter_access.py b/litellm/proxy/management_endpoints/logging_exporter_access.py index 80af7b8a869..6c029034a1f 100644 --- a/litellm/proxy/management_endpoints/logging_exporter_access.py +++ b/litellm/proxy/management_endpoints/logging_exporter_access.py @@ -1,18 +1,14 @@ -"""Shared visibility predicate for admin-owned logging destinations. +"""Request-time routing predicate for admin-owned logging destinations. -``credential_info.access`` answers "who may see and assign this destination". It -is visibility, decoupled from enablement (a named assignment plus the explicit -``auto_enable`` default-on flag). One predicate serves three callers so "visible" -means the same thing everywhere: the ``GET /credentials`` list filter, the -assignment validator, and the request-time resolver. When these disagree a -non-admin can be shown, or route traffic to, a destination it should never see; -keeping the check in one place is what prevents that. +``credential_info.access`` answers "which identities' traces may this destination +receive". It is routing scope, decoupled from enablement (a named assignment plus +the explicit ``auto_enable`` default-on flag). The request-time resolver in +``litellm_pre_call_utils`` is the consumer: at call time it checks whether the +request's team/org is granted before firing the destination. -``access_grants`` is the primitive: does this ``access`` reach a caller whose -admin scope is the given set of team ids and org ids. Single-identity callers -(the resolver, the per-write assignment gate) pass a one-element scope built with -``identity_scope``; the list endpoint, whose caller may administer several teams -and orgs, passes the full scope. +``access_grants`` is the primitive: does this ``access`` reach an identity whose +scope is the given set of team ids and org ids. The resolver passes a +one-element scope built with ``identity_scope``. """ from pydantic import ValidationError @@ -25,8 +21,8 @@ def parse_credential_info(raw: object) -> CredentialInfo | None: absent or malformed. Callers fail closed on ``None``: a destination whose stored ``access`` cannot be - parsed (a legacy shape the strict read model rejects) is treated as invisible - rather than granted to everyone. + parsed (a legacy shape the strict read model rejects) is treated as granted to + no one rather than granted to everyone. """ if not isinstance(raw, dict): return None @@ -37,8 +33,8 @@ def parse_credential_info(raw: object) -> CredentialInfo | None: def identity_scope(team_id: str | None, org_id: str | None) -> tuple[frozenset[str], frozenset[str]]: - """A single request identity's admin scope as ``(team_ids, org_ids)`` for - ``access_grants`` / ``is_destination_visible``.""" + """A single request identity's scope as ``(team_ids, org_ids)`` for + ``access_grants``.""" return ( frozenset({team_id}) if team_id else frozenset(), frozenset({org_id}) if org_id else frozenset(), @@ -50,11 +46,11 @@ def access_grants( team_ids: frozenset[str], org_ids: frozenset[str], ) -> bool: - """Whether ``access`` makes a destination visible to a caller admin-scoped to + """Whether ``access`` grants a destination to an identity scoped to ``team_ids`` / ``org_ids``. - ``global`` reaches everyone; otherwise one of the caller's admin teams or orgs - must be granted. A missing ``access`` grants no one (fail closed): visibility is + ``global`` reaches everyone; otherwise one of the identity's teams or orgs + must be granted. A missing ``access`` grants no one (fail closed): routing is an explicit admin grant, never the accident of an absent field. """ if access is None: @@ -64,18 +60,3 @@ def access_grants( if not team_ids.isdisjoint(access.teams): return True return not org_ids.isdisjoint(access.orgs) - - -def is_destination_visible( - info: CredentialInfo, - team_ids: frozenset[str], - org_ids: frozenset[str], -) -> bool: - """Whether a caller admin-scoped to ``team_ids`` / ``org_ids`` may see and assign - this destination. - - Visibility is decided entirely by ``access``: empty access grants no one, so an - empty-access destination is invisible regardless of ``auto_enable``. Proxy-wide - visibility must be requested explicitly with ``access.global = true``. - """ - return access_grants(info.access, team_ids, org_ids) diff --git a/litellm/proxy/management_endpoints/logging_exporter_validation.py b/litellm/proxy/management_endpoints/logging_exporter_validation.py index 42f9d9f9152..5ffc9d6643b 100644 --- a/litellm/proxy/management_endpoints/logging_exporter_validation.py +++ b/litellm/proxy/management_endpoints/logging_exporter_validation.py @@ -15,17 +15,6 @@ from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth LOGGING_EXPORTERS_KEY = "logging_exporters" -def is_admin_gated_credential_info(credential_info: dict | None) -> bool: - """Whether a credential write must be proxy-admin only. - - True when the credential is a logging destination or carries an ``access`` grant, - since both control where other tenants' traces are exported. - """ - if not isinstance(credential_info, dict): - return False - return credential_info.get("credential_type") == "logging" or "access" in credential_info - - def validate_credential_access(credential_info: dict | None) -> None: """Validate ``credential_info.access`` shape when the write sets one. diff --git a/litellm/proxy/management_endpoints/team_endpoints.py b/litellm/proxy/management_endpoints/team_endpoints.py index 2dcc77cc7a1..92a6ceb6078 100644 --- a/litellm/proxy/management_endpoints/team_endpoints.py +++ b/litellm/proxy/management_endpoints/team_endpoints.py @@ -1019,10 +1019,8 @@ async def new_team( user_api_key_cache, ) - # New team has no admins yet, so only proxy admin or an org admin of - # the destination org may assign logging exporters at creation time. - # Skip the org-admin lookup entirely when the field isn't being - # written, to avoid hitting the cache for unrelated /team/new calls. + # logging_exporters is proxy-admin only. Skip the check when the field + # isn't being written so unrelated /team/new calls stay cheap. if data.logging_exporters is not None: validate_logging_exporter_field(data.logging_exporters, user_api_key_dict) diff --git a/tests/proxy_behavior/management/test_credential_migration_endpoint.py b/tests/proxy_behavior/management/test_credential_migration_endpoint.py index 1eb2a216152..0ae4257d76d 100644 --- a/tests/proxy_behavior/management/test_credential_migration_endpoint.py +++ b/tests/proxy_behavior/management/test_credential_migration_endpoint.py @@ -50,12 +50,9 @@ async def test_migrate_encryption_check_requires_admin(proxy_client, scratch): async def test_migrate_encryption_requires_admin(proxy_client, scratch): """A non-admin key cannot trigger the migration; it is rejected before any write. - ``/credentials/{credential_name}`` is a self-managed route (team admins reach - ``PATCH /credentials/{name}`` to grant their own team a logging destination), so - the single-segment ``/credentials/migrate-encryption`` path matches that pattern - and clears the route-level gate. The migration endpoint then rejects the - non-admin on its first line via ``_require_proxy_admin`` (403), ahead of any DB - read or write. Either way the non-admin never triggers the migration. + No ``/credentials`` route is self-managed: credentials are proxy-admin only, so + a non-admin POST to ``/credentials/migrate-encryption`` is rejected at the + route-level gate (401) before the handler or any DB read/write runs. """ gen = await proxy_client.post( "/key/generate", @@ -72,4 +69,4 @@ async def test_migrate_encryption_requires_admin(proxy_client, scratch): "/credentials/migrate-encryption", headers={"Authorization": f"Bearer {nonadmin_key}"}, ) - assert resp.status_code == 403, resp.text + assert resp.status_code == 401, resp.text diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py index a0261e98951..ad513f21ab9 100644 --- a/tests/test_litellm/proxy/auth/test_route_checks.py +++ b/tests/test_litellm/proxy/auth/test_route_checks.py @@ -3091,7 +3091,7 @@ def test_internal_user_blocked_from_search_tool_writes(route): assert "Your role=internal_user" in str(exc_info.value) -# --- Credential route gating (PR #30873: /credentials opened to self-managed) --- # +# --- Credential route gating (PR #30873: /credentials is proxy-admin only) --- # @pytest.mark.parametrize( diff --git a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py index 4fe0ec91316..5edc3d7316c 100644 --- a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py @@ -88,13 +88,7 @@ async def test_create_logging_credential_allowed_for_admin(_connected_db): @pytest.mark.asyncio async def test_create_provider_credential_forbidden_for_non_admin(_connected_db): - """POST is proxy-admin only even for provider credentials. - - The route gate now lets non-admins reach the credentials path so they can - PATCH ``access.teams`` on logging destinations they should be able to - self-assign. POST remains admin-only to keep credential creation a - platform-admin concern. - """ + """POST is proxy-admin only for provider and logging credentials alike.""" with pytest.raises(HTTPException) as exc: await endpoints.create_credential( request=MagicMock(), @@ -191,11 +185,9 @@ def test_update_db_credential_preserves_existing_info_on_partial_patch(): def test_update_db_credential_preserves_untouched_access_subfields(): - """Veria regression: an access patch carrying only `teams` must NOT clobber - existing `global` / `orgs`. Pre-fix this caused a scope-tampering bug — - the decider only allowed team-admin patches that touched access.teams, - but the merge replaced the entire access object, silently dropping - access.global=true and any access.orgs entries. + """An access patch carrying only `teams` must NOT clobber existing + `global` / `orgs`: a top-level replace of the access object would silently + drop access.global=true and any access.orgs entries the patch never named. """ from litellm.proxy.credential_endpoints.endpoints import update_db_credential @@ -213,8 +205,7 @@ def test_update_db_credential_preserves_untouched_access_subfields(): }, }, ) - # Team-admin's allowed shape: add their team to access.teams. Crucially, - # they don't (and per the decider can't) include global/orgs. + # A partial patch touching only access.teams; global/orgs are not sent. patch = CredentialItem( credential_name="dest", credential_values={}, @@ -314,19 +305,14 @@ async def test_delete_db_only_logging_credential_forbidden_for_non_admin( _connected_db.delete_by_name.assert_not_awaited() -# --- team-admin self-assign tests (LIT-3850 follow-up) ---------------------- +# --- PATCH gating (proxy-admin only) ---------------------------------------- @pytest.mark.asyncio async def test_provider_credential_patch_forbidden_for_non_admin( _connected_db, monkeypatch ): - """A team-admin (or any non-admin) cannot PATCH a non-logging credential. - - The route gate was widened to let team-admins reach /credentials/{name} - for logging-credential access edits. A provider credential is not - is_admin_gated_credential_info, so the decider block is skipped; without - an explicit else-branch a team-admin could rotate the upstream api_key. - """ + """A non-admin cannot PATCH a provider credential (or any credential): + without the gate a non-admin could rotate the upstream api_key.""" provider_cred = CredentialItem( credential_name="openai-prod", credential_values={"api_key": "sk-real"}, diff --git a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py index 4de81a42d48..7f83b3eb52d 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py +++ b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_access.py @@ -1,9 +1,8 @@ -"""The shared visibility predicate for admin-owned logging destinations. +"""The request-time routing predicate for admin-owned logging destinations. -This is the single chokepoint the list endpoint, the assignment validator, and -the request-time resolver all route through, so a mutation here would let a -non-admin see or route to a destination outside their scope. Each case is written -to fail if the corresponding branch is flipped. +``access_grants`` is the chokepoint the resolver routes through at call time, so a +mutation here would route an identity's traces to a destination outside its scope. +Each case is written to fail if the corresponding branch is flipped. """ import os @@ -15,7 +14,6 @@ from litellm.models.credentials import CredentialAccess, CredentialInfo from litellm.proxy.management_endpoints.logging_exporter_access import ( access_grants, identity_scope, - is_destination_visible, parse_credential_info, ) @@ -99,54 +97,55 @@ def test_access_grants_not_global_when_false(): assert access_grants(a, frozenset({"t1"}), frozenset({"o1"})) is False -# --- is_destination_visible: decided entirely by access -------------------- +# --- routing scope decided entirely by access ------------------------------- # -# Visibility is access-only. auto_enable does not affect it: an empty-access -# destination is invisible regardless of auto_enable (empty access = deny-all). -# Proxy-wide visibility must be requested explicitly with access.global=True. +# Routing is access-only. auto_enable does not widen it: an empty-access +# destination fires for no one regardless of auto_enable (empty access = +# deny-all). Proxy-wide routing must be requested explicitly with +# access.global=True. -def test_visible_empty_access_is_deny_all_even_with_auto_enable(): +def test_empty_access_is_deny_all_even_with_auto_enable(): """Empty access grants no one, even when auto_enable=True: not proxy-wide.""" info = CredentialInfo(credential_type="logging", auto_enable=True) - assert is_destination_visible(info, frozenset(), frozenset()) is False - assert is_destination_visible(info, frozenset({"any-team"}), frozenset()) is False - assert is_destination_visible(info, frozenset(), frozenset({"any-org"})) is False + assert access_grants(info.access, frozenset(), frozenset()) is False + assert access_grants(info.access, frozenset({"any-team"}), frozenset()) is False + assert access_grants(info.access, frozenset(), frozenset({"any-org"})) is False -def test_visible_global_access_is_proxy_wide(): +def test_global_access_is_proxy_wide(): """access.global=True is proxy-wide regardless of auto_enable.""" info = CredentialInfo(credential_type="logging", auto_enable=True, access=_access(global_=True)) - assert is_destination_visible(info, frozenset({"t1"}), frozenset()) is True - assert is_destination_visible(info, frozenset(), frozenset()) is True + assert access_grants(info.access, frozenset({"t1"}), frozenset()) is True + assert access_grants(info.access, frozenset(), frozenset()) is True manual = CredentialInfo(credential_type="logging", access=_access(global_=True)) - assert is_destination_visible(manual, frozenset(), frozenset()) is True + assert access_grants(manual.access, frozenset(), frozenset()) is True -def test_visible_auto_enable_team_scoped(): - """auto_enable=True + access.teams=[t1] is visible only to t1 admins.""" +def test_auto_enable_team_scoped(): + """auto_enable=True + access.teams=[t1] fires only for t1 identities.""" info = CredentialInfo(credential_type="logging", auto_enable=True, access=_access(teams=["t1"])) - assert is_destination_visible(info, frozenset({"t1"}), frozenset()) is True - assert is_destination_visible(info, frozenset({"t2"}), frozenset()) is False - assert is_destination_visible(info, frozenset(), frozenset()) is False + assert access_grants(info.access, frozenset({"t1"}), frozenset()) is True + assert access_grants(info.access, frozenset({"t2"}), frozenset()) is False + assert access_grants(info.access, frozenset(), frozenset()) is False -def test_visible_auto_enable_org_scoped(): - """auto_enable=True + access.orgs=[o1] is visible only to o1 admins.""" +def test_auto_enable_org_scoped(): + """auto_enable=True + access.orgs=[o1] fires only for o1 identities.""" info = CredentialInfo(credential_type="logging", auto_enable=True, access=_access(orgs=["o1"])) - assert is_destination_visible(info, frozenset(), frozenset({"o1"})) is True - assert is_destination_visible(info, frozenset(), frozenset({"o2"})) is False + assert access_grants(info.access, frozenset(), frozenset({"o1"})) is True + assert access_grants(info.access, frozenset(), frozenset({"o2"})) is False -def test_visible_delegates_to_access_when_not_auto_enable(): +def test_access_scoped_when_not_auto_enable(): info = CredentialInfo(credential_type="logging", access=_access(teams=["t1"])) - assert is_destination_visible(info, frozenset({"t1"}), frozenset()) is True - assert is_destination_visible(info, frozenset({"t2"}), frozenset()) is False + assert access_grants(info.access, frozenset({"t1"}), frozenset()) is True + assert access_grants(info.access, frozenset({"t2"}), frozenset()) is False -def test_visible_denies_when_neither(): +def test_denies_when_no_access(): info = CredentialInfo(credential_type="logging") - assert is_destination_visible(info, frozenset({"t1"}), frozenset({"o1"})) is False + assert access_grants(info.access, frozenset({"t1"}), frozenset({"o1"})) is False # --- identity_scope -------------------------------------------------------- diff --git a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_validation.py b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_validation.py index 1210efe2222..c149b613016 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_validation.py +++ b/tests/test_litellm/proxy/management_endpoints/test_logging_exporter_validation.py @@ -19,7 +19,6 @@ import litellm from litellm.models.credentials import CredentialItem from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth from litellm.proxy.management_endpoints.logging_exporter_validation import ( - is_admin_gated_credential_info, validate_credential_access, validate_logging_exporter_assignment, validate_logging_exporter_field, @@ -101,9 +100,6 @@ def test_non_admin_with_no_flags_is_forbidden(_registry): assert exc.value.status_code == 403 -# --- Scope checks: a non-proxy-admin may only name destinations granted to them - - - # --- Shape / registry checks (run regardless of who's calling) -------------- @@ -210,22 +206,7 @@ def test_omitted_on_both_sides_is_noop(_registry): ) -# --- is_admin_gated_credential_info / validate_credential_access ------------ - - -@pytest.mark.parametrize( - "credential_info, gated", - [ - ({"credential_type": "logging"}, True), - ({"access": {"global": True}}, True), - ({"credential_type": "logging", "access": {"teams": ["t"]}}, True), - ({"custom_llm_provider": "openai"}, False), - ({}, False), - (None, False), - ], -) -def test_is_admin_gated_credential_info(credential_info, gated): - assert is_admin_gated_credential_info(credential_info) is gated +# --- validate_credential_access --------------------------------------------- def test_validate_credential_access_accepts_valid_object(): diff --git a/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTable.tsx b/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTable.tsx index 6f5c2995b08..a90d8348a57 100644 --- a/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTable.tsx +++ b/ui/litellm-dashboard/src/components/Settings/LoggingAndAlerts/LoggingCallbacks/LoggingCallbacksTable.tsx @@ -23,6 +23,7 @@ type LoggingCallbacksProps = { onDelete?: (callback: AlertingObject) => void; onEditAccess?: (callback: AlertingObject) => void; onAdd?: () => void; + readOnly?: boolean; }; function EmptyState() { @@ -48,21 +49,25 @@ export const LoggingCallbacksTable: React.FC = ({ onDelete = () => {}, onEditAccess = () => {}, onAdd = () => {}, + readOnly = false, }) => { const columns = useMemo(() => { const deps = { availableCallbacks, onTest, onEdit, onDelete, onEditAccess }; - return getLoggingCallbacksTableColumns(deps); - }, [availableCallbacks, onTest, onEdit, onDelete, onEditAccess]); + const all = getLoggingCallbacksTableColumns(deps); + return readOnly ? all.filter((column) => column.id !== "actions") : all; + }, [availableCallbacks, onTest, onEdit, onDelete, onEditAccess, readOnly]); return (

Active Logging Callbacks

-
- -
+ {!readOnly && ( +
+ +
+ )} = ({ accessToken, userID, userRole, premiumUser Logging Settings - - - + />
= ({ value = {}, o <> onChange({ ...value, global })} /> { render( {}} />); expect(screen.queryAllByTestId("option")).toHaveLength(0); - expect(screen.getByTestId("empty").textContent).toMatch(/proxy admin/i); + expect(screen.getByTestId("empty").textContent).toMatch(/Create one under Settings/i); }); it("shows exactly the logging destinations the backend returned, without any client-side scope filtering", () => { - // GET /credentials is already scoped server-side (proxy admin -> all; team/org - // admin -> only in-scope destinations) by the same predicate the assignment gate - // and the resolver use. The picker must therefore render every logging-typed - // destination in the response verbatim; re-filtering here by role/scope on the - // client would risk disagreeing with the authoritative backend in either - // direction. This response mixes access shapes to prove none are dropped locally. + // GET /credentials is proxy-admin only and returns every destination; the + // picker (which itself renders only for a proxy admin) must show every + // logging-typed destination in the response verbatim regardless of its + // access shape, since access controls request-time routing, not what the + // admin may assign. This response mixes access shapes to prove none are + // dropped locally. mockUseCredentials.mockReturnValue({ data: { credentials: [ diff --git a/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.tsx b/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.tsx index d15e04d86c4..fe4a745ee02 100644 --- a/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.tsx +++ b/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.tsx @@ -1,4 +1,4 @@ -import { Select } from "antd"; +import { Form, Select } from "antd"; import React from "react"; import { useCredentials } from "@/app/(dashboard)/hooks/credentials/useCredentials"; @@ -47,9 +47,32 @@ const LoggingExportersSelect: React.FC = ({ value, options={options} style={{ width: "100%" }} optionFilterProp="label" - notFoundContent="No logging destinations available. Ask your proxy admin to create one under Settings -> Logging Callbacks." + notFoundContent="No logging destinations available. Create one under Settings -> Logging Callbacks." /> ); }; export default LoggingExportersSelect; + +interface LoggingExportersFormItemProps { + tooltip: string; + className?: string; +} + +/** + * The antd Form.Item wrapper for LoggingExportersSelect, gated to proxy admins so + * non-admin forms render neither the picker nor an orphaned "Logging Exporters" + * label. Keeps the role gate in one place for every antd form that binds the + * logging_exporters field. + */ +export const LoggingExportersFormItem: React.FC = ({ tooltip, className }) => { + const { userRole } = useAuthorized(); + if (userRole == null || !isProxyAdminRole(userRole)) { + return null; + } + return ( + + + + ); +}; diff --git a/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.tsx b/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.tsx index 33fc6c3dfb5..318ef7a69a8 100644 --- a/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.tsx +++ b/ui/litellm-dashboard/src/components/organization/org-create/OrgCreateDialog.tsx @@ -4,7 +4,9 @@ import { useMutation, useQueryClient } from "@tanstack/react-query"; import * as React from "react"; import { organizationKeys } from "@/app/(dashboard)/hooks/organizations/useOrganizations"; +import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; import LoggingExportersSelect from "@/components/logging_credentials/LoggingExportersSelect"; +import { isProxyAdminRole } from "@/utils/roles"; import { ModelSelect } from "@/components/ModelSelect/ModelSelect"; import MCPServerSelector from "@/components/mcp_server_management/MCPServerSelector"; import NotificationsManager from "@/components/molecules/notifications_manager"; @@ -43,6 +45,8 @@ export const OrgCreateDialog = ({ }: OrgCreateDialogProps) => { const queryClient = useQueryClient(); const form = useZodForm(orgSettingsSchema, { defaultValues: emptyOrgFormValues }); + const { userRole } = useAuthorized(); + const isProxyAdmin = userRole != null && isProxyAdminRole(userRole); const closeAndReset = () => { form.reset(emptyOrgFormValues); @@ -162,14 +166,16 @@ export const OrgCreateDialog = ({ )} - - {(field) => } - + {isProxyAdmin && ( + + {(field) => } + + )} {({ ref, ...field }) =>