mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(proxy): read both memory and the row when authorizing a credential write
The gate resolved the existing credential through hydrate_named_credential, which answers from the in-memory list and only reads the database when memory has no entry. That order is right when serving a request and wrong when deciding authorization: a pod whose copy predates an admin adding federation fields sees none of them and lets a non-admin through. In a multi-pod deployment the window is a config reload, and with store_model_in_db off it never closes Authorization now goes through its own helper that reads the in-memory entry and the row and takes the union, so the gate refuses whenever either side says the credential is server-owned. Resolution keeps memory-first, which is still correct for serving
This commit is contained in:
parent
0a69790bb9
commit
79b5738dab
3 changed files with 50 additions and 7 deletions
|
|
@ -12,6 +12,7 @@ import litellm
|
|||
from litellm.proxy.common_utils.encrypt_decrypt_utils import decrypt_value_helper
|
||||
from litellm.proxy.utils import PrismaClient
|
||||
from litellm.repositories.credentials_repository import CredentialsRepository
|
||||
from litellm.types.router import anthropic_wif_fields_named
|
||||
from litellm.types.utils import CredentialItem
|
||||
|
||||
|
||||
|
|
@ -38,3 +39,27 @@ async def hydrate_named_credential(
|
|||
credential_values=decrypted_values,
|
||||
credential_info=db_credential.credential_info,
|
||||
)
|
||||
|
||||
|
||||
async def named_credential_wif_fields(
|
||||
credential_name: str,
|
||||
prisma_client: PrismaClient | None,
|
||||
) -> tuple[str, ...]:
|
||||
"""Federation field names a write to ``credential_name`` would touch, from memory AND the row.
|
||||
|
||||
Resolution reads memory first and stops there, which is right when serving a request. An
|
||||
authorization decision cannot: a pod whose in-memory copy predates an admin adding federation
|
||||
fields would see none and allow the write. This reads both and returns the union, so the gate
|
||||
refuses whenever either side says the credential is server-owned.
|
||||
"""
|
||||
in_memory: Final = tuple(
|
||||
name
|
||||
for credential in litellm.credential_list
|
||||
if credential.credential_name == credential_name
|
||||
for name in anthropic_wif_fields_named(credential.credential_values)
|
||||
)
|
||||
if prisma_client is None:
|
||||
return in_memory
|
||||
db_credential: Final = await CredentialsRepository(prisma_client).find_by_name(credential_name)
|
||||
stored: Final = () if db_credential is None else anthropic_wif_fields_named(db_credential.credential_values)
|
||||
return tuple(dict.fromkeys(in_memory + stored))
|
||||
|
|
|
|||
|
|
@ -22,7 +22,10 @@ from litellm.llms.base_llm.auth.identity_source import (
|
|||
from litellm.llms.base_llm.auth.internal_issuer import internal_issuer_jwks_document
|
||||
from litellm.proxy._types import CommonProxyErrors, LitellmUserRoles, UserAPIKeyAuth
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.common_utils.credential_hydration import hydrate_named_credential
|
||||
from litellm.proxy.common_utils.credential_hydration import (
|
||||
hydrate_named_credential,
|
||||
named_credential_wif_fields,
|
||||
)
|
||||
from litellm.proxy.common_utils.encrypt_decrypt_utils import encrypt_value_helper
|
||||
from litellm.proxy.utils import handle_exception_on_proxy, jsonify_object
|
||||
from litellm.repositories.credentials_repository import CredentialsRepository
|
||||
|
|
@ -172,9 +175,9 @@ async def create_credential(
|
|||
detail="Credential values are required. Unable to infer credential values from model ID.",
|
||||
)
|
||||
_reject_non_admin_wif_fields(anthropic_wif_fields_named(credential.credential_values), user_api_key_dict)
|
||||
existing_credential: Final = await hydrate_named_credential(credential.credential_name, prisma_client)
|
||||
if existing_credential is not None:
|
||||
_reject_non_admin_wif_fields(_stored_wif_fields(existing_credential), user_api_key_dict)
|
||||
_reject_non_admin_wif_fields(
|
||||
await named_credential_wif_fields(credential.credential_name, prisma_client), user_api_key_dict
|
||||
)
|
||||
processed_credential: Final = CredentialItem(
|
||||
credential_name=credential.credential_name,
|
||||
credential_values=credential.credential_values,
|
||||
|
|
@ -392,9 +395,9 @@ async def delete_credential(
|
|||
status_code=500,
|
||||
detail={"error": CommonProxyErrors.db_not_connected_error.value},
|
||||
)
|
||||
existing_credential: Final = await hydrate_named_credential(credential_name, prisma_client)
|
||||
if existing_credential is not None:
|
||||
_reject_non_admin_wif_fields(_stored_wif_fields(existing_credential), user_api_key_dict)
|
||||
_reject_non_admin_wif_fields(
|
||||
await named_credential_wif_fields(credential_name, prisma_client), user_api_key_dict
|
||||
)
|
||||
await CredentialsRepository(prisma_client).delete_by_name(credential_name)
|
||||
|
||||
## DELETE FROM LITELLM ##
|
||||
|
|
|
|||
|
|
@ -704,6 +704,21 @@ class TestNonAdminCannotTouchAStoredWifCredential:
|
|||
assert "anthropic_keycloak_token_url" in response.text
|
||||
repository.delete_by_name.assert_not_awaited()
|
||||
|
||||
def test_a_stale_in_memory_copy_does_not_authorize_deleting_a_stored_wif_credential(
|
||||
self, restore_credential_list, monkeypatch
|
||||
):
|
||||
"""Resolution reads memory first and stops, which is right when serving a request. A pod
|
||||
whose in-memory copy predates an admin adding the federation fields must not read that
|
||||
stale object and authorize the delete: the gate takes the union of memory and the row."""
|
||||
monkeypatch.setattr(litellm, "credential_list", [_plain_credential("federated-cred")])
|
||||
|
||||
with _repository_holding(_wif_credential("federated-cred")) as repository:
|
||||
response = _delete_credential("federated-cred", auth=_as_non_admin)
|
||||
|
||||
assert response.status_code == 403, response.text
|
||||
assert "anthropic_keycloak_token_url" in response.text
|
||||
repository.delete_by_name.assert_not_awaited()
|
||||
|
||||
def test_proxy_admin_can_delete_a_stored_wif_credential(self, restore_credential_list):
|
||||
with _repository_holding(_wif_credential("federated-cred")) as repository:
|
||||
response = _delete_credential("federated-cred", auth=_as_admin)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue