From 9733ce580932a3f312eae22765b26facaf7cee55 Mon Sep 17 00:00:00 2001 From: yucheng-berriai Date: Thu, 2 Jul 2026 00:22:33 -0700 Subject: [PATCH] feat(ui): send logging_exporters as a typed field; picker trusts backend scoping Assignments now live on a typed logging_exporters column, so the key/team/org create and update forms send logging_exporters as a top-level field instead of packing it into the metadata JSON blob, and the info views read it off the object (via loggingExportersOf, column-first with a metadata fallback for rows written before the column existed). The destination picker no longer filters options by the caller's role client-side. GET /credentials is already scoped server-side by the same predicate the assignment gate and the resolver use, so the picker renders exactly what the backend returns; the previous isAdminRole short-circuit treated org-admins and admin-viewers as 'see every destination', which was broader than the backend's proxy-admin-only 'see all'. Regenerated schema.d.ts for the new field. --- .../src/components/OldTeams.tsx | 19 ++--- .../LoggingExportersSelect.test.tsx | 72 ++++--------------- .../LoggingExportersSelect.tsx | 39 +++------- .../logging_credentials/loggingExportersOf.ts | 18 +++++ .../organization/organization_view.tsx | 18 ++--- .../src/components/organizations.tsx | 13 ++-- .../src/components/settings.tsx | 10 +-- .../src/components/team/TeamInfo.tsx | 14 ++-- .../components/templates/key_edit_view.tsx | 5 +- .../components/templates/key_info_view.tsx | 15 +--- ui/litellm-dashboard/src/lib/http/schema.d.ts | 65 +++++++++++++---- 11 files changed, 126 insertions(+), 162 deletions(-) create mode 100644 ui/litellm-dashboard/src/components/logging_credentials/loggingExportersOf.ts diff --git a/ui/litellm-dashboard/src/components/OldTeams.tsx b/ui/litellm-dashboard/src/components/OldTeams.tsx index 4db7b79c6b8..cb42d0e2933 100644 --- a/ui/litellm-dashboard/src/components/OldTeams.tsx +++ b/ui/litellm-dashboard/src/components/OldTeams.tsx @@ -470,21 +470,12 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser formValues.metadata = JSON.stringify(metadata); } - // Merge admin-owned logging_exporters into metadata at create time so the - // user can assign destinations from the new-team form (instead of create-then-edit). - if (Array.isArray(formValues.logging_exporters) && formValues.logging_exporters.length > 0) { - let metadata: Record = {}; - if (typeof formValues.metadata === "string" && formValues.metadata.trim().length > 0) { - try { - metadata = JSON.parse(formValues.metadata); - } catch (e) { - console.warn("Invalid JSON in metadata field, starting with empty object"); - } - } - metadata = { ...metadata, logging_exporters: formValues.logging_exporters }; - formValues.metadata = JSON.stringify(metadata); + // logging_exporters is a top-level typed field on the team (its own column), + // not part of the free-form metadata blob; send it as-is when set so the user + // can assign destinations from the new-team form (instead of create-then-edit). + if (!Array.isArray(formValues.logging_exporters) || formValues.logging_exporters.length === 0) { + delete formValues.logging_exporters; } - delete formValues.logging_exporters; if (formValues.secret_manager_settings) { if (typeof formValues.secret_manager_settings === "string") { diff --git a/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.test.tsx b/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.test.tsx index a97b59c7b7a..e310949f0da 100644 --- a/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.test.tsx +++ b/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.test.tsx @@ -4,22 +4,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import LoggingExportersSelect from "./LoggingExportersSelect"; const mockUseCredentials = vi.fn(); -const mockUseAuthorized = vi.fn(); -const mockUseTeams = vi.fn(); -const mockUseOrganizations = vi.fn(); vi.mock("@/app/(dashboard)/hooks/credentials/useCredentials", () => ({ useCredentials: () => mockUseCredentials(), })); -vi.mock("@/app/(dashboard)/hooks/useAuthorized", () => ({ - default: () => mockUseAuthorized(), -})); -vi.mock("@/app/(dashboard)/hooks/teams/useTeams", () => ({ - useTeams: () => mockUseTeams(), -})); -vi.mock("@/app/(dashboard)/hooks/organizations/useOrganizations", () => ({ - useOrganizations: () => mockUseOrganizations(), -})); vi.mock("antd", async () => { const React = await import("react"); @@ -48,10 +36,7 @@ vi.mock("antd", async () => { }); beforeEach(() => { - // Default: a proxy admin (formatted role "Admin"), no team/org membership needed. - mockUseAuthorized.mockReturnValue({ userRole: "Admin" }); - mockUseTeams.mockReturnValue({ data: [] }); - mockUseOrganizations.mockReturnValue({ data: [] }); + mockUseCredentials.mockReset(); }); describe("LoggingExportersSelect", () => { @@ -99,32 +84,21 @@ describe("LoggingExportersSelect", () => { expect(screen.getByTestId("empty").textContent).toMatch(/proxy admin/i); }); - it("scopes a non-admin caller to destinations granted to their team/org (plus global/auto_enable)", () => { - // An internal_user who is a member of team-a. They must see only what they could - // actually assign: the team-a destination, the global one, and the auto_enable - // default -- never the team-b destination or the foreign-org one. This mirrors the - // backend assignment gate; the backend stays the authoritative check. - mockUseAuthorized.mockReturnValue({ userRole: "Internal User" }); - mockUseTeams.mockReturnValue({ data: [{ team_id: "team-a" }] }); - mockUseOrganizations.mockReturnValue({ data: [{ organization_id: "org-a" }] }); + 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. mockUseCredentials.mockReturnValue({ data: { credentials: [ - { - credential_name: "mine-team", - credential_info: { credential_type: "logging", access: { teams: ["team-a"] } }, - }, - { - credential_name: "foreign-team", - credential_info: { credential_type: "logging", access: { teams: ["team-b"] } }, - }, - { credential_name: "mine-org", credential_info: { credential_type: "logging", access: { orgs: ["org-a"] } } }, - { - credential_name: "foreign-org", - credential_info: { credential_type: "logging", access: { orgs: ["org-z"] } }, - }, + { credential_name: "team-scoped", credential_info: { credential_type: "logging", access: { teams: ["t"] } } }, + { credential_name: "org-scoped", credential_info: { credential_type: "logging", access: { orgs: ["o"] } } }, { credential_name: "everyone", credential_info: { credential_type: "logging", access: { global: true } } }, { credential_name: "always-on", credential_info: { credential_type: "logging", auto_enable: true } }, + { credential_name: "provider", credential_info: { custom_llm_provider: "openai" } }, ], }, }); @@ -132,27 +106,7 @@ describe("LoggingExportersSelect", () => { render( {}} />); const options = screen.getAllByTestId("option").map((el) => el.textContent); - expect(options).toEqual(["mine-team", "mine-org", "everyone", "always-on"]); - }); - - it("shows every logging destination to a proxy admin regardless of access scope", () => { - mockUseAuthorized.mockReturnValue({ userRole: "Admin" }); - mockUseTeams.mockReturnValue({ data: [] }); - mockUseCredentials.mockReturnValue({ - data: { - credentials: [ - { - credential_name: "team-b-only", - credential_info: { credential_type: "logging", access: { teams: ["team-b"] } }, - }, - { credential_name: "no-access", credential_info: { credential_type: "logging" } }, - ], - }, - }); - - render( {}} />); - - const options = screen.getAllByTestId("option").map((el) => el.textContent); - expect(options).toEqual(["team-b-only", "no-access"]); + // every logging destination the backend returned, and only those (provider dropped) + expect(options).toEqual(["team-scoped", "org-scoped", "everyone", "always-on"]); }); }); diff --git a/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.tsx b/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.tsx index 6738ec7ae8a..e42a7ed9c3e 100644 --- a/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.tsx +++ b/ui/litellm-dashboard/src/components/logging_credentials/LoggingExportersSelect.tsx @@ -2,10 +2,6 @@ import { Select } from "antd"; import React from "react"; import { useCredentials } from "@/app/(dashboard)/hooks/credentials/useCredentials"; -import { useOrganizations } from "@/app/(dashboard)/hooks/organizations/useOrganizations"; -import { useTeams } from "@/app/(dashboard)/hooks/teams/useTeams"; -import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; -import { isAdminRole } from "@/utils/roles"; interface LoggingExportersSelectProps { value?: string[]; @@ -14,39 +10,22 @@ interface LoggingExportersSelectProps { /** * Multi-select of admin-owned logging destinations (credential_type=logging) that an - * identity (key / team / org) exports its traces to. The selected names are stored in - * metadata.logging_exporters; the proxy unions them across the identity chain and fans - * out. + * identity (key / team / org) exports its traces to. The selected names are persisted to + * the identity's logging_exporters column; the proxy unions them across the identity + * chain and fans out. * - * Options are scoped to what the caller can actually assign: a proxy admin sees every - * destination; everyone else sees only the ones visible to a team or org they belong to - * (plus global / auto_enable destinations). This mirrors the backend assignment gate so - * a team admin is not offered another tenant's destination only to have the save - * rejected, and it avoids surfacing other tenants' destination names. The backend stays - * the authoritative check -- this filter is UX, not a security boundary. + * The options are exactly what GET /credentials returns for the caller, which the backend + * already scopes: a proxy admin receives every destination, while a team or org admin + * receives only the destinations granted to a scope they administer. Visibility is + * enforced server-side by the same predicate the assignment gate and the request-time + * resolver use, so this component does no role-based filtering of its own; doing so would + * risk disagreeing with the backend in either direction. */ const LoggingExportersSelect: React.FC = ({ value, onChange }) => { const { data } = useCredentials(); - const { userRole } = useAuthorized(); - const { data: teams } = useTeams(); - const { data: orgs } = useOrganizations(); - - const seesEveryDestination = isAdminRole(userRole ?? ""); - const myTeamIds = new Set((teams ?? []).map((t) => t.team_id)); - const myOrgIds = new Set((orgs ?? []).map((o) => o.organization_id)); - - const assignable = (info: { - auto_enable?: boolean; - access?: { global?: boolean; teams?: string[]; orgs?: string[] }; - }) => { - if (info.auto_enable === true || info.access?.global === true) return true; - if ((info.access?.teams ?? []).some((id) => myTeamIds.has(id))) return true; - return (info.access?.orgs ?? []).some((id) => myOrgIds.has(id)); - }; const options = (data?.credentials ?? []) .filter((credential) => credential.credential_info?.credential_type === "logging") - .filter((credential) => seesEveryDestination || assignable(credential.credential_info)) .map((credential) => ({ value: credential.credential_name, label: credential.credential_info?.host diff --git a/ui/litellm-dashboard/src/components/logging_credentials/loggingExportersOf.ts b/ui/litellm-dashboard/src/components/logging_credentials/loggingExportersOf.ts new file mode 100644 index 00000000000..f1489cded98 --- /dev/null +++ b/ui/litellm-dashboard/src/components/logging_credentials/loggingExportersOf.ts @@ -0,0 +1,18 @@ +/** + * The admin-owned logging destinations assigned to an identity (key / team / org). + * + * Assignments live on a typed logging_exporters column, surfaced at the top level of + * the API object. A metadata.logging_exporters fallback is kept only so a row written + * before the column existed still renders; the column is the source of truth. + */ +export const loggingExportersOf = (obj: unknown): string[] => { + const record = obj as { + logging_exporters?: unknown; + metadata?: { logging_exporters?: unknown } | null; + } | null; + if (Array.isArray(record?.logging_exporters)) { + return record.logging_exporters as string[]; + } + const fromMetadata = record?.metadata?.logging_exporters; + return Array.isArray(fromMetadata) ? (fromMetadata as string[]) : []; +}; diff --git a/ui/litellm-dashboard/src/components/organization/organization_view.tsx b/ui/litellm-dashboard/src/components/organization/organization_view.tsx index 99dc0923be5..17478caed1c 100644 --- a/ui/litellm-dashboard/src/components/organization/organization_view.tsx +++ b/ui/litellm-dashboard/src/components/organization/organization_view.tsx @@ -13,6 +13,7 @@ import MemberTable from "../common_components/MemberTable"; import UserSearchModal from "../common_components/user_search_modal"; import MCPServerSelector from "../mcp_server_management/MCPServerSelector"; import LoggingExportersSelect from "../logging_credentials/LoggingExportersSelect"; +import { loggingExportersOf } from "../logging_credentials/loggingExportersOf"; import { useCredentials } from "@/app/(dashboard)/hooks/credentials/useCredentials"; import { ModelSelect } from "../ModelSelect/ModelSelect"; import NotificationsManager from "../molecules/notifications_manager"; @@ -155,10 +156,9 @@ const OrganizationInfoView: React.FC = ({ max_budget: values.max_budget, budget_duration: values.budget_duration, }, - metadata: { - ...(values.metadata ? JSON.parse(values.metadata) : {}), - ...(values.logging_exporters !== undefined ? { logging_exporters: values.logging_exporters } : {}), - }, + metadata: values.metadata ? JSON.parse(values.metadata) : {}, + // logging_exporters is a top-level typed column on the org, not metadata. + ...(values.logging_exporters !== undefined ? { logging_exporters: values.logging_exporters } : {}), }; // Handle object_permission updates @@ -336,9 +336,7 @@ const OrganizationInfoView: React.FC = ({ Logging Exporters
{(() => { - const own = Array.isArray(orgData.metadata?.logging_exporters) - ? (orgData.metadata.logging_exporters as string[]) - : []; + const own = loggingExportersOf(orgData); const ownSet = new Set(own); const scopedOnly = scopedExportersForOrg.filter((n) => !ownSet.has(n)); const all = [ @@ -416,7 +414,7 @@ const OrganizationInfoView: React.FC = ({ max_budget: orgData.litellm_budget_table.max_budget, budget_duration: orgData.litellm_budget_table.budget_duration, metadata: orgData.metadata ? JSON.stringify(orgData.metadata, null, 2) : "", - logging_exporters: orgData.metadata?.logging_exporters || [], + logging_exporters: loggingExportersOf(orgData), vector_stores: orgData.object_permission?.vector_stores || [], mcp_servers_and_groups: { servers: orgData.object_permission?.mcp_servers || [], @@ -553,9 +551,7 @@ const OrganizationInfoView: React.FC = ({
Logging Exporters {(() => { - const own = Array.isArray(orgData.metadata?.logging_exporters) - ? (orgData.metadata.logging_exporters as string[]) - : []; + const own = loggingExportersOf(orgData); const ownSet = new Set(own); const scopedOnly = scopedExportersForOrg.filter((n) => !ownSet.has(n)); const all = [ diff --git a/ui/litellm-dashboard/src/components/organizations.tsx b/ui/litellm-dashboard/src/components/organizations.tsx index 13a33496e5e..1b02377e2a2 100644 --- a/ui/litellm-dashboard/src/components/organizations.tsx +++ b/ui/litellm-dashboard/src/components/organizations.tsx @@ -159,16 +159,11 @@ const OrganizationsTable: React.FC = ({ } } - if (Array.isArray(values.logging_exporters) && values.logging_exporters.length > 0) { - let existingMetadata: Record = {}; - if (typeof values.metadata === "string" && values.metadata.trim().length > 0) { - existingMetadata = JSON.parse(values.metadata); - } else if (values.metadata && typeof values.metadata === "object") { - existingMetadata = values.metadata; - } - values.metadata = { ...existingMetadata, logging_exporters: values.logging_exporters }; + // logging_exporters is a top-level typed field on the org (its own column), + // not part of the free-form metadata blob; send it as-is when set. + if (!Array.isArray(values.logging_exporters) || values.logging_exporters.length === 0) { + delete values.logging_exporters; } - delete values.logging_exporters; await organizationCreateCall(accessToken, values); NotificationsManager.success("Organization created successfully"); diff --git a/ui/litellm-dashboard/src/components/settings.tsx b/ui/litellm-dashboard/src/components/settings.tsx index ff8a15d2ce6..f5508ac702b 100644 --- a/ui/litellm-dashboard/src/components/settings.tsx +++ b/ui/litellm-dashboard/src/components/settings.tsx @@ -46,6 +46,7 @@ import { useTeams } from "@/app/(dashboard)/hooks/teams/useTeams"; import { useOrganizations } from "@/app/(dashboard)/hooks/organizations/useOrganizations"; import EditLoggingCredentialModal from "./logging_credentials/EditLoggingCredentialModal"; import AccessControlFields from "./logging_credentials/AccessControlFields"; +import { loggingExportersOf } from "./logging_credentials/loggingExportersOf"; import { backendLabel, createLoggingCredential, @@ -295,15 +296,14 @@ const Settings: React.FC = ({ accessToken, userRole, userID, for (const teamId of access?.teams ?? []) teams.add(teamAlias(teamId)); for (const orgId of access?.orgs ?? []) orgs.add(orgAlias(orgId)); for (const team of teamsData ?? []) { - const teamMetadata = (team as { metadata?: Record | null }).metadata; - const exporters = teamMetadata?.logging_exporters; - if (Array.isArray(exporters) && exporters.includes(destinationName)) { + const exporters = loggingExportersOf(team); + if (exporters.includes(destinationName)) { teams.add(team.team_alias || team.team_id); } } for (const org of orgsData ?? []) { - const exporters = (org.metadata as Record | null | undefined)?.logging_exporters; - if (Array.isArray(exporters) && exporters.includes(destinationName)) { + const exporters = loggingExportersOf(org); + if (exporters.includes(destinationName)) { orgs.add(org.organization_alias || org.organization_id); } } diff --git a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx index e5902800899..8b34892b693 100644 --- a/ui/litellm-dashboard/src/components/team/TeamInfo.tsx +++ b/ui/litellm-dashboard/src/components/team/TeamInfo.tsx @@ -53,6 +53,7 @@ import VectorStoreSelector from "../vector_store_management/VectorStoreSelector" import SearchToolSelector from "../SearchTools/SearchToolSelector"; import EditLoggingSettings from "./EditLoggingSettings"; import LoggingExportersSelect from "../logging_credentials/LoggingExportersSelect"; +import { loggingExportersOf } from "../logging_credentials/loggingExportersOf"; import RouterSettingsAccordion, { RouterSettingsAccordionRef } from "../common_components/RouterSettingsAccordion"; import MemberModal from "./EditMembership"; import MemberPermissions from "./member_permissions"; @@ -545,13 +546,14 @@ const TeamInfoView: React.FC = ({ max_budget: values.max_budget, soft_budget: sanitizeNumeric(values.soft_budget), budget_duration: values.budget_duration, + // logging_exporters is a top-level typed column on the team, not metadata. + ...(values.logging_exporters !== undefined ? { logging_exporters: values.logging_exporters } : {}), metadata: { ...parsedMetadata, ...passthroughRoutesMetadata, guardrails: (values.guardrails || []).filter((n: string) => !globalGuardrailNames.has(n)), opted_out_global_guardrails: optedOutGlobalGuardrails, ...(values.logging_settings?.length > 0 ? { logging: values.logging_settings } : {}), - ...(values.logging_exporters !== undefined ? { logging_exporters: values.logging_exporters } : {}), disable_global_guardrails: killSwitchOnAtSave, soft_budget_alerting_emails: typeof values.soft_budget_alerting_emails === "string" @@ -888,9 +890,7 @@ const TeamInfoView: React.FC = ({ = ({ ) : "", logging_settings: info.metadata?.logging || [], - logging_exporters: info.metadata?.logging_exporters || [], + logging_exporters: loggingExportersOf(info), secret_manager_settings: info.metadata?.secret_manager_settings ? JSON.stringify(info.metadata.secret_manager_settings, null, 2) : "", @@ -1678,9 +1678,7 @@ const TeamInfoView: React.FC = ({ 0 ? { logging: formValues.logging_settings } : {}), - ...(formValues.logging_exporters !== undefined ? { logging_exporters: formValues.logging_exporters } : {}), ...(formValues.disabled_callbacks?.length > 0 ? { litellm_disabled_callbacks: mapDisplayToInternalNames(formValues.disabled_callbacks), @@ -300,7 +300,6 @@ export default function KeyInfoView({ ...(Array.isArray(formValues.logging_settings) && formValues.logging_settings.length > 0 ? { logging: formValues.logging_settings } : {}), - ...(formValues.logging_exporters !== undefined ? { logging_exporters: formValues.logging_exporters } : {}), ...(formValues.disabled_callbacks?.length > 0 ? { litellm_disabled_callbacks: mapDisplayToInternalNames(formValues.disabled_callbacks), @@ -643,11 +642,7 @@ export default function KeyInfoView({