From 0d17f954c04eb68212c66aad95f450d64f6177fb Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Thu, 8 Oct 2026 23:49:56 -0700 Subject: [PATCH] feat(credentials): add display_name and make credential_name immutable on PATCH (#43148) * feat(credentials): add credential_alias and make credential_name immutable on PATCH Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(ui): query credential rows by role to stay under the no-node-access budget Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(proxy): expect credential_alias in load_credential_list dump Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(ui): drive the credential SearchSelect by placeholder and option roles Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(proxy): pass the decrypted CredentialItem to update_db_credential during master key rotation Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(ui): trim credential_alias in CredentialModal so whitespace-only input clears the alias Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * feat(credentials): replace credential_alias with display_name and reject edits to config credentials credential_name stays the immutable reference key. display_name is a nullable, trimmed, max 255 character label set on POST or PATCH (omit keeps it, null clears it, blank is a 400). Reads return display_name plus an in-memory source tag (db or config), and PATCH or DELETE on a config-defined credential answers 400 instead of a misleading 404 * feat(ui): show credential display names everywhere and lock config credentials The credentials table shows the display name with the credential name beneath it, badges config credentials and disables their edit and delete actions. The edit modal adds an editable display name next to the read-only credential name and only sends it when it changed. Every credential picker and reference (add model, model info, models table, vector store form and info) shows the label while still submitting credential_name * feat(cli): set display names on credentials and show them in the list * refactor(credentials): keep the new credential types inside the lint gate budgets * refactor(cli): print credential command JSON through one helper * fix(credentials): treat an empty credential_name on PATCH as omitted and pin the 404 for vanished rows A blank credential_name never renamed anything, so PATCH accepts it again instead of answering 400. New tests pin a trimmed display_name on PATCH, the repository carrying display_name, and a 404 (not the config-owned 400) for a DB credential another worker already deleted. The hydration helper no longer copies display_name, since none of its callers read it, and the CLI update passes display_name straight through because the exactly-one check already makes it None when clearing. * fix(ui): keep a model's credential when its picker text is emptied and skip the admin-only list for other roles Clearing the search text in the model edit form's credential picker used to submit null, silently detaching the model's credential on save. None is now the only way to clear it. The models table fetches /credentials only for proxy admins, since everyone else got a 403 on each page load and falls back to the raw name anyway. Also fixes a type error in the re-use credential dialog and adds tests for the display name surviving a provider switch, the 255 character limit, the request payload, and the vector store None choice. * fix(client): percent-encode the credential name when updating its display name A name with ? or # was cut short in the URL, so the PATCH landed on a different credential or 404'd. * fix(credentials): answer 405 with Allow: GET for PATCH and DELETE on config credentials A config-defined credential exists (GET returns it) but is read-only through the API, which is what 405 Method Not Allowed means. 400 described a malformed request, and 404 would claim the credential does not exist. * fix(proxy): ignore a display_name set on config credential_list entries so non-string values cannot fail boot --------- Co-authored-by: yuneng Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../migration.sql | 2 + .../litellm_proxy_extras/schema.prisma | 1 + litellm/models/credentials.py | 17 +- .../proxy/client/cli/commands/credentials.py | 53 +++- litellm/proxy/client/credentials.py | 27 ++ .../proxy/credential_endpoints/endpoints.py | 110 ++++--- litellm/proxy/proxy_server.py | 5 +- litellm/proxy/schema.prisma | 1 + .../repositories/credentials_repository.py | 1 + schema.prisma | 1 + .../test_credential_display_name.py | 129 ++++++++ .../client/cli/test_credentials_commands.py | 53 ++++ tests/unit/proxy/client/test_credentials.py | 25 ++ .../credential_endpoints/test_endpoints.py | 288 ++++++++++++++---- .../proxy/proxy_server/test_proxy_config.py | 59 ++++ tests/unit/repositories/test_repositories.py | 2 + .../hooks/credentials/useCredentials.test.ts | 10 + .../hooks/credentials/useCredentials.ts | 4 +- .../components/AllModelsTab.test.tsx | 37 +++ .../components/AllModelsTab.tsx | 9 + .../components/AllModelsTable.test.tsx | 13 + .../components/AllModelsTable.tsx | 15 +- .../components/ModelsTableColumns.tsx | 16 +- .../VectorStoreForm.integration.test.tsx | 46 ++- .../_components/VectorStoreForm.tsx | 52 +--- .../_components/vector_store_info.tsx | 68 +---- .../src/components/ModelInfoEditForm.test.tsx | 117 +++++++ .../src/components/ModelInfoEditForm.tsx | 48 ++- .../AddModelForm.integration.test.tsx | 42 +++ .../src/components/add_model/AddModelForm.tsx | 14 +- .../model_add/CredentialModal.test.tsx | 141 ++++++++- .../components/model_add/CredentialModal.tsx | 44 ++- .../model_add/CredentialsTable.test.tsx | 65 ++++ .../model_add/CredentialsTableColumns.tsx | 31 +- .../model_add/credential_form_helpers.test.ts | 34 ++- .../model_add/credential_form_helpers.ts | 7 +- .../src/components/model_info_view.test.tsx | 38 ++- .../src/components/model_info_view.tsx | 6 +- .../src/components/networking.tsx | 2 + .../shared/credentialOptions.test.ts | 57 ++++ .../components/shared/credentialOptions.ts | 21 ++ ui/litellm-dashboard/src/lib/http/schema.d.ts | 30 +- 42 files changed, 1477 insertions(+), 264 deletions(-) create mode 100644 litellm-proxy-extras/litellm_proxy_extras/migrations/20261009000000_add_credential_display_name/migration.sql create mode 100644 tests/integration/management/test_credential_display_name.py create mode 100644 ui/litellm-dashboard/src/components/ModelInfoEditForm.test.tsx create mode 100644 ui/litellm-dashboard/src/components/shared/credentialOptions.test.ts create mode 100644 ui/litellm-dashboard/src/components/shared/credentialOptions.ts diff --git a/litellm-proxy-extras/litellm_proxy_extras/migrations/20261009000000_add_credential_display_name/migration.sql b/litellm-proxy-extras/litellm_proxy_extras/migrations/20261009000000_add_credential_display_name/migration.sql new file mode 100644 index 00000000000..264bd88a807 --- /dev/null +++ b/litellm-proxy-extras/litellm_proxy_extras/migrations/20261009000000_add_credential_display_name/migration.sql @@ -0,0 +1,2 @@ +-- AlterTable +ALTER TABLE "LiteLLM_CredentialsTable" ADD COLUMN IF NOT EXISTS "display_name" TEXT; diff --git a/litellm-proxy-extras/litellm_proxy_extras/schema.prisma b/litellm-proxy-extras/litellm_proxy_extras/schema.prisma index 038dfdeaca5..59ffb037177 100644 --- a/litellm-proxy-extras/litellm_proxy_extras/schema.prisma +++ b/litellm-proxy-extras/litellm_proxy_extras/schema.prisma @@ -42,6 +42,7 @@ model LiteLLM_BudgetTable { model LiteLLM_CredentialsTable { credential_id String @id @default(uuid()) credential_name String @unique + display_name String? credential_values Json credential_info Json? created_at DateTime @default(now()) @map("created_at") diff --git a/litellm/models/credentials.py b/litellm/models/credentials.py index b91ced275ff..730778a323b 100644 --- a/litellm/models/credentials.py +++ b/litellm/models/credentials.py @@ -6,14 +6,18 @@ layer; ``litellm.types.utils`` re-exports them for backwards compatibility. """ from collections.abc import Mapping +from typing import Literal, TypeAlias -from pydantic import Field, model_validator +from pydantic import ConfigDict, Field, model_validator from litellm.types.llms.base import LiteLLMBaseModel +CredentialSource: TypeAlias = Literal["db", "config"] + class CredentialBase(LiteLLMBaseModel): credential_name: str + display_name: str | None = None credential_info: dict @@ -23,6 +27,14 @@ class CredentialItem(CredentialBase): # edit rather than the credential, so it stays out of dumps: those feed config loading, the DB # write, and the in-memory list, none of which have a place for it. credential_values_to_delete: tuple[str, ...] | None = Field(default=None, exclude=True) + source: CredentialSource = Field(default="db", exclude=True) + + +class CredentialView(CredentialBase): + model_config = ConfigDict(frozen=True) + + credential_values: Mapping[str, object] + source: CredentialSource class CreateCredentialItem(CredentialBase): @@ -38,7 +50,8 @@ class CreateCredentialItem(CredentialBase): class UpdateCredentialItem(LiteLLMBaseModel): - credential_name: str + credential_name: str | None = None + display_name: str | None = None credential_info: Mapping[str, object] credential_values: Mapping[str, object] | None = None model_id: str | None = None diff --git a/litellm/proxy/client/cli/commands/credentials.py b/litellm/proxy/client/cli/commands/credentials.py index 2c4080dbeb2..2028bdb5599 100644 --- a/litellm/proxy/client/cli/commands/credentials.py +++ b/litellm/proxy/client/cli/commands/credentials.py @@ -18,6 +18,8 @@ class _CredentialInfo(TypedDict): class _CredentialItem(TypedDict): credential_name: ReadOnly[NotRequired[str]] + display_name: ReadOnly[NotRequired[str | None]] + source: ReadOnly[NotRequired[str]] credential_info: ReadOnly[NotRequired[_CredentialInfo]] @@ -33,6 +35,10 @@ class _JsonBodyView(TypedDict): body: ReadOnly[object] +def _print_json(data: object) -> None: + rich.print_json(data=data) + + @click.group() def credentials(): """Manage credentials for the LiteLLM proxy server""" @@ -55,12 +61,14 @@ def list(ctx: click.Context, output_format: Literal["table", "json"]): assert isinstance(response, dict) if output_format == "json": - rich.print_json(data=response) + _print_json(response) else: # table format table: Final = Table(title="Credentials") # Add columns table.add_column("Credential Name", style="cyan") + table.add_column("Display Name") + table.add_column("Source") table.add_column("Custom LLM Provider", style="green") # Add rows @@ -69,6 +77,8 @@ def list(ctx: click.Context, output_format: Literal["table", "json"]): info = cred.get("credential_info", {}) table.add_row( str(cred.get("credential_name", "")), + cred.get("display_name") or "", + str(cred.get("source", "")), str(info.get("custom_llm_provider", "")), ) @@ -89,8 +99,9 @@ def list(ctx: click.Context, output_format: Literal["table", "json"]): help="JSON string containing credential values", required=True, ) +@click.option("--display-name", type=str, default=None, help="Optional label shown in the UI") @click.pass_context -def create(ctx: click.Context, credential_name: str, info: str, values: str): +def create(ctx: click.Context, credential_name: str, info: str, values: str, display_name: str | None) -> None: """Create a new credential""" context: Final = cli_context_values(ctx) client: Final = CredentialsManagementClient(context["base_url"], context["api_key"]) @@ -101,13 +112,39 @@ def create(ctx: click.Context, credential_name: str, info: str, values: str): raise click.BadParameter(f"Invalid JSON: {e}") try: - response: Final = client.create(credential_name, credential_info["value"], credential_values["value"]) - rich.print_json(data=response) + response: Final = client.create( + credential_name, credential_info["value"], credential_values["value"], display_name=display_name + ) + _print_json(response) except requests.exceptions.HTTPError as e: click.echo(f"Error: HTTP {e.response.status_code}", err=True) try: error_body: Final[_JsonBodyView] = {"body": e.response.json()} - rich.print_json(data=error_body["body"]) + _print_json(error_body["body"]) + except json.JSONDecodeError: + click.echo(e.response.text, err=True) + raise click.Abort() + + +@credentials.command() +@click.argument("credential_name") +@click.option("--display-name", type=str, default=None, help="New label shown in the UI") +@click.option("--clear-display-name", is_flag=True, help="Remove the label so the UI shows the credential name") +@click.pass_context +def update(ctx: click.Context, credential_name: str, display_name: str | None, clear_display_name: bool) -> None: + """Change a credential's display name. The credential name itself cannot change""" + if (display_name is None) == (not clear_display_name): + raise click.UsageError("Pass exactly one of --display-name or --clear-display-name") + context: Final = cli_context_values(ctx) + client: Final = CredentialsManagementClient(context["base_url"], context["api_key"]) + try: + response: Final = client.update_display_name(credential_name, display_name) + _print_json(response) + except requests.exceptions.HTTPError as e: + click.echo(f"Error: HTTP {e.response.status_code}", err=True) + try: + error_body: Final[_JsonBodyView] = {"body": e.response.json()} + _print_json(error_body["body"]) except json.JSONDecodeError: click.echo(e.response.text, err=True) raise click.Abort() @@ -122,12 +159,12 @@ def delete(ctx: click.Context, credential_name: str): client: Final = CredentialsManagementClient(context["base_url"], context["api_key"]) try: response: Final = client.delete(credential_name) - rich.print_json(data=response) + _print_json(response) except requests.exceptions.HTTPError as e: click.echo(f"Error: HTTP {e.response.status_code}", err=True) try: error_body: Final[_JsonBodyView] = {"body": e.response.json()} - rich.print_json(data=error_body["body"]) + _print_json(error_body["body"]) except json.JSONDecodeError: click.echo(e.response.text, err=True) raise click.Abort() @@ -141,4 +178,4 @@ def get(ctx: click.Context, credential_name: str): context: Final = cli_context_values(ctx) client: Final = CredentialsManagementClient(context["base_url"], context["api_key"]) response: Final = client.get(credential_name) - rich.print_json(data=response) + _print_json(response) diff --git a/litellm/proxy/client/credentials.py b/litellm/proxy/client/credentials.py index d9edecd2eb7..ebcd79457a6 100644 --- a/litellm/proxy/client/credentials.py +++ b/litellm/proxy/client/credentials.py @@ -1,5 +1,6 @@ from collections.abc import Mapping from typing import Any, Final +from urllib.parse import quote import requests @@ -73,6 +74,7 @@ class CredentialsManagementClient: credential_info: Mapping[str, object], credential_values: Mapping[str, object], return_request: bool = False, + display_name: str | None = None, ) -> dict[str, Any] | requests.Request: """ Create a new credential. @@ -97,6 +99,7 @@ class CredentialsManagementClient: "credential_name": credential_name, "credential_info": credential_info, "credential_values": credential_values, + **({} if display_name is None else {"display_name": display_name}), } request: Final = requests.Request("POST", url, headers=self._get_headers(), json=data) @@ -151,6 +154,30 @@ class CredentialsManagementClient: raise UnauthorizedError(e) raise + def update_display_name( + self, + credential_name: str, + display_name: str | None, + return_request: bool = False, + ) -> Mapping[str, object] | requests.Request: + url: Final = f"{self._base_url}/credentials/{quote(credential_name, safe='')}" + data: Final[Mapping[str, object]] = {"display_name": display_name, "credential_info": {}} + + request: Final = requests.Request("PATCH", url, headers=self._get_headers(), json=data) + + if return_request: + return request + + session: Final = requests.Session() + try: + response: Final = session.send(request.prepare(), timeout=self._timeout) + response.raise_for_status() + return response.json() + except requests.exceptions.HTTPError as e: + if e.response.status_code == 401: + raise UnauthorizedError(e) + raise + def get( self, credential_name: str, diff --git a/litellm/proxy/credential_endpoints/endpoints.py b/litellm/proxy/credential_endpoints/endpoints.py index 6d25307db96..8df22f458cf 100644 --- a/litellm/proxy/credential_endpoints/endpoints.py +++ b/litellm/proxy/credential_endpoints/endpoints.py @@ -22,7 +22,7 @@ from litellm.llms.anthropic.wif import ( UnbuildableIdentitySource, anthropic_internal_issuer_jwks, ) -from litellm.models.credentials import UpdateCredentialItem +from litellm.models.credentials import CredentialView, UpdateCredentialItem from litellm.proxy._types import ( CommonProxyErrors, LitellmUserRoles, @@ -32,7 +32,6 @@ from litellm.proxy._types import ( ) from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.common_utils.credential_hydration import ( - hydrate_named_credential, hydrate_named_credential_authoritative, named_credential_wif_fields, stored_credential_provider, @@ -46,6 +45,7 @@ from litellm.types.utils import CreateCredentialItem, CredentialItem router: Final = APIRouter() _CREDENTIAL_DICT_ADAPTER: Final = TypeAdapter(dict[str, object]) +_DISPLAY_NAME_MAX_LENGTH: Final = 255 def _reject_non_admin_wif_fields( @@ -105,7 +105,7 @@ def _without_null_values(credential_values: Mapping[str, object]) -> dict[str, o return {key: value for key, value in credential_values.items() if value is not None} -def _sync_in_memory_credential(credential: CredentialItem, credential_name: str, new_name: str) -> None: +def _sync_in_memory_credential(credential: CredentialItem, credential_name: str) -> None: """Mirror a DB credential update into the in-memory ``credential_list`` used by request-time resolution; a no-op if the credential isn't loaded in memory (e.g. proxy restarted since boot). """ @@ -127,13 +127,11 @@ def _sync_in_memory_credential(credential: CredentialItem, credential_name: str, if credential.credential_info: in_memory_info.update(credential.credential_info) updated_in_memory: Final = CredentialItem( - credential_name=new_name, + credential_name=credential_name, + display_name=credential.display_name, credential_values=in_memory_values, credential_info=in_memory_info, ) - # Remove old entry if renamed, then use upsert_credentials to handle duplicates - if new_name != credential_name: - litellm.credential_list = [c for c in litellm.credential_list if c.credential_name != credential_name] CredentialAccessor.upsert_credentials([updated_in_memory]) @@ -149,11 +147,56 @@ class CredentialHelperUtils: # is kept in memory and should remain unencrypted. return CredentialItem( credential_name=credential.credential_name, + display_name=credential.display_name, credential_values=encrypted_credential_values, credential_info=credential.credential_info or {}, ) +def _normalized_display_name(display_name: str | None) -> str | None: + if display_name is None: + return None + trimmed: Final = display_name.strip() + if not trimmed: + raise ProxyException( + message="display_name cannot be blank. Send null to clear it or omit the field to leave it unchanged.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="display_name", + ) + if len(trimmed) > _DISPLAY_NAME_MAX_LENGTH: + raise ProxyException( + message=f"display_name cannot be longer than {_DISPLAY_NAME_MAX_LENGTH} characters.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="display_name", + ) + return trimmed + + +def _not_found_unless_config_defined(credential_name: str, not_found_detail: str) -> HTTPException | ProxyException: + in_memory: Final = CredentialAccessor.find_credential(credential_name) + if in_memory is None or in_memory.source != "config": + return HTTPException(status_code=404, detail=not_found_detail) + return ProxyException( + message=f"Credential '{credential_name}' is defined in config and cannot be edited from the API or UI.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_405_METHOD_NOT_ALLOWED, + param="credential_name", + headers={"Allow": "GET"}, + ) + + +def _credential_view(credential: CredentialItem, credential_values: Mapping[str, object]) -> CredentialView: + return CredentialView( + credential_name=credential.credential_name, + display_name=credential.display_name, + credential_values=credential_values, + credential_info=credential.credential_info, + source=credential.source, + ) + + def _credential_exists_detail(credential_name: str) -> str: return ( f"Credential '{credential_name}' already exists. " @@ -222,6 +265,7 @@ async def create_credential( ) processed_credential: Final = CredentialItem( credential_name=credential.credential_name, + display_name=_normalized_display_name(credential.display_name), credential_values=_without_null_values(_CREDENTIAL_DICT_ADAPTER.validate_python(credential_values)), credential_info=credential.credential_info, ) @@ -267,11 +311,7 @@ async def get_credentials( """ try: masked_credentials: Final = [ - { - "credential_name": credential.credential_name, - "credential_values": get_masked_values(credential.credential_values), - "credential_info": credential.credential_info, - } + _credential_view(credential, get_masked_values(credential.credential_values)) for credential in litellm.credential_list ] return {"success": True, "credentials": masked_credentials} @@ -283,7 +323,7 @@ async def get_credentials( "/credentials/by_name/{credential_name:path}", dependencies=[Depends(user_api_key_auth)], tags=["credential management"], - response_model=CredentialItem, + response_model=CredentialView, ) async def get_credential_by_name( request: Request, @@ -297,16 +337,10 @@ async def get_credential_by_name( try: for credential in litellm.credential_list: if credential.credential_name == credential_name: - masked_credential = CredentialItem( - credential_name=credential.credential_name, - credential_values=get_masked_values( - credential.credential_values, - unmasked_length=4, - number_of_asterisks=4, - ), - credential_info=credential.credential_info, + return _credential_view( + credential, + get_masked_values(credential.credential_values, unmasked_length=4, number_of_asterisks=4), ) - return masked_credential raise HTTPException( status_code=404, detail="Credential not found. Got credential name: " + credential_name, @@ -444,9 +478,8 @@ async def delete_credential( ) deleted: Final = await CredentialsRepository(prisma_client).delete_by_name(credential_name) if deleted is None: - raise HTTPException( - status_code=404, - detail="Credential not found. Got credential name: " + credential_name, + raise _not_found_unless_config_defined( + credential_name, "Credential not found. Got credential name: " + credential_name ) ## DELETE FROM LITELLM ## @@ -466,6 +499,7 @@ def update_db_credential( """ merged_credential: Final = CredentialItem( credential_name=db_credential.credential_name, + display_name=updated_patch.display_name, credential_info=db_credential.credential_info, credential_values=db_credential.credential_values, ) @@ -474,10 +508,6 @@ def update_db_credential( updated_patch, new_encryption_key, ) - # update model name - if encrypted_credential.credential_name: - merged_credential.credential_name = encrypted_credential.credential_name - # update litellm params if encrypted_credential.credential_values: # Encrypt any sensitive values @@ -515,6 +545,14 @@ async def update_credential( from litellm.proxy.proxy_server import prisma_client try: + if credential.credential_name and credential.credential_name != credential_name: + raise ProxyException( + message="credential_name is immutable. Set display_name to change how the credential is labeled.", + type=ProxyErrorTypes.validation_error.value, + code=status.HTTP_400_BAD_REQUEST, + param="credential_name", + ) + requested_display_name: Final = _normalized_display_name(credential.display_name) _reject_overlapping_credential_values(credential) incoming_values: Final = _CREDENTIAL_DICT_ADAPTER.validate_python( _resolve_deployment_credentials(llm_router, credential.model_id) @@ -530,14 +568,13 @@ async def update_credential( credentials_repository: Final = CredentialsRepository(prisma_client) db_credential: Final = await credentials_repository.find_by_name(credential_name) if db_credential is None: - raise HTTPException(status_code=404, detail="Credential not found in DB.") + raise _not_found_unless_config_defined(credential_name, "Credential not found in DB.") _reject_non_admin_wif_fields(_stored_wif_fields(db_credential), user_api_key_dict) - if credential.credential_name != credential_name: - shadowed_credential: Final = await hydrate_named_credential(credential.credential_name, prisma_client) - if shadowed_credential is not None: - _reject_non_admin_wif_fields(_stored_wif_fields(shadowed_credential), user_api_key_dict) patch: Final = CredentialItem( - credential_name=credential.credential_name, + credential_name=credential_name, + display_name=( + requested_display_name if "display_name" in credential.model_fields_set else db_credential.display_name + ), credential_info=_CREDENTIAL_DICT_ADAPTER.validate_python(credential.credential_info), credential_values=incoming_values, credential_values_to_delete=credential.credential_values_to_delete, @@ -550,12 +587,13 @@ async def update_credential( credential_name, data={ **credential_object_jsonified, + "display_name": merged_credential.display_name, "updated_by": user_api_key_dict.user_id, }, ) # Sync in-memory credential_list (skip if not in memory - e.g., proxy restarted) - _sync_in_memory_credential(patch, credential_name, merged_credential.credential_name) + _sync_in_memory_credential(patch, credential_name) return {"success": True, "message": "Credential updated successfully"} except Exception as e: diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index e035f061db9..67fec38e39e 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -6313,7 +6313,10 @@ class ProxyConfig: credential_list_dict: Final = config.get("credential_list") credential_list = [] if credential_list_dict: - credential_list = [CredentialItem(**cred) for cred in credential_list_dict] + credential_list = [ + CredentialItem.model_validate({**cred, "display_name": None, "source": "config"}) + for cred in credential_list_dict + ] return credential_list def parse_search_tools(self, config: dict) -> list[SearchToolTypedDict] | None: diff --git a/litellm/proxy/schema.prisma b/litellm/proxy/schema.prisma index 038dfdeaca5..59ffb037177 100644 --- a/litellm/proxy/schema.prisma +++ b/litellm/proxy/schema.prisma @@ -42,6 +42,7 @@ model LiteLLM_BudgetTable { model LiteLLM_CredentialsTable { credential_id String @id @default(uuid()) credential_name String @unique + display_name String? credential_values Json credential_info Json? created_at DateTime @default(now()) @map("created_at") diff --git a/litellm/repositories/credentials_repository.py b/litellm/repositories/credentials_repository.py index ddb9767b2b9..fa6aeca534d 100644 --- a/litellm/repositories/credentials_repository.py +++ b/litellm/repositories/credentials_repository.py @@ -58,6 +58,7 @@ class CredentialsRepository: return CredentialItem.model_validate( { "credential_name": data["credential_name"], + "display_name": data.get("display_name"), "credential_values": data.get("credential_values") or {}, "credential_info": data.get("credential_info") or {}, } diff --git a/schema.prisma b/schema.prisma index 038dfdeaca5..59ffb037177 100644 --- a/schema.prisma +++ b/schema.prisma @@ -42,6 +42,7 @@ model LiteLLM_BudgetTable { model LiteLLM_CredentialsTable { credential_id String @id @default(uuid()) credential_name String @unique + display_name String? credential_values Json credential_info Json? created_at DateTime @default(now()) @map("created_at") diff --git a/tests/integration/management/test_credential_display_name.py b/tests/integration/management/test_credential_display_name.py new file mode 100644 index 00000000000..0e72dcf8b2f --- /dev/null +++ b/tests/integration/management/test_credential_display_name.py @@ -0,0 +1,129 @@ +import uuid +from typing import Final + +from pydantic import JsonValue + +from tests.integration._support.client import Gateway, Scenario +from tests.integration._support.database import read_rows + + +def _credential_with_display_name(gateway: Gateway, scenario: Scenario, display_name: str) -> str: + name: Final = f"credential-{uuid.uuid4().hex}" + gateway.post( + "/credentials", + { + "credential_name": name, + "display_name": display_name, + "credential_values": {"api_key": "synthetic-credential"}, + "credential_info": {"custom_llm_provider": "openai"}, + }, + ) + scenario.cleanups.callback(_delete_credential_if_present, gateway, name) + return name + + +def _delete_credential_if_present(gateway: Gateway, name: str) -> None: + response: Final = gateway.request("DELETE", f"/credentials/{name}") + assert response.status_code in (200, 404), response.text + + +def _stored_display_name(name: str) -> list[dict[str, JsonValue]]: + return read_rows( + 'SELECT credential_name, display_name FROM "LiteLLM_CredentialsTable" WHERE credential_name = %s', + (name,), + ) + + +def _served_credential(gateway: Gateway, name: str) -> dict[str, JsonValue]: + by_name: Final = gateway.get(f"/credentials/by_name/{name}") + listed: Final = [entry for entry in gateway.get("/credentials")["credentials"] if isinstance(entry, dict)] + from_list: Final = [entry for entry in listed if entry["credential_name"] == name] + assert len(from_list) == 1, listed + assert from_list[0]["display_name"] == by_name["display_name"], (from_list[0], by_name) + return by_name + + +def _model_using(gateway: Gateway, scenario: Scenario, credential: str) -> str: + return scenario.model( + model="openai/gpt-4o-mini", + api_base=f"{gateway.upstream_url}/v1", + litellm_credential_name=credential, + ) + + +def test_display_name_round_trips_and_patch_keeps_clears_and_rejects_blank(gateway: Gateway) -> None: + with gateway.scenario() as scenario: + name: Final = _credential_with_display_name(gateway, scenario, "Prod OpenAI") + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Prod OpenAI"}] + assert _served_credential(gateway, name)["display_name"] == "Prod OpenAI" + + kept: Final = gateway.request("PATCH", f"/credentials/{name}", {"credential_info": {"description": "d"}}) + assert kept.status_code == 200, kept.text + assert _served_credential(gateway, name)["display_name"] == "Prod OpenAI" + + relabeled: Final = gateway.request( + "PATCH", f"/credentials/{name}", {"display_name": "Staging OpenAI", "credential_info": {}} + ) + assert relabeled.status_code == 200, relabeled.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Staging OpenAI"}] + assert _served_credential(gateway, name)["display_name"] == "Staging OpenAI" + + blank: Final = gateway.request("PATCH", f"/credentials/{name}", {"display_name": " ", "credential_info": {}}) + assert blank.status_code == 400, blank.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Staging OpenAI"}] + + too_long: Final = gateway.request( + "PATCH", f"/credentials/{name}", {"display_name": "x" * 256, "credential_info": {}} + ) + assert too_long.status_code == 400, too_long.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Staging OpenAI"}] + + trimmed: Final = gateway.request( + "PATCH", f"/credentials/{name}", {"display_name": " Staging OpenAI v2 ", "credential_info": {}} + ) + assert trimmed.status_code == 200, trimmed.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Staging OpenAI v2"}] + assert _served_credential(gateway, name)["source"] == "db" + + cleared: Final = gateway.request("PATCH", f"/credentials/{name}", {"display_name": None, "credential_info": {}}) + assert cleared.status_code == 200, cleared.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": None}] + assert _served_credential(gateway, name)["display_name"] is None + + +def test_blank_display_name_on_create_is_rejected(gateway: Gateway) -> None: + name: Final = f"credential-{uuid.uuid4().hex}" + rejected: Final = gateway.request( + "POST", + "/credentials", + {"credential_name": name, "display_name": "", "credential_values": {"api_key": "k"}, "credential_info": {}}, + ) + assert rejected.status_code == 400, rejected.text + assert _stored_display_name(name) == [] + + +def test_patch_rename_is_rejected_and_model_keeps_working(gateway: Gateway) -> None: + with gateway.scenario() as scenario: + name: Final = _credential_with_display_name(gateway, scenario, "Prod OpenAI") + model: Final = _model_using(gateway, scenario, name) + assert gateway.chat(model)["object"] == "chat.completion" + + other: Final = f"credential-{uuid.uuid4().hex}" + renamed: Final = gateway.request( + "PATCH", f"/credentials/{name}", {"credential_name": other, "credential_info": {"description": "x"}} + ) + assert renamed.status_code == 400, renamed.text + assert "display_name" in renamed.text, renamed.text + assert _stored_display_name(name) == [{"credential_name": name, "display_name": "Prod OpenAI"}] + assert _stored_display_name(other) == [] + assert gateway.request("GET", f"/credentials/by_name/{other}").status_code == 404 + + same_name: Final = gateway.request( + "PATCH", f"/credentials/{name}", {"credential_name": name, "credential_info": {"description": "y"}} + ) + assert same_name.status_code == 200, same_name.text + assert _served_credential(gateway, name)["credential_info"] == { + "custom_llm_provider": "openai", + "description": "y", + } + assert gateway.chat(model)["object"] == "chat.completion" diff --git a/tests/unit/proxy/client/cli/test_credentials_commands.py b/tests/unit/proxy/client/cli/test_credentials_commands.py index fb9d749dd02..fe9bc333f37 100644 --- a/tests/unit/proxy/client/cli/test_credentials_commands.py +++ b/tests/unit/proxy/client/cli/test_credentials_commands.py @@ -108,6 +108,7 @@ def test_acreate_credential_success(cli_runner, mock_credentials_client): "test-cred", {"custom_llm_provider": "azure"}, {"api_key": "test-key"}, + display_name=None, ) @@ -215,3 +216,55 @@ def test_aget_credential_success(cli_runner, mock_credentials_client): output_data = json.loads(result.output) assert output_data == mock_response mock_instance.get.assert_called_once_with("test-cred") + + +def test_list_credentials_table_shows_display_name_and_source(cli_runner, mock_credentials_client): + mock_credentials_client.return_value.list.return_value = { + "credentials": [ + {"credential_name": "openai-prod", "display_name": "Prod OpenAI", "source": "db", "credential_info": {}}, + {"credential_name": "from-yaml", "display_name": None, "source": "config", "credential_info": {}}, + ] + } + + result = cli_runner.invoke(cli, ["credentials", "list"]) + + assert result.exit_code == 0 + assert "Prod OpenAI" in result.output + assert "config" in result.output + assert "None" not in result.output + + +def test_create_credential_forwards_the_display_name(cli_runner, mock_credentials_client): + mock_instance = mock_credentials_client.return_value + mock_instance.create.return_value = {"success": True} + + result = cli_runner.invoke( + cli, + ["credentials", "create", "openai-prod", "--info", "{}", "--values", "{}", "--display-name", "Prod OpenAI"], + ) + + assert result.exit_code == 0 + mock_instance.create.assert_called_once_with("openai-prod", {}, {}, display_name="Prod OpenAI") + + +@pytest.mark.parametrize( + ("flags", "expected"), + [(["--display-name", "Prod OpenAI"], "Prod OpenAI"), (["--clear-display-name"], None)], +) +def test_update_credential_sets_or_clears_the_display_name(cli_runner, mock_credentials_client, flags, expected): + mock_instance = mock_credentials_client.return_value + mock_instance.update_display_name.return_value = {"success": True} + + result = cli_runner.invoke(cli, ["credentials", "update", "openai-prod", *flags]) + + assert result.exit_code == 0, result.output + mock_instance.update_display_name.assert_called_once_with("openai-prod", expected) + + +@pytest.mark.parametrize("flags", [[], ["--display-name", "Prod", "--clear-display-name"]]) +def test_update_credential_requires_exactly_one_display_name_flag(cli_runner, mock_credentials_client, flags): + result = cli_runner.invoke(cli, ["credentials", "update", "openai-prod", *flags]) + + assert result.exit_code != 0 + assert "exactly one" in result.output + mock_credentials_client.return_value.update_display_name.assert_not_called() diff --git a/tests/unit/proxy/client/test_credentials.py b/tests/unit/proxy/client/test_credentials.py index 666c5dac2b0..9de990da487 100644 --- a/tests/unit/proxy/client/test_credentials.py +++ b/tests/unit/proxy/client/test_credentials.py @@ -130,6 +130,31 @@ def test_create_request(client, base_url, api_key): } +def test_create_request_includes_the_display_name_only_when_given(client): + labeled = client.create("azure1", {}, {"api_key": "sk-123"}, return_request=True, display_name="Azure EU") + unlabeled = client.create("azure1", {}, {"api_key": "sk-123"}, return_request=True) + + assert labeled.json["display_name"] == "Azure EU" + assert "display_name" not in unlabeled.json + + +@pytest.mark.parametrize("display_name", ["Azure EU", None]) +def test_update_display_name_request(client, base_url, api_key, display_name): + request = client.update_display_name("azure1", display_name, return_request=True) + + assert request.method == "PATCH" + assert request.url == f"{base_url}/credentials/azure1" + assert request.headers["Authorization"] == f"Bearer {api_key}" + assert request.json == {"display_name": display_name, "credential_info": {}} + + +@pytest.mark.parametrize(("credential_name", "path"), [("team?prod", "team%3Fprod"), ("a/b#c", "a%2Fb%23c")]) +def test_update_display_name_targets_the_whole_credential_name(client, base_url, credential_name, path): + request = client.update_display_name(credential_name, "Label", return_request=True) + + assert request.prepare().url == f"{base_url}/credentials/{path}" + + @responses.activate def test_create_mock_response(client): """Test create with a mocked successful response""" diff --git a/tests/unit/proxy/credential_endpoints/test_endpoints.py b/tests/unit/proxy/credential_endpoints/test_endpoints.py index aff7babe424..83d6167256a 100644 --- a/tests/unit/proxy/credential_endpoints/test_endpoints.py +++ b/tests/unit/proxy/credential_endpoints/test_endpoints.py @@ -14,6 +14,7 @@ from litellm.proxy._types import UserAPIKeyAuth from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.credential_endpoints.endpoints import get_llm_router from litellm.proxy.proxy_server import app +from litellm.models.credentials import CredentialSource from litellm.types.utils import CredentialItem client = TestClient(app) @@ -771,7 +772,7 @@ class TestNonAdminCannotPersistWifFieldsOnCredential: update_mock.assert_awaited_once() -def _wif_credential(name: str = "federated-cred") -> CredentialItem: +def _wif_credential(name: str = "federated-cred", source: CredentialSource = "db") -> CredentialItem: return CredentialItem( credential_name=name, credential_values={ @@ -779,6 +780,7 @@ def _wif_credential(name: str = "federated-cred") -> CredentialItem: "api_key": "sk-old", }, credential_info={"custom_llm_provider": "anthropic"}, + source=source, ) @@ -978,7 +980,7 @@ class TestNonAdminCannotTouchAStoredWifCredential: def test_non_admin_cannot_delete_a_config_only_wif_credential(self, restore_credential_list, monkeypatch): """A ``credential_list`` entry from config.yaml has no DB row, so a gate that consulted only the DB let a non-admin evict the admin-owned federation settings from memory.""" - config_credential = _wif_credential("config-wif") + config_credential = _wif_credential("config-wif", source="config") monkeypatch.setattr(litellm, "credential_list", [config_credential]) with _repository_holding(None) as repository: response = _delete_credential("config-wif", auth=_as_non_admin) @@ -989,15 +991,17 @@ class TestNonAdminCannotTouchAStoredWifCredential: assert litellm.credential_list == [config_credential] def test_proxy_admin_can_delete_a_config_only_wif_credential(self, restore_credential_list, monkeypatch): - """The gate lets the admin through to the row delete. The 404 that follows is the rule for + """The gate lets the admin through to the row delete. The 400 that follows is the rule for every config-only credential (no row to delete, the entry is back on the next boot), so the in-memory entry stays put too.""" - config_credential = _wif_credential("config-wif") + config_credential = _wif_credential("config-wif", source="config") monkeypatch.setattr(litellm, "credential_list", [config_credential]) with _repository_holding(None) as repository: response = _delete_credential("config-wif", auth=_as_admin) - assert response.status_code == 404, response.text + assert response.status_code == 405, response.text + assert response.headers["allow"] == "GET" + assert "defined in config" in response.json()["error"]["message"] repository.delete_by_name.assert_awaited_once_with("config-wif") assert litellm.credential_list == [config_credential] @@ -1005,7 +1009,7 @@ class TestNonAdminCannotTouchAStoredWifCredential: """POST with the same name carries no WIF field and collides with no DB row, yet ``CredentialAccessor.upsert_credentials`` would replace the admin entry in memory and the periodic config sync would then make the takeover permanent.""" - config_credential = _wif_credential("config-wif") + config_credential = _wif_credential("config-wif", source="config") monkeypatch.setattr(litellm, "credential_list", [config_credential]) with _repository_holding(None) as repository: response = _post_credential( @@ -1024,7 +1028,7 @@ class TestNonAdminCannotTouchAStoredWifCredential: assert litellm.credential_list[0].credential_values["api_key"] == "sk-old" def test_proxy_admin_can_post_over_a_config_only_wif_credential(self, restore_credential_list, monkeypatch): - monkeypatch.setattr(litellm, "credential_list", [_wif_credential("config-wif")]) + monkeypatch.setattr(litellm, "credential_list", [_wif_credential("config-wif", source="config")]) with _repository_holding(None) as repository: response = _post_credential( { @@ -1039,50 +1043,6 @@ class TestNonAdminCannotTouchAStoredWifCredential: repository.create.assert_awaited_once() assert litellm.credential_list[0].credential_values == {"api_key": "sk-rotated"} - def test_non_admin_cannot_rename_a_credential_onto_a_config_only_wif_credential( - self, restore_credential_list, monkeypatch - ): - """PATCH is the other way to shadow: renaming an ordinary credential onto the WIF - credential's name makes ``_sync_in_memory_credential`` upsert the attacker's values over - the admin entry, with no WIF field in the payload and no DB row to collide with.""" - config_credential = _wif_credential("config-wif") - monkeypatch.setattr(litellm, "credential_list", [_plain_credential("mine"), config_credential]) - with _repository_holding(_plain_credential("mine")) as repository: - response = _patch_credential( - "mine", - { - "credential_name": "config-wif", - "credential_values": {"api_key": "sk-attacker"}, - "credential_info": {}, - }, - auth=_as_non_admin, - ) - - assert response.status_code == 403, response.text - assert "anthropic_keycloak_token_url" in response.text - repository.update_by_name.assert_not_awaited() - assert config_credential in litellm.credential_list - assert litellm.credential_list[1].credential_values["api_key"] == "sk-old" - - def test_proxy_admin_can_rename_a_credential_onto_a_config_only_wif_credential( - self, restore_credential_list, monkeypatch - ): - monkeypatch.setattr(litellm, "credential_list", [_plain_credential("mine"), _wif_credential("config-wif")]) - with _repository_holding(_plain_credential("mine")) as repository: - response = _patch_credential( - "mine", - { - "credential_name": "config-wif", - "credential_values": {"api_key": "sk-rotated"}, - "credential_info": {}, - }, - auth=_as_admin, - ) - - assert response.status_code == 200, response.text - repository.update_by_name.assert_awaited_once() - assert [c.credential_name for c in litellm.credential_list] == ["config-wif"] - def test_non_admin_cannot_post_a_null_wif_field(self, restore_credential_list): """Same key-presence rule on the create path: ``{"anthropic_issuer_url": null}`` persists the key, and the resolver reacts to the key.""" @@ -1228,17 +1188,20 @@ def test_delete_credential_still_answers_200_and_drops_the_credential_from_memor def test_delete_credential_leaves_a_credential_that_only_exists_in_memory_in_place(credential_store): """A credential declared in the config yaml is never written to the table, so the delete matches no row. Reporting success would be the same lie: it comes straight back on the next - proxy boot. ``PATCH /credentials/{name}`` already answers 404 for that credential.""" + proxy boot.""" config_only = CredentialItem( credential_name="from-config-yaml", credential_values={"api_key": "sk-config"}, credential_info={}, + source="config", ) credential_store(in_memory=(config_only,), delete_by_name=AsyncMock(return_value=None)) response = _delete_credential("from-config-yaml") - assert response.status_code == 404, response.text + assert response.status_code == 405, response.text + assert response.headers["allow"] == "GET" + assert "defined in config" in response.json()["error"]["message"] assert [credential.credential_name for credential in litellm.credential_list] == ["from-config-yaml"] @@ -1399,3 +1362,222 @@ def test_update_credential_still_accepts_a_body_without_credential_values(creden written = update_by_name.await_args.kwargs["data"] assert json.loads(written["credential_info"]) == {"custom_llm_provider": "openai"} assert set(json.loads(written["credential_values"])) == {"api_key"}, "stored values survive an info-only patch" + + +def _labeled_credential(name: str = "openai-prod", display_name: str | None = "Prod OpenAI") -> CredentialItem: + return CredentialItem( + credential_name=name, + display_name=display_name, + credential_values={"api_key": "sk-old"}, + credential_info={"custom_llm_provider": "openai"}, + ) + + +class TestCredentialDisplayName: + def test_create_stores_the_trimmed_display_name_in_the_row_and_in_memory(self, restore_credential_list): + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "openai-prod", + "display_name": " Prod OpenAI ", + "credential_values": {"api_key": "sk-new"}, + "credential_info": {"custom_llm_provider": "openai"}, + } + ) + + assert response.status_code == 200, response.text + assert repository.create.await_args.kwargs["data"]["display_name"] == "Prod OpenAI" + assert [(c.credential_name, c.display_name) for c in litellm.credential_list] == [ + ("openai-prod", "Prod OpenAI") + ] + + @pytest.mark.parametrize("display_name", ["", " ", "x" * 256]) + def test_create_rejects_an_unusable_display_name(self, restore_credential_list, display_name): + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "openai-prod", + "display_name": display_name, + "credential_values": {"api_key": "sk-new"}, + "credential_info": {}, + } + ) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "display_name" + repository.create.assert_not_awaited() + assert litellm.credential_list == [] + + def test_create_accepts_a_display_name_of_exactly_the_maximum_length(self, restore_credential_list): + with _repository_holding(None) as repository: + response = _post_credential( + { + "credential_name": "openai-prod", + "display_name": "x" * 255, + "credential_values": {"api_key": "sk-new"}, + "credential_info": {}, + } + ) + + assert response.status_code == 200, response.text + assert repository.create.await_args.kwargs["data"]["display_name"] == "x" * 255 + + def test_patch_relabels_without_touching_the_name_or_the_values(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"display_name": "Staging OpenAI", "credential_info": {}}) + + assert response.status_code == 200, response.text + repository.update_by_name.assert_awaited_once() + assert repository.update_by_name.await_args.args[0] == "openai-prod" + written = repository.update_by_name.await_args.kwargs["data"] + assert written["credential_name"] == "openai-prod" + assert written["display_name"] == "Staging OpenAI" + assert json.loads(written["credential_values"]) == {"api_key": "sk-old"} + assert [(c.credential_name, c.display_name, c.credential_values) for c in litellm.credential_list] == [ + ("openai-prod", "Staging OpenAI", {"api_key": "sk-old"}) + ] + + def test_patch_without_display_name_keeps_the_stored_one(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"credential_info": {"description": "rotated"}}) + + assert response.status_code == 200, response.text + assert repository.update_by_name.await_args.kwargs["data"]["display_name"] == "Prod OpenAI" + assert litellm.credential_list[0].display_name == "Prod OpenAI" + + def test_patch_with_null_display_name_clears_it(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"display_name": None, "credential_info": {}}) + + assert response.status_code == 200, response.text + written = repository.update_by_name.await_args.kwargs["data"] + assert "display_name" in written + assert written["display_name"] is None + assert litellm.credential_list[0].display_name is None + + @pytest.mark.parametrize("display_name", ["", " ", "x" * 256]) + def test_patch_rejects_an_unusable_display_name_and_changes_nothing( + self, restore_credential_list, monkeypatch, display_name + ): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"display_name": display_name, "credential_info": {}}) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "display_name" + repository.update_by_name.assert_not_awaited() + assert litellm.credential_list == [_labeled_credential()] + + def test_patch_stores_the_trimmed_display_name(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential("openai-prod", {"display_name": " Staging OpenAI ", "credential_info": {}}) + + assert response.status_code == 200, response.text + assert repository.update_by_name.await_args.kwargs["data"]["display_name"] == "Staging OpenAI" + assert litellm.credential_list[0].display_name == "Staging OpenAI" + + def test_patch_treats_an_empty_credential_name_as_omitted(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential( + "openai-prod", {"credential_name": "", "display_name": "Staging OpenAI", "credential_info": {}} + ) + + assert response.status_code == 200, response.text + assert repository.update_by_name.await_args.args[0] == "openai-prod" + assert repository.update_by_name.await_args.kwargs["data"]["credential_name"] == "openai-prod" + + def test_patch_rejects_a_different_credential_name_and_changes_nothing(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential( + "openai-prod", + { + "credential_name": "openai-production", + "credential_values": {"api_key": "sk-new"}, + "credential_info": {}, + }, + ) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "credential_name" + assert "display_name" in response.json()["error"]["message"] + repository.update_by_name.assert_not_awaited() + assert litellm.credential_list == [_labeled_credential()] + + def test_patch_accepts_the_same_credential_name_echoed_back(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(_labeled_credential()) as repository: + response = _patch_credential( + "openai-prod", + {"credential_name": "openai-prod", "credential_values": {"api_key": "sk-new"}, "credential_info": {}}, + ) + + assert response.status_code == 200, response.text + assert [(c.credential_name, c.credential_values) for c in litellm.credential_list] == [ + ("openai-prod", {"api_key": "sk-new"}) + ] + + def test_patch_on_a_config_credential_is_rejected_as_config_owned(self, restore_credential_list, monkeypatch): + config_credential = _labeled_credential(display_name=None).model_copy(update={"source": "config"}) + monkeypatch.setattr(litellm, "credential_list", [config_credential]) + with _repository_holding(None) as repository: + response = _patch_credential("openai-prod", {"display_name": "Prod", "credential_info": {}}) + + assert response.status_code == 405, response.text + assert response.headers["allow"] == "GET" + assert "defined in config" in response.json()["error"]["message"] + repository.update_by_name.assert_not_awaited() + assert litellm.credential_list == [config_credential] + + def test_patch_on_an_unknown_credential_still_answers_404(self, restore_credential_list): + with _repository_holding(None): + response = _patch_credential("nowhere", {"display_name": "Prod", "credential_info": {}}) + + assert response.status_code == 404, response.text + + @pytest.mark.parametrize("method", ["PATCH", "DELETE"]) + def test_a_db_credential_already_gone_from_the_db_answers_404_not_config_owned( + self, restore_credential_list, monkeypatch, method + ): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential()]) + with _repository_holding(None) as repository: + response = ( + _patch_credential("openai-prod", {"display_name": "Prod", "credential_info": {}}) + if method == "PATCH" + else _delete_credential("openai-prod") + ) + + assert response.status_code == 404, response.text + repository.update_by_name.assert_not_awaited() + + def test_patch_on_a_name_stored_in_the_db_and_config_applies_to_the_row(self, restore_credential_list, monkeypatch): + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential(display_name=None)]) + with _repository_holding(_labeled_credential(display_name=None)) as repository: + response = _patch_credential("openai-prod", {"display_name": "Prod", "credential_info": {}}) + + assert response.status_code == 200, response.text + assert repository.update_by_name.await_args.kwargs["data"]["display_name"] == "Prod" + + def test_reads_return_display_name_and_source(self, restore_credential_list, monkeypatch): + config_credential = CredentialItem( + credential_name="from-config", + credential_values={"api_key": "sk-config-value"}, + credential_info={}, + source="config", + ) + monkeypatch.setattr(litellm, "credential_list", [_labeled_credential(), config_credential]) + + listed = {entry["credential_name"]: entry for entry in _list_credentials().json()["credentials"]} + by_name = _call_as("GET", "/credentials/by_name/openai-prod").json() + config_by_name = _call_as("GET", "/credentials/by_name/from-config").json() + + assert (listed["openai-prod"]["display_name"], listed["openai-prod"]["source"]) == ("Prod OpenAI", "db") + assert (listed["from-config"]["display_name"], listed["from-config"]["source"]) == (None, "config") + assert (by_name["display_name"], by_name["source"]) == ("Prod OpenAI", "db") + assert config_by_name["source"] == "config" + assert listed["openai-prod"]["credential_values"]["api_key"] != "sk-old" diff --git a/tests/unit/proxy/proxy_server/test_proxy_config.py b/tests/unit/proxy/proxy_server/test_proxy_config.py index 912404f91b8..c8c89662583 100644 --- a/tests/unit/proxy/proxy_server/test_proxy_config.py +++ b/tests/unit/proxy/proxy_server/test_proxy_config.py @@ -1851,11 +1851,46 @@ def test_ProxyConfig_load_credential_list_returns_items(): dumped = creds[0].model_dump() assert dumped == { "credential_name": "openai-key", + "display_name": None, "credential_info": {"provider": "openai"}, "credential_values": {"api_key": "sk-x"}, } +def test_ProxyConfig_load_credential_list_tags_every_entry_as_config_defined(): + creds = ProxyConfig().load_credential_list( + { + "credential_list": [ + {"credential_name": "plain", "credential_info": {}, "credential_values": {"api_key": "sk-x"}}, + { + "credential_name": "claims-db", + "source": "db", + "credential_info": {}, + "credential_values": {"api_key": "sk-y"}, + }, + ] + } + ) + assert [(cred.credential_name, cred.source) for cred in creds] == [("plain", "config"), ("claims-db", "config")] + + +@pytest.mark.parametrize("display_name", [2024, True, "Azure Prod"]) +def test_ProxyConfig_load_credential_list_ignores_a_display_name_set_in_config(display_name): + creds = ProxyConfig().load_credential_list( + { + "credential_list": [ + { + "credential_name": "azure_cred", + "display_name": display_name, + "credential_info": {}, + "credential_values": {"api_key": "sk-x"}, + } + ] + } + ) + assert [(cred.credential_name, cred.display_name) for cred in creds] == [("azure_cred", None)] + + def test_ProxyConfig_load_credential_list_invalid_entry_raises(): pc = ProxyConfig() with pytest.raises(ValidationError): @@ -4021,6 +4056,30 @@ async def test_ProxyConfig_get_credentials_reads_from_writer_not_replica(monkeyp reader_inner.litellm_credentialstable.find_many.assert_not_awaited() +@pytest.mark.asyncio +async def test_ProxyConfig_get_credentials_carries_the_stored_display_name_and_marks_rows_as_db( + monkeypatch: pytest.MonkeyPatch, +) -> None: + from litellm.litellm_core_utils.credential_accessor import CredentialAccessor + + pc = ProxyConfig() + fake_prisma = MagicMock() + fake_prisma.db.litellm_credentialstable.find_many = AsyncMock( + return_value=[{**_encrypted_credential_row("labeled-cred", "sk-labeled"), "display_name": "Prod OpenAI"}] + ) + _stub_add_deployment_collaborators(monkeypatch, pc, fake_prisma) + + await pc.get_credentials(prisma_client=fake_prisma) + + loaded = CredentialAccessor.find_credential("labeled-cred") + assert loaded is not None + assert (loaded.display_name, loaded.source, loaded.credential_values) == ( + "Prod OpenAI", + "db", + {"api_key": "sk-labeled"}, + ) + + # --------------------------------------------------------------------------- # ProxyConfig._reschedule_spend_log_cleanup_job # --------------------------------------------------------------------------- diff --git a/tests/unit/repositories/test_repositories.py b/tests/unit/repositories/test_repositories.py index f2e033c9141..cfea6f880fd 100644 --- a/tests/unit/repositories/test_repositories.py +++ b/tests/unit/repositories/test_repositories.py @@ -1487,11 +1487,13 @@ class TestCredentialsRepository: repo._prisma_client.db.litellm_credentialstable._records["my-key"] = { "credential_id": "cred-1", "credential_name": "my-key", + "display_name": "My Key", "credential_values": {"api_key": "encrypted_secret"}, "credential_info": {"provider": "openai"}, } cred = await repo.find_by_name("my-key") assert isinstance(cred, CredentialItem) + assert cred.display_name == "My Key" assert cred.credential_values == {"api_key": "encrypted_secret"} assert cred.credential_info == {"provider": "openai"} diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/credentials/useCredentials.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/credentials/useCredentials.test.ts index ee903628f08..7732ddc8fb3 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/credentials/useCredentials.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/credentials/useCredentials.test.ts @@ -143,6 +143,16 @@ describe("useCredentials", () => { expect(credentialListCall).not.toHaveBeenCalled(); }); + it("does not call the API when the caller disables the query", async () => { + (credentialListCall as any).mockResolvedValue(mockCredentialsResponse); + + const { result } = renderHook(() => useCredentials({ enabled: false }), { wrapper }); + + expect(result.current.isFetched).toBe(false); + expect(result.current.data).toBeUndefined(); + expect(credentialListCall).not.toHaveBeenCalled(); + }); + it("should return empty credentials array when API returns empty data", async () => { // Mock API returning empty credentials array (credentialListCall as any).mockResolvedValue({ credentials: [] }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/credentials/useCredentials.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/credentials/useCredentials.ts index bdbf4445514..cd2ec717ffb 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/credentials/useCredentials.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/credentials/useCredentials.ts @@ -5,11 +5,11 @@ import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; export const credentialsKeys = createQueryKeys("credentials"); -export const useCredentials = () => { +export const useCredentials = ({ enabled = true }: { enabled?: boolean } = {}) => { const { accessToken } = useAuthorized(); return useQuery({ queryKey: credentialsKeys.list({}), queryFn: async () => await credentialListCall(accessToken!), - enabled: Boolean(accessToken), + enabled: enabled && Boolean(accessToken), }); }; diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTab.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTab.test.tsx index 0c93bb234a5..09e00db9fb0 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTab.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTab.test.tsx @@ -82,6 +82,14 @@ vi.mock("../../hooks/models/useModelCostMap", () => ({ })); const mockTeams = [{ team_id: "team-1", team_alias: "Engineering" }]; +const mockCredentials = [ + { credential_name: "openai-prod", display_name: "Prod OpenAI", credential_values: {}, credential_info: {} }, +]; +const mockUseCredentials = vi.hoisted(() => vi.fn()); +vi.mock("../../hooks/credentials/useCredentials", () => ({ + useCredentials: mockUseCredentials, +})); + vi.mock("../../hooks/teams/useTeams", () => ({ useTeams: () => ({ data: mockTeams, isLoading: false, error: null, refetch: vi.fn() }), })); @@ -152,6 +160,35 @@ describe("AllModelsTab", () => { modelsInfoCalls.length = 0; setModelsInfo([makeRow()]); vi.spyOn(useAuthorizedModule, "default").mockReturnValue(MOCK_AUTHORIZED); + mockUseCredentials.mockImplementation(({ enabled = true }: { enabled?: boolean } = {}) => ({ + data: enabled ? { credentials: mockCredentials } : undefined, + isLoading: false, + })); + }); + + describe("credential labels", () => { + const rowWithCredential = () => { + const row = makeRow(); + return { ...row, litellm_params: { ...row.litellm_params, litellm_credential_name: "openai-prod" } }; + }; + + it("shows the credential's display name for a proxy admin", async () => { + setModelsInfo([rowWithCredential()]); + renderWithProviders(); + + expect(await screen.findByText("Prod OpenAI")).toBeInTheDocument(); + expect(mockUseCredentials).toHaveBeenLastCalledWith({ enabled: true }); + }); + + it("skips the admin-only credential list for other roles and shows the raw name", async () => { + vi.spyOn(useAuthorizedModule, "default").mockReturnValue({ ...MOCK_AUTHORIZED, userRole: "Internal User" }); + setModelsInfo([rowWithCredential()]); + renderWithProviders(); + + expect(await screen.findByText("openai-prod")).toBeInTheDocument(); + expect(screen.queryByText("Prod OpenAI")).not.toBeInTheDocument(); + expect(mockUseCredentials).toHaveBeenLastCalledWith({ enabled: false }); + }); }); it("renders the fetched models and the server row count", async () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTab.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTab.tsx index 2217bca0fa0..ea502396072 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTab.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTab.tsx @@ -2,11 +2,14 @@ import { useModelCostMap } from "@/app/(dashboard)/hooks/models/useModelCostMap"; import { useTeams } from "@/app/(dashboard)/hooks/teams/useTeams"; +import { useCredentials } from "@/app/(dashboard)/hooks/credentials/useCredentials"; +import { credentialLabelsByName } from "@/components/shared/credentialOptions"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; import DeleteResourceModal from "@/components/common_components/DeleteResourceModal"; import ModelSettingsModal from "@/components/model_dashboard/ModelSettingsModal/ModelSettingsModal"; import { ModelData } from "@/components/model_dashboard/types"; import { toast } from "@/lib/toast"; +import { isProxyAdminRole } from "@/utils/roles"; import { uiHref } from "@/utils/uiHref"; import { modelDeleteCall, modelPatchUpdateCall } from "@/components/networking"; import { useQueryClient } from "@tanstack/react-query"; @@ -80,6 +83,11 @@ const AllModelsTab = ({ const { data: modelCostMapData, isLoading: isLoadingModelCostMap } = useModelCostMap(); const { accessToken, userId, userRole, isViewOnly } = useAuthorized(); const { data: teams, isLoading: isLoadingTeams } = useTeams(); + const { data: credentialsResponse } = useCredentials({ enabled: isProxyAdminRole(userRole ?? "") }); + const credentialLabels = useMemo( + () => credentialLabelsByName(credentialsResponse?.credentials ?? []), + [credentialsResponse], + ); const queryClient = useQueryClient(); const [tableState, setTableState] = useQueryStates(TABLE_STATE); @@ -324,6 +332,7 @@ const AllModelsTab = ({ onDeleteClick={handleDeleteClick} onTogglePauseClick={handleTogglePause} pausingModelId={pausingModelId} + credentialLabels={credentialLabels} /> {modelViewMode === "current_team" && ( diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.test.tsx index be6130b0288..adbeb70a9c5 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.test.tsx @@ -157,6 +157,19 @@ describe("AllModelsTable", () => { expect(screen.getByText("Manual")).toBeInTheDocument(); }); + it("renders the credential's display name when one is set and keeps the name as the tooltip", () => { + render( + , + ); + expect(screen.getByText("Prod OpenAI")).toBeInTheDocument(); + expect(screen.queryByText("openai-prod")).not.toBeInTheDocument(); + expect(screen.getByTitle("openai-prod")).toBeInTheDocument(); + }); + it("shows 'Defined in config' for a config model and the creator for a DB model", () => { const { rerender } = render(); expect(screen.getByText("alice")).toBeInTheDocument(); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.tsx index 2a52bdfb46e..e9eca183a7a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/AllModelsTable.tsx @@ -80,6 +80,7 @@ interface AllModelsTableProps { onDeleteClick: (modelId: string) => void; onTogglePauseClick: (modelId: string, blocked: boolean) => void | Promise; pausingModelId: string | null; + credentialLabels?: ReadonlyMap; } function EmptyState() { @@ -128,6 +129,7 @@ export function AllModelsTable({ onDeleteClick, onTogglePauseClick, pausingModelId, + credentialLabels, }: AllModelsTableProps) { const [filtersOpen, setFiltersOpen] = useState(false); @@ -141,9 +143,20 @@ export function AllModelsTable({ onDeleteClick, onTogglePauseClick, pausingModelId, + credentialLabels, }; return getModelsTableColumns(columnDeps); - }, [userRole, userID, isViewOnly, onModelIdClick, onTeamIdClick, onDeleteClick, onTogglePauseClick, pausingModelId]); + }, [ + userRole, + userID, + isViewOnly, + onModelIdClick, + onTeamIdClick, + onDeleteClick, + onTogglePauseClick, + pausingModelId, + credentialLabels, + ]); const modelGroupOptions = useMemo( () => [ diff --git a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelsTableColumns.tsx b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelsTableColumns.tsx index 9581d3db198..c277f527f86 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelsTableColumns.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/components/ModelsTableColumns.tsx @@ -160,7 +160,7 @@ function CredentialsHeader() { ); } -function CredentialsCell({ credentialName }: { credentialName: string | undefined }) { +function CredentialsCell({ credentialName, label }: { credentialName: string | undefined; label: string | undefined }) { if (!credentialName) { return ( @@ -173,7 +173,7 @@ function CredentialsCell({ credentialName }: { credentialName: string | undefine return ( - {credentialName} + {label ?? credentialName} ); } @@ -364,6 +364,7 @@ export interface ModelsTableColumnDeps { onDeleteClick?: (modelId: string) => void; onTogglePauseClick?: (modelId: string, blocked: boolean) => void | Promise; pausingModelId?: string | null; + credentialLabels?: ReadonlyMap; } export const getModelsTableColumns = ({ @@ -375,6 +376,7 @@ export const getModelsTableColumns = ({ onDeleteClick, onTogglePauseClick, pausingModelId, + credentialLabels, }: ModelsTableColumnDeps): ColumnDef[] => [ { id: MODEL_ID_COLUMN_ID, @@ -412,7 +414,15 @@ export const getModelsTableColumns = ({ enableSorting: false, size: 180, minSize: 110, - cell: ({ row }) => , + cell: ({ row }) => { + const credentialName = row.original.litellm_params?.litellm_credential_name; + return ( + + ); + }, }, { id: CREATED_BY_COLUMN_ID, diff --git a/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/VectorStoreForm.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/VectorStoreForm.integration.test.tsx index 3b9c714d354..d7ec325653f 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/VectorStoreForm.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/VectorStoreForm.integration.test.tsx @@ -30,7 +30,14 @@ const renderForm = () => onCancel={vi.fn()} onSuccess={onSuccess} accessToken="test-token" - credentials={[{ credential_name: "bedrock-prod", credential_info: {}, credential_values: {} }]} + credentials={[ + { + credential_name: "bedrock-prod", + display_name: "Bedrock Prod", + credential_info: {}, + credential_values: {}, + }, + ]} />, ); @@ -166,6 +173,43 @@ describe("VectorStoreForm submit payload", () => { }); }); + it("shows the credential display name beside the name, searches by it, and still submits the name", async () => { + const user = setupUser(); + renderForm(); + + const picker = screen.getByPlaceholderText("Select or search for existing credentials"); + await user.click(picker); + + const option = await screen.findByRole("option", { name: /bedrock-prod/ }); + expect(option).toHaveTextContent("bedrock-prod"); + expect(option).toHaveTextContent("Bedrock Prod"); + + await user.type(picker, "bedrock prod"); + await user.click(await screen.findByRole("option", { name: /bedrock-prod/ })); + await user.type(screen.getByPlaceholderText("Enter vector store ID from your provider"), "vs-labeled"); + await submit(user); + + await vi.waitFor(() => expect(mockCreate).toHaveBeenCalledTimes(1)); + expect(createdPayload().litellm_credential_name).toBe("bedrock-prod"); + }); + + it("sends no credential when None is picked after a credential", async () => { + const user = setupUser(); + renderForm(); + + const picker = screen.getByPlaceholderText("Select or search for existing credentials"); + await user.click(picker); + await user.click(await screen.findByRole("option", { name: /bedrock-prod/ })); + expect(picker).toHaveValue("Bedrock Prod"); + await user.click(picker); + await user.click(await screen.findByRole("option", { name: "None" })); + await user.type(screen.getByPlaceholderText("Enter vector store ID from your provider"), "vs-none"); + await submit(user); + + await vi.waitFor(() => expect(mockCreate).toHaveBeenCalledTimes(1)); + expect(createdPayload().litellm_credential_name).toBeUndefined(); + }); + it("blocks the request and reports invalid metadata JSON instead of submitting", async () => { const user = setupUser(); renderForm(); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/VectorStoreForm.tsx b/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/VectorStoreForm.tsx index 01abfe82920..d82f56f3870 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/VectorStoreForm.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/VectorStoreForm.tsx @@ -33,6 +33,8 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@ import { Textarea } from "@/components/ui/textarea"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; import { useZodForm } from "@/lib/forms/useZodForm"; +import { SearchSelect } from "@/components/shared/SearchSelect"; +import { credentialOptions } from "@/components/shared/credentialOptions"; const EMBEDDING_MODEL_RENAME_PROVIDERS = new Set(["milvus", "valkey", "mongodb"]); @@ -159,11 +161,6 @@ const EMPTY_VALUES: VectorStoreFormValues = { valkey_embedding_field: "embedding", }; -interface CredentialOption { - label: string; - value: string | null; -} - const labelWithHint = (label: string, hint: string): React.ReactNode => ( <> {label} @@ -225,14 +222,6 @@ const VectorStoreForm: React.FC = ({ loadModels(); }, [accessToken]); - const credentialOptions: CredentialOption[] = [ - { value: null, label: "None" }, - ...credentials.map((credential) => ({ - value: credential.credential_name, - label: credential.credential_name, - })), - ]; - const makeProviderChangeHandler = (onChange: (provider: string) => void) => (provider: string | null) => { if (provider === null) return; onChange(provider); @@ -495,35 +484,14 @@ const VectorStoreForm: React.FC = ({ "Optionally select API provider credentials for this vector store eg. Bedrock API KEY", )} > - {({ id, value, onChange, "aria-invalid": ariaInvalid, "aria-describedby": ariaDescribedBy }) => ( - option.value === value) ?? null} - onValueChange={(option: CredentialOption | null) => onChange(option ? option.value : undefined)} - itemToStringLabel={(option: CredentialOption) => option.label} - isItemEqualToValue={(option: CredentialOption, selected: CredentialOption) => - option.value === selected.value - } - > - - - No matching credentials - - {(option: CredentialOption) => ( - - {option.label} - - )} - - - + {({ id, value, onChange }) => ( + onChange(selected === "" || selected === null ? undefined : selected)} + /> )} diff --git a/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/vector_store_info.tsx b/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/vector_store_info.tsx index b8d82eee0a2..71d6a4545f5 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/vector_store_info.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/vector-stores/_components/vector_store_info.tsx @@ -20,20 +20,14 @@ import { StatusBadge } from "@/components/shared/table_cells"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Card, CardContent } from "@/components/ui/card"; -import { - Combobox, - ComboboxContent, - ComboboxEmpty, - ComboboxInput, - ComboboxItem, - ComboboxList, -} from "@/components/ui/combobox"; import { Input } from "@/components/ui/input"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { Textarea } from "@/components/ui/textarea"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; import { useZodForm } from "@/lib/forms/useZodForm"; +import { SearchSelect } from "@/components/shared/SearchSelect"; +import { credentialOptions } from "@/components/shared/credentialOptions"; interface VectorStoreInfoViewProps { vectorStoreId: string; @@ -68,11 +62,6 @@ const toFormValues = (vectorStore: VectorStore): VectorStoreEditValues => ({ litellm_credential_name: vectorStore.litellm_credential_name, }); -interface CredentialOption { - label: string; - value: string | null; -} - const labelWithHint = (label: string, hint: string): React.ReactNode => ( <> {label} @@ -175,14 +164,6 @@ const VectorStoreInfoView: React.FC = ({ } }; - const credentialOptions: CredentialOption[] = [ - { value: null, label: "None" }, - ...credentials.map((credential) => ({ - value: credential.credential_name, - label: credential.credential_name, - })), - ]; - if (loadFailed) { return (
@@ -321,43 +302,16 @@ const VectorStoreInfoView: React.FC = ({

- {({ - id, - value, - onChange, - "aria-invalid": ariaInvalid, - "aria-describedby": ariaDescribedBy, - }) => ( - option.value === value) ?? null} - onValueChange={(option: CredentialOption | null) => - onChange(option ? option.value : undefined) + {({ id, value, onChange }) => ( + + onChange(selected === "" || selected === null ? undefined : selected) } - itemToStringLabel={(option: CredentialOption) => option.label} - isItemEqualToValue={(option: CredentialOption, selected: CredentialOption) => - option.value === selected.value - } - > - - - No matching credentials - - {(option: CredentialOption) => ( - - {option.label} - - )} - - - + /> )} diff --git a/ui/litellm-dashboard/src/components/ModelInfoEditForm.test.tsx b/ui/litellm-dashboard/src/components/ModelInfoEditForm.test.tsx new file mode 100644 index 00000000000..1a05374d883 --- /dev/null +++ b/ui/litellm-dashboard/src/components/ModelInfoEditForm.test.tsx @@ -0,0 +1,117 @@ +import { render, screen } from "@testing-library/react"; +import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; + +import type { CredentialItem } from "@/components/networking"; + +import ModelInfoEditForm from "./ModelInfoEditForm"; + +vi.mock("@/components/networking", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + vectorStoreListCall: vi.fn().mockResolvedValue({ data: [] }), + }; +}); + +const credentialsList: CredentialItem[] = [ + { + credential_name: "openai-main", + display_name: "Main OpenAI", + credential_values: {}, + credential_info: { custom_llm_provider: "openai" }, + }, +]; + +const renderForm = ({ + onSubmit = vi.fn().mockResolvedValue(undefined), + isEditing = true, + litellmParams = { model: "gpt-4o" } as Record, +} = {}) => + render( + , + ); + +describe("ModelInfoEditForm existing-credentials picker", () => { + it("shows the display name beside the credential name, searches by it, and submits the name", async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + renderForm({ onSubmit }); + + const picker = await screen.findByPlaceholderText("Select or search for existing credentials"); + await user.click(picker); + + const option = await screen.findByRole("option", { name: /openai-main/ }); + expect(option).toHaveTextContent("openai-main"); + expect(option).toHaveTextContent("Main OpenAI"); + + await user.type(picker, "main openai"); + await user.click(await screen.findByRole("option", { name: /openai-main/ })); + await user.click(screen.getByRole("button", { name: /save changes/i })); + + await vi.waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0].litellm_credential_name).toBe("openai-main"); + }); + + it("keeps the attached credential when the search text is emptied and only clears through None", async () => { + const onSubmit = vi.fn().mockResolvedValue(undefined); + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + renderForm({ onSubmit, litellmParams: { model: "gpt-4o", litellm_credential_name: "openai-main" } }); + + const picker = await screen.findByPlaceholderText("Select or search for existing credentials"); + expect(picker).toHaveValue("Main OpenAI"); + await user.clear(picker); + await user.tab(); + await user.click(screen.getByRole("button", { name: /save changes/i })); + await vi.waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(onSubmit.mock.calls[0][0].litellm_credential_name).toBe("openai-main"); + + await user.click(picker); + await user.click(await screen.findByRole("option", { name: "None" })); + await user.click(screen.getByRole("button", { name: /save changes/i })); + await vi.waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(2)); + expect(onSubmit.mock.calls[1][0].litellm_credential_name).toBeNull(); + }); + + it("shows the attached credential's display name when not editing", () => { + renderForm({ isEditing: false, litellmParams: { model: "gpt-4o", litellm_credential_name: "openai-main" } }); + + expect(screen.getByText("Main OpenAI")).toBeInTheDocument(); + expect(screen.queryByText("openai-main")).not.toBeInTheDocument(); + }); + + it("falls back to the credential name, then Manual, when not editing", () => { + const { unmount } = renderForm({ + isEditing: false, + litellmParams: { model: "gpt-4o", litellm_credential_name: "not-listed" }, + }); + expect(screen.getByText("not-listed")).toBeInTheDocument(); + unmount(); + + renderForm({ isEditing: false }); + expect(screen.getByText("Manual")).toBeInTheDocument(); + }); +}); diff --git a/ui/litellm-dashboard/src/components/ModelInfoEditForm.tsx b/ui/litellm-dashboard/src/components/ModelInfoEditForm.tsx index 9ee49c262d8..2a451711407 100644 --- a/ui/litellm-dashboard/src/components/ModelInfoEditForm.tsx +++ b/ui/litellm-dashboard/src/components/ModelInfoEditForm.tsx @@ -14,6 +14,8 @@ import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; +import { SearchSelect } from "@/components/shared/SearchSelect"; +import { credentialLabelsByName, credentialOptions } from "@/components/shared/credentialOptions"; import { Switch } from "@/components/ui/switch"; import { Textarea } from "@/components/ui/textarea"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; @@ -345,6 +347,9 @@ const ChipList: React.FC<{ values: unknown; emptyLabel: string }> = ({ values, e ); }; +const attachedCredentialLabel = (credentialName: string | null | undefined, credentials: CredentialItem[]): string => + credentialName ? credentialLabelsByName(credentials).get(credentialName) ?? credentialName : "Manual"; + const ModelInfoEditForm: React.FC = ({ localModelData, modelData, @@ -634,36 +639,23 @@ const ModelInfoEditForm: React.FC = ({ Existing Credentials {isEditing ? ( - {({ id, value, onChange, onBlur }) => { - const items: { value: string | null; label: string }[] = [ - { value: null, label: "None" }, - ...credentialsList.map((credential) => ({ - value: credential.credential_name, - label: credential.credential_name, - })), - ]; - return ( - - ); - }} + {({ id, value, onChange }) => ( + { + if (selected !== null) onChange(selected === "" ? null : selected); + }} + /> + )} ) : ( - {localModelData.litellm_params?.litellm_credential_name || "Manual"} + + {attachedCredentialLabel(localModelData.litellm_params?.litellm_credential_name, credentialsList)} + )}
diff --git a/ui/litellm-dashboard/src/components/add_model/AddModelForm.integration.test.tsx b/ui/litellm-dashboard/src/components/add_model/AddModelForm.integration.test.tsx index 5fb95331114..45014d06a3a 100644 --- a/ui/litellm-dashboard/src/components/add_model/AddModelForm.integration.test.tsx +++ b/ui/litellm-dashboard/src/components/add_model/AddModelForm.integration.test.tsx @@ -150,6 +150,7 @@ const createTestProps = (userRole = "proxy_admin", userId = "user-1", isTeamAdmi const credentials: CredentialItem[] = [ { credential_name: "test-credential", + display_name: "Prod OpenAI", credential_values: {}, credential_info: { custom_llm_provider: "openai", @@ -299,6 +300,47 @@ describe("AddModelForm", () => { expect(screen.queryByRole("switch")).not.toBeInTheDocument(); }); + describe("the existing-credentials picker", () => { + const openPicker = async () => { + const mockUseAuthorized = vi.mocked(await import("@/app/(dashboard)/hooks/useAuthorized")); + mockUseAuthorized.default.mockReturnValue(mockAuthorizedUser("proxy_admin", "user-1", true)); + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + const props = createTestProps(); + renderWithProviders(); + const input = await screen.findByPlaceholderText("Select or search for existing credentials"); + await user.click(input); + return { user, input, form: props.form }; + }; + + it("shows each credential's display name and credential name together", async () => { + await openPicker(); + + const option = await screen.findByRole("option", { name: /test-credential/ }); + expect(option).toHaveTextContent("test-credential"); + expect(option).toHaveTextContent("Prod OpenAI"); + }); + + it("filters by the display name, not only the credential name", async () => { + const { user, input } = await openPicker(); + + await user.type(input, "prod open"); + + expect(await screen.findByRole("option", { name: /test-credential/ })).toBeInTheDocument(); + }); + + it("selecting by display name sets litellm_credential_name to the credential name", async () => { + const { user, input, form } = await openPicker(); + expect(screen.getByText("OR")).toBeInTheDocument(); + + await user.type(input, "prod open"); + await user.click(await screen.findByRole("option", { name: /test-credential/ })); + + expect(input).toHaveValue("Prod OpenAI"); + expect(form.getValues("litellm_credential_name")).toBe("test-credential"); + expect(screen.queryByText("OR")).not.toBeInTheDocument(); + }); + }); + it("should display the provider field and the Test Connect / Add Model buttons", async () => { const mockUseAuthorized = vi.mocked(await import("@/app/(dashboard)/hooks/useAuthorized")); mockUseAuthorized.default.mockReturnValue(mockAuthorizedUser("proxy_admin", "user-1", true)); diff --git a/ui/litellm-dashboard/src/components/add_model/AddModelForm.tsx b/ui/litellm-dashboard/src/components/add_model/AddModelForm.tsx index b5b6c439dc2..2b3593e0149 100644 --- a/ui/litellm-dashboard/src/components/add_model/AddModelForm.tsx +++ b/ui/litellm-dashboard/src/components/add_model/AddModelForm.tsx @@ -8,6 +8,7 @@ import { Field, FieldLabel } from "@/components/ui/field"; import { Card, CardContent } from "@/components/ui/card"; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"; import { SearchSelect, type SearchSelectOption } from "@/components/shared/SearchSelect"; +import { credentialOptions } from "@/components/shared/credentialOptions"; import { SimpleTooltip } from "@/components/ui/tooltip"; import { Info } from "lucide-react"; import { Alert, AlertDescription, AlertTitle } from "@/components/shared/Alert"; @@ -151,16 +152,7 @@ const AddModelForm: React.FC = ({ [sortedProviderMetadata], ); - const credentialOptions: SearchSelectOption[] = useMemo( - () => [ - { label: "None", value: "" }, - ...credentials.map((credential) => ({ - label: credential.credential_name, - value: credential.credential_name, - })), - ], - [credentials], - ); + const credentialSelectOptions: SearchSelectOption[] = useMemo(() => credentialOptions(credentials), [credentials]); const applyProviderSelection = (provider: string | null) => { setSelectedProvider(provider); @@ -323,7 +315,7 @@ const AddModelForm: React.FC = ({ control.onChange(value === "" ? null : value)} /> diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx index 8d94ae0d213..4fa3a3cc6bd 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.test.tsx @@ -1,5 +1,6 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { render, screen, waitFor } from "@testing-library/react"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import userEvent, { PointerEventsCheckLevel } from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { Providers } from "../provider_info_helpers"; import { CredentialItem } from "../networking"; @@ -125,4 +126,142 @@ describe("CredentialModal", () => { expect(screen.getByLabelText("Credential Name:")).toBeDisabled(); }); }); + + describe("display name", () => { + const fillRequiredAddFields = async (user: ReturnType) => { + fireEvent.change(screen.getByLabelText("Credential Name:"), { target: { value: "new-cred" } }); + const providerInput = await screen.findByPlaceholderText("Select a provider"); + await user.click(providerInput); + await user.click(await screen.findByText("OpenAI")); + fireEvent.change(await screen.findByLabelText("OpenAI API Key"), { target: { value: "sk-test" } }); + }; + + it("submits the display name with the credential name in add mode", async () => { + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + const onSubmit = vi.fn(); + renderModal({ mode: "add", onSubmit }); + + await fillRequiredAddFields(user); + fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: "Prod" } }); + fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0]).toMatchObject({ credential_name: "new-cred", display_name: "Prod" }); + }); + + it("submits no display_name key when the display name is left blank in add mode", async () => { + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + const onSubmit = vi.fn(); + renderModal({ mode: "add", onSubmit }); + + await fillRequiredAddFields(user); + fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0]).not.toHaveProperty("display_name"); + }); + + it("prefills the display name and submits an edited one", async () => { + const onSubmit = vi.fn(); + renderModal({ + mode: "edit", + onSubmit, + existingCredential: { ...mockCredential, display_name: "Prod" }, + }); + + const displayNameInput = screen.getByLabelText("Display Name:") as HTMLInputElement; + expect(displayNameInput.value).toBe("Prod"); + + fireEvent.change(displayNameInput, { target: { value: "Staging" } }); + fireEvent.click(screen.getByRole("button", { name: "Update Credential" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0]).toMatchObject({ + credential_name: "test-credential", + display_name: "Staging", + }); + }); + + it("submits display_name: null when the display name is cleared in edit mode", async () => { + const onSubmit = vi.fn(); + renderModal({ + mode: "edit", + onSubmit, + existingCredential: { ...mockCredential, display_name: "Prod" }, + }); + + fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: "" } }); + fireEvent.click(screen.getByRole("button", { name: "Update Credential" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0].display_name).toBeNull(); + }); + + it("trims the display name before submitting in add mode", async () => { + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + const onSubmit = vi.fn(); + renderModal({ mode: "add", onSubmit }); + + await fillRequiredAddFields(user); + fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: " Prod " } }); + fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0]).toMatchObject({ credential_name: "new-cred", display_name: "Prod" }); + }); + + it("submits display_name: null when the display name is whitespace-only in edit mode", async () => { + const onSubmit = vi.fn(); + renderModal({ + mode: "edit", + onSubmit, + existingCredential: { ...mockCredential, display_name: "Prod OpenAI" }, + }); + + fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: " " } }); + fireEvent.click(screen.getByRole("button", { name: "Update Credential" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0].display_name).toBeNull(); + }); + + it("leaves display_name out of the edit when it was not changed", async () => { + const onSubmit = vi.fn(); + renderModal({ + mode: "edit", + onSubmit, + existingCredential: { ...mockCredential, display_name: "Prod" }, + }); + + fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: " Prod " } }); + fireEvent.click(screen.getByRole("button", { name: "Update Credential" })); + + await waitFor(() => expect(onSubmit).toHaveBeenCalled()); + expect(onSubmit.mock.calls[0][0]).not.toHaveProperty("display_name"); + }); + + it("accepts a 255-character display name and blocks a 256-character one", async () => { + const user = userEvent.setup({ pointerEventsCheck: PointerEventsCheckLevel.Never }); + const onSubmit = vi.fn(); + renderModal({ mode: "add", onSubmit }); + + await fillRequiredAddFields(user); + fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: "x".repeat(256) } }); + fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); + expect(await screen.findByText("Display name must be at most 255 characters")).toBeInTheDocument(); + expect(onSubmit).not.toHaveBeenCalled(); + + fireEvent.change(screen.getByLabelText("Display Name:"), { target: { value: "x".repeat(255) } }); + fireEvent.click(screen.getByRole("button", { name: "Add Credential" })); + await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1)); + expect(onSubmit.mock.calls[0][0].display_name).toBe("x".repeat(255)); + }); + + it("keeps the credential name read-only while the display name stays editable in edit mode", () => { + renderModal({ mode: "edit", existingCredential: { ...mockCredential, display_name: "Prod" } }); + + expect(screen.getByLabelText("Credential Name:")).toBeDisabled(); + expect(screen.getByLabelText("Display Name:")).toBeEnabled(); + }); + }); }); diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx index 202274d2aa2..61afd5eea99 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialModal.tsx @@ -62,6 +62,22 @@ interface CredentialModalProps { const sameProvider = (left: string | null | undefined, right: string | null | undefined): boolean => (left ?? "").toLowerCase() === (right ?? "").toLowerCase(); +const DISPLAY_NAME_MAX_LENGTH = 255; + +const displayNameChange = ( + value: unknown, + existingCredential: CredentialItem | null | undefined, +): { display_name?: string | null } => { + const trimmed = typeof value === "string" ? value.trim() : ""; + if (!existingCredential) { + return trimmed ? { display_name: trimmed } : {}; + } + if (trimmed === (existingCredential.display_name ?? "")) { + return {}; + } + return { display_name: trimmed || null }; +}; + const initialFormValues = ( existingCredential: CredentialItem | null | undefined, initialProvider: string | null | undefined, @@ -69,6 +85,7 @@ const initialFormValues = ( if (existingCredential) { return { credential_name: existingCredential.credential_name, + display_name: existingCredential.display_name ?? "", custom_llm_provider: existingCredential.credential_info.custom_llm_provider, ...Object.fromEntries( Object.entries(existingCredential.credential_values || {}).map(([key, value]) => [key, value ?? null]), @@ -127,6 +144,7 @@ export default function CredentialModal({ const meta = { credential_name: values.credential_name, custom_llm_provider: values.custom_llm_provider, + ...displayNameChange(values.display_name, existingCredential), }; if (!isEdit) { onSubmit({ ...meta, ...buildCreateCredentialValues(withoutRestrictedFields(values), selection) }, []); @@ -170,12 +188,36 @@ export default function CredentialModal({ value={typeof control.value === "string" ? control.value : ""} onChange={control.onChange} onBlur={control.onBlur} - placeholder="Enter a friendly name for these credentials" + placeholder="Unique name that models reference this credential by" disabled={isEdit} /> )} + + typeof value !== "string" || + value.trim().length <= DISPLAY_NAME_MAX_LENGTH || + `Display name must be at most ${DISPLAY_NAME_MAX_LENGTH} characters`, + }, + }} + className="mb-4" + > + {(control) => ( + + )} + + { expect(await window.navigator.clipboard.readText()).toBe("b-openai-key"); }); + it("should show the display name with the credential name beneath it, and the bare name when unset", () => { + const credentials: CredentialItem[] = [ + { + credential_name: "openai-prod", + display_name: "Prod OpenAI", + credential_values: {}, + credential_info: { custom_llm_provider: "openai" }, + }, + { credential_name: "plain-key", credential_values: {}, credential_info: { custom_llm_provider: "openai" } }, + ]; + render(); + + const labeledRow = screen.getByRole("row", { name: /Prod OpenAI/ }); + expect(within(labeledRow).getByText("openai-prod")).toBeInTheDocument(); + const plainRow = screen.getByRole("row", { name: /plain-key/ }); + expect(within(plainRow).getAllByText("plain-key")).toHaveLength(1); + }); + + it("should sort by the display name when one is set", () => { + const credentials: CredentialItem[] = [ + { credential_name: "a-key", display_name: "zulu", credential_values: {}, credential_info: {} }, + { credential_name: "b-key", credential_values: {}, credential_info: {} }, + ]; + render(); + + const rows = screen.getAllByRole("row").slice(1); + expect(within(rows[0]).getByText("b-key")).toBeInTheDocument(); + expect(within(rows[1]).getByText("zulu")).toBeInTheDocument(); + }); + + it("should badge a config credential and block editing and deleting it", async () => { + const user = userEvent.setup(); + const credentials: CredentialItem[] = [ + { credential_name: "from-config", source: "config", credential_values: {}, credential_info: {} }, + { credential_name: "from-db", source: "db", credential_values: {}, credential_info: {} }, + ]; + render(); + + expect(within(screen.getByRole("row", { name: /from-config/ })).getByText("Config")).toBeInTheDocument(); + expect(within(screen.getByRole("row", { name: /from-db/ })).queryByText("Config")).not.toBeInTheDocument(); + + await user.click(screen.getByTestId("credential-actions-from-config")); + expect(await screen.findByTestId("credential-config-owned-hint")).toBeInTheDocument(); + const edit = screen.getByTestId("credential-action-edit"); + const remove = screen.getByTestId("credential-action-delete"); + expect(edit).toHaveAttribute("data-disabled"); + expect(remove).toHaveAttribute("data-disabled"); + await user.click(edit); + await user.click(remove); + expect(mockOnEdit).not.toHaveBeenCalled(); + expect(mockOnDelete).not.toHaveBeenCalled(); + }); + + it("should keep editing enabled for a DB credential", async () => { + const user = userEvent.setup(); + const credentials: CredentialItem[] = [ + { credential_name: "from-db", source: "db", credential_values: {}, credential_info: {} }, + ]; + render(); + + await user.click(screen.getByTestId("credential-actions-from-db")); + expect(await screen.findByTestId("credential-action-edit")).not.toHaveAttribute("data-disabled"); + expect(screen.queryByTestId("credential-config-owned-hint")).not.toBeInTheDocument(); + }); + it("should not render the actions menu when the user cannot modify credentials", () => { render(); // Read parity: names still render... diff --git a/ui/litellm-dashboard/src/components/model_add/CredentialsTableColumns.tsx b/ui/litellm-dashboard/src/components/model_add/CredentialsTableColumns.tsx index 69ea0acd330..581b4d3da32 100644 --- a/ui/litellm-dashboard/src/components/model_add/CredentialsTableColumns.tsx +++ b/ui/litellm-dashboard/src/components/model_add/CredentialsTableColumns.tsx @@ -6,13 +6,16 @@ import { Copy, MoreHorizontal, Pencil, Trash2 } from "lucide-react"; import { CredentialItem } from "@/components/networking"; import { getProviderLogoAndName } from "@/components/provider_info_helpers"; import { DataTableSortHeader } from "@/components/shared/DataTable"; +import { credentialLabel } from "@/components/shared/credentialOptions"; import { IdentityCell } from "@/components/shared/table_cells"; import { Badge } from "@/components/ui/badge"; import { buttonVariants } from "@/components/ui/button"; import { DropdownMenu, DropdownMenuContent, + DropdownMenuGroup, DropdownMenuItem, + DropdownMenuLabel, DropdownMenuSeparator, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; @@ -51,6 +54,7 @@ interface CredentialRowActionsProps { } function CredentialRowActions({ credential, onEdit, onDelete }: CredentialRowActionsProps) { + const configOwned = credential.source === "config"; return ( - onEdit(credential)}> + {configOwned && ( + <> + + + Defined in config.yaml. Edit the file to change or delete it. + + + + + )} + onEdit(credential)} + > Edit @@ -76,6 +94,7 @@ function CredentialRowActions({ credential, onEdit, onDelete }: CredentialRowAct onDelete(credential)} > @@ -100,13 +119,19 @@ export const getCredentialsTableColumns = ({ const dataColumns: ColumnDef[] = [ { id: "credential_name", - accessorKey: "credential_name", + accessorFn: credentialLabel, meta: { title: "Credential Name" }, header: ({ column }) => , size: 260, enableSorting: true, cell: ({ row }) => ( - + Config : undefined} + className="max-w-72" + titleClassName="font-medium" + /> ), }, { diff --git a/ui/litellm-dashboard/src/components/model_add/credential_form_helpers.test.ts b/ui/litellm-dashboard/src/components/model_add/credential_form_helpers.test.ts index d7218f6f4d3..efad20dddaf 100644 --- a/ui/litellm-dashboard/src/components/model_add/credential_form_helpers.test.ts +++ b/ui/litellm-dashboard/src/components/model_add/credential_form_helpers.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it, vi } from "vitest"; import { Providers } from "../provider_info_helpers"; -import { resetCredentialFormOnProviderChange } from "./credential_form_helpers"; +import { + buildCredential, + resetCredentialFormOnProviderChange, + withoutRestrictedFields, +} from "./credential_form_helpers"; /** * Build a minimal FormInstance stub that records calls. We don't depend @@ -58,6 +62,14 @@ describe("resetCredentialFormOnProviderChange", () => { expect(fields.credential_name).toBe("my-prod-key"); }); + it("preserves a display name typed before the provider was picked", () => { + const { stub, fields } = makeFormStub({ credential_name: "my-prod-key", display_name: "Prod OpenAI" }); + + resetCredentialFormOnProviderChange(stub, Providers.OpenAI, vi.fn()); + + expect(fields.display_name).toBe("Prod OpenAI"); + }); + it("updates custom_llm_provider and selectedProvider state to the new value", () => { const { stub, fields } = makeFormStub({ credential_name: "x" }); const setSelectedProvider = vi.fn(); @@ -81,3 +93,23 @@ describe("resetCredentialFormOnProviderChange", () => { expect(credentialNameCalls).toHaveLength(0); }); }); + +describe("buildCredential", () => { + const values = { credential_name: "openai-prod", custom_llm_provider: "openai", api_key: "sk-test" }; + + it.each([ + ["a label", "Prod OpenAI"], + ["a cleared label", null], + ])("sends %s as a top-level display_name, never as a credential value", (_, displayName) => { + const formValues = { ...values, display_name: displayName }; + + const credential = buildCredential(formValues, withoutRestrictedFields(formValues)); + + expect(credential.display_name).toBe(displayName); + expect(credential.credential_values).toEqual({ api_key: "sk-test" }); + }); + + it("leaves display_name out when the form never set it", () => { + expect(buildCredential(values, withoutRestrictedFields(values))).not.toHaveProperty("display_name"); + }); +}); diff --git a/ui/litellm-dashboard/src/components/model_add/credential_form_helpers.ts b/ui/litellm-dashboard/src/components/model_add/credential_form_helpers.ts index 4db1bb78371..7de443ae7cc 100644 --- a/ui/litellm-dashboard/src/components/model_add/credential_form_helpers.ts +++ b/ui/litellm-dashboard/src/components/model_add/credential_form_helpers.ts @@ -21,10 +21,11 @@ interface CredentialFormAdapter { * The credential name is preserved because it's a user-supplied label * that shouldn't reset just because the admin re-selected a provider. */ -const restrictedFields: readonly string[] = ["credential_name", "custom_llm_provider"]; +const restrictedFields: readonly string[] = ["credential_name", "display_name", "custom_llm_provider"]; export const buildCredential = (values: Record, credentialValues: Record) => ({ credential_name: values.credential_name as string, + ...(values.display_name !== undefined ? { display_name: values.display_name as string | null } : {}), credential_values: credentialValues, credential_info: { custom_llm_provider: values.custom_llm_provider as string, @@ -40,10 +41,14 @@ export function resetCredentialFormOnProviderChange( setSelectedProvider: (p: string | null) => void, ): void { const preservedName = form.getFieldValue("credential_name"); + const preservedDisplayName = form.getFieldValue("display_name"); form.resetFields(); if (preservedName !== undefined) { form.setFieldValue("credential_name", preservedName); } + if (preservedDisplayName !== undefined) { + form.setFieldValue("display_name", preservedDisplayName); + } setSelectedProvider(newProvider); form.setFieldValue("custom_llm_provider", newProvider); } diff --git a/ui/litellm-dashboard/src/components/model_info_view.test.tsx b/ui/litellm-dashboard/src/components/model_info_view.test.tsx index 037ebc4040e..f555911eb5c 100644 --- a/ui/litellm-dashboard/src/components/model_info_view.test.tsx +++ b/ui/litellm-dashboard/src/components/model_info_view.test.tsx @@ -1542,6 +1542,27 @@ describe("ModelInfoView", () => { expect(screen.getByTestId("reuse-credentials-button")).toBeInTheDocument(); }); + it("names the attached credential by its display name in the re-use dialog", async () => { + mockCredentialListCall.mockResolvedValue({ + credentials: [ + { + credential_name: "selected-credential", + display_name: "Selected Label", + credential_values: {}, + credential_info: {}, + }, + ], + } as never); + const user = userEvent.setup(); + render(, { wrapper }); + + await user.click(await screen.findByTestId("reuse-credentials-button")); + + const dialog = await screen.findByRole("dialog", { name: "Using Existing Credential" }); + await vi.waitFor(() => expect(dialog).toHaveTextContent("Selected Label")); + expect(dialog).not.toHaveTextContent("selected-credential"); + }); + it.each([["auto_router/adaptive_router"], ["auto_router/quality_router"]])( "offers no Test Connection for %s, whose targets it cannot build", async (model) => { @@ -1577,13 +1598,13 @@ describe("ModelInfoView", () => { const openCredentialSelect = async (user: ReturnType, triggerText?: string) => { const trigger = screen .getAllByRole("combobox") - .filter((element) => element.getAttribute("data-slot") === "select-trigger") - .find((element) => triggerText === undefined || element.textContent?.includes(triggerText)); + .filter((element) => element.getAttribute("placeholder") === "Select or search for existing credentials") + .find((element) => triggerText === undefined || (element as HTMLInputElement).value.includes(triggerText)); if (trigger === undefined) { throw new Error(`Could not find credential selector${triggerText ? ` with ${triggerText}` : ""}`); } await user.click(trigger); - await screen.findByRole("combobox", { expanded: true }); + await screen.findByRole("option", { name: "None" }); }; const save = async (user: ReturnType) => { @@ -1837,8 +1858,8 @@ describe("ModelInfoView", () => { const user = userEvent.setup(); await enterEditMode(user); - await openSelect(user, "selected-credential"); - await user.click(await screen.findByText("other-credential")); + await openCredentialSelect(user, "selected-credential"); + await user.click(await screen.findByRole("option", { name: "other-credential" })); const payload = await save(user); @@ -1902,11 +1923,10 @@ describe("ModelInfoView", () => { await user.click(screen.getByRole("button", { name: /cancel/i })); await user.click(await screen.findByRole("button", { name: /edit settings/i })); - const credentialTrigger: HTMLElement = screen + const credentialTrigger = screen .getAllByRole("combobox") - .filter((element) => element.getAttribute("data-slot") === "select-trigger") - .at(0) as HTMLElement; - expect(credentialTrigger).toHaveTextContent("selected-credential"); + .find((element) => element.getAttribute("placeholder") === "Select or search for existing credentials"); + expect(credentialTrigger).toHaveValue("selected-credential"); }); it("shows Manual in read mode after saving None", async () => { diff --git a/ui/litellm-dashboard/src/components/model_info_view.tsx b/ui/litellm-dashboard/src/components/model_info_view.tsx index 4fe49a28936..26c6308e3ac 100644 --- a/ui/litellm-dashboard/src/components/model_info_view.tsx +++ b/ui/litellm-dashboard/src/components/model_info_view.tsx @@ -28,6 +28,7 @@ import { useTeams } from "@/app/(dashboard)/hooks/teams/useTeams"; import DeleteResourceModal from "./common_components/DeleteResourceModal"; import EditAutoRouterModal from "./edit_auto_router/edit_auto_router_modal"; import ReuseCredentialsModal from "./model_add/reuse_credentials"; +import { credentialLabelsByName } from "./shared/credentialOptions"; import { toast } from "@/lib/toast"; import { CredentialItem, @@ -808,7 +809,10 @@ export default function ModelInfoView({ Using Existing Credential -

{modelData.litellm_params.litellm_credential_name}

+

+ {credentialLabelsByName(credentialsList).get(modelData.litellm_params.litellm_credential_name ?? "") ?? + modelData.litellm_params.litellm_credential_name} +