mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 16:38:34 +08:00
fix: reconcile and label per-(user,org) LiteLLM managed keys (#14803)
This commit is contained in:
@@ -75,6 +75,18 @@ def get_byor_key_alias(keycloak_user_id: str, org_id: str) -> str:
|
||||
return f'BYOR Key - user {keycloak_user_id}, org {org_id}'
|
||||
|
||||
|
||||
def get_org_team_alias(org_id: str, org_name: str | None, user_id: str | None) -> str:
|
||||
"""Human-readable LiteLLM team_alias for an org's team.
|
||||
|
||||
Personal orgs (org_id == user_id) get "Personal Workspace"; team orgs use
|
||||
their display name. Falls back to the id when no name is available, never
|
||||
the bare user uid (which made teams indistinguishable in the dashboard).
|
||||
"""
|
||||
if str(org_id) == str(user_id):
|
||||
return 'Personal Workspace'
|
||||
return org_name or f'Organization {org_id}'
|
||||
|
||||
|
||||
class LiteLlmManager:
|
||||
"""Manage LiteLLM interactions."""
|
||||
|
||||
@@ -180,8 +192,11 @@ class LiteLlmManager:
|
||||
extra={'org_id': org_id, 'user_id': keycloak_user_id},
|
||||
)
|
||||
|
||||
team_alias = await LiteLlmManager._team_alias_for_org(
|
||||
org_id, keycloak_user_id
|
||||
)
|
||||
await LiteLlmManager._create_team(
|
||||
client, keycloak_user_id, org_id, team_budget
|
||||
client, team_alias, org_id, team_budget
|
||||
)
|
||||
|
||||
if create_user:
|
||||
@@ -349,9 +364,10 @@ class LiteLlmManager:
|
||||
'LiteLlmManager:migrate_lite_llm_entries:create_team',
|
||||
extra={'org_id': org_id, 'user_id': keycloak_user_id},
|
||||
)
|
||||
await LiteLlmManager._create_team(
|
||||
client, keycloak_user_id, org_id, credits
|
||||
team_alias = await LiteLlmManager._team_alias_for_org(
|
||||
org_id, keycloak_user_id
|
||||
)
|
||||
await LiteLlmManager._create_team(client, team_alias, org_id, credits)
|
||||
|
||||
logger.debug(
|
||||
'LiteLlmManager:migrate_lite_llm_entries:update_user',
|
||||
@@ -605,6 +621,29 @@ class LiteLlmManager:
|
||||
client, user_id, team_id, max_budget
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
async def _team_alias_for_org(org_id: str, keycloak_user_id: str) -> str:
|
||||
"""Resolve the dashboard-friendly team_alias for an org (its display
|
||||
name, or 'Personal Workspace' for the user's personal org). The org
|
||||
name is looked up lazily; lookup failures fall back to a stable label."""
|
||||
if str(org_id) == str(keycloak_user_id):
|
||||
return get_org_team_alias(org_id, None, keycloak_user_id)
|
||||
# Lazy import: org_store imports this module at load time.
|
||||
from uuid import UUID
|
||||
|
||||
from storage.org_store import OrgStore
|
||||
|
||||
org_name = None
|
||||
try:
|
||||
org = await OrgStore.get_org_by_id(UUID(org_id))
|
||||
org_name = org.name if org else None
|
||||
except Exception:
|
||||
logger.warning(
|
||||
'Failed to resolve org name for LiteLLM team_alias',
|
||||
extra={'org_id': org_id},
|
||||
)
|
||||
return get_org_team_alias(org_id, org_name, keycloak_user_id)
|
||||
|
||||
@staticmethod
|
||||
async def _create_team(
|
||||
client: httpx.AsyncClient,
|
||||
|
||||
@@ -20,7 +20,11 @@ from sqlalchemy import delete, func, select, text
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from sqlalchemy.orm import joinedload
|
||||
from storage.database import a_session_maker
|
||||
from storage.lite_llm_manager import LiteLlmManager, get_openhands_cloud_key_alias
|
||||
from storage.lite_llm_manager import (
|
||||
LiteLlmManager,
|
||||
get_openhands_cloud_key_alias,
|
||||
get_org_team_alias,
|
||||
)
|
||||
from storage.org import Org
|
||||
from storage.org_git_claim import OrgGitClaim
|
||||
from storage.org_invitation import OrgInvitation
|
||||
@@ -400,6 +404,8 @@ class OrgStore:
|
||||
if not org:
|
||||
return None
|
||||
|
||||
old_name = org.name
|
||||
|
||||
if 'id' in org_kwargs:
|
||||
org_kwargs.pop('id')
|
||||
|
||||
@@ -466,6 +472,22 @@ class OrgStore:
|
||||
|
||||
await session.commit()
|
||||
await session.refresh(org)
|
||||
|
||||
# Keep the LiteLLM team_alias in sync with the org's display name so
|
||||
# the proxy dashboard stays readable after a rename. Best-effort —
|
||||
# never fail an org update because the proxy is briefly unreachable.
|
||||
if org.name != old_name:
|
||||
try:
|
||||
await LiteLlmManager.update_team(
|
||||
str(org.id),
|
||||
get_org_team_alias(str(org.id), org.name, user_id),
|
||||
None,
|
||||
)
|
||||
except Exception:
|
||||
logger.warning(
|
||||
'Failed to propagate org rename to LiteLLM team_alias',
|
||||
extra={'org_id': str(org.id)},
|
||||
)
|
||||
return org
|
||||
|
||||
@staticmethod
|
||||
@@ -843,18 +865,9 @@ class OrgStore:
|
||||
):
|
||||
return existing_key_raw
|
||||
|
||||
if openhands_type:
|
||||
logger.info(
|
||||
'Generated managed LLM key for acting user on org-defaults save',
|
||||
extra={'user_id': user_id, 'org_id': str(updated_org.id)},
|
||||
)
|
||||
return await LiteLlmManager.generate_key(
|
||||
user_id,
|
||||
str(updated_org.id),
|
||||
None,
|
||||
{'type': 'openhands'},
|
||||
)
|
||||
|
||||
# One managed key per (user, org) under the same deterministic alias,
|
||||
# deleting any prior key first — symmetric across openhands/* and BYOR
|
||||
# defaults so switching between them never orphans a key.
|
||||
key_alias = get_openhands_cloud_key_alias(user_id, str(updated_org.id))
|
||||
await LiteLlmManager.delete_key_by_alias(key_alias=key_alias)
|
||||
logger.info(
|
||||
@@ -865,7 +878,7 @@ class OrgStore:
|
||||
user_id,
|
||||
str(updated_org.id),
|
||||
key_alias,
|
||||
None,
|
||||
{'type': 'openhands'} if openhands_type else None,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
|
||||
@@ -512,23 +512,17 @@ class SaasSettingsStore(SettingsStore):
|
||||
org_id,
|
||||
openhands_type=openhands_type,
|
||||
):
|
||||
if openhands_type:
|
||||
generated_key = await LiteLlmManager.generate_key(
|
||||
self.user_id,
|
||||
org_id,
|
||||
None,
|
||||
{'type': 'openhands'},
|
||||
)
|
||||
else:
|
||||
# Must delete any existing key with the same alias first
|
||||
key_alias = get_openhands_cloud_key_alias(self.user_id, org_id)
|
||||
await LiteLlmManager.delete_key_by_alias(key_alias=key_alias)
|
||||
generated_key = await LiteLlmManager.generate_key(
|
||||
self.user_id,
|
||||
org_id,
|
||||
key_alias,
|
||||
None,
|
||||
)
|
||||
# Both branches mint one managed key per (user, org) under the same
|
||||
# deterministic alias, deleting any prior key first — so switching
|
||||
# the default to/from an openhands/* model never orphans a key.
|
||||
key_alias = get_openhands_cloud_key_alias(self.user_id, org_id)
|
||||
await LiteLlmManager.delete_key_by_alias(key_alias=key_alias)
|
||||
generated_key = await LiteLlmManager.generate_key(
|
||||
self.user_id,
|
||||
org_id,
|
||||
key_alias,
|
||||
{'type': 'openhands'} if openhands_type else None,
|
||||
)
|
||||
|
||||
item.agent_settings.llm.api_key = SecretStr(generated_key)
|
||||
logger.info(
|
||||
|
||||
@@ -17,6 +17,7 @@ from storage.lite_llm_manager import (
|
||||
LiteLlmManager,
|
||||
get_byor_key_alias,
|
||||
get_openhands_cloud_key_alias,
|
||||
get_org_team_alias,
|
||||
)
|
||||
from storage.user_settings import UserSettings
|
||||
|
||||
@@ -37,6 +38,58 @@ def _secret_value(settings: Settings, key: str):
|
||||
return secret.get_secret_value() if secret else None
|
||||
|
||||
|
||||
class TestOrgTeamAlias:
|
||||
"""Human-readable LiteLLM team_alias derivation."""
|
||||
|
||||
def test_personal_org_labeled_personal_workspace(self):
|
||||
# Personal org: org_id == user_id.
|
||||
assert get_org_team_alias('user-1', 'ignored', 'user-1') == 'Personal Workspace'
|
||||
|
||||
def test_team_org_uses_display_name(self):
|
||||
assert get_org_team_alias('org-2', 'Acme Inc', 'user-1') == 'Acme Inc'
|
||||
|
||||
def test_team_org_without_name_falls_back_to_id_not_uid(self):
|
||||
# Never the bare user uid (the old behavior that hid teams).
|
||||
assert get_org_team_alias('org-2', None, 'user-1') == 'Organization org-2'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_team_alias_for_org_personal_skips_lookup(self):
|
||||
# Personal org short-circuits without touching OrgStore.
|
||||
with patch(
|
||||
'storage.org_store.OrgStore.get_org_by_id', new_callable=AsyncMock
|
||||
) as mock_get:
|
||||
alias = await LiteLlmManager._team_alias_for_org('user-1', 'user-1')
|
||||
assert alias == 'Personal Workspace'
|
||||
mock_get.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_team_alias_for_org_team_resolves_name(self):
|
||||
org = MagicMock()
|
||||
org.name = 'Acme Inc'
|
||||
with patch(
|
||||
'storage.org_store.OrgStore.get_org_by_id',
|
||||
new_callable=AsyncMock,
|
||||
return_value=org,
|
||||
):
|
||||
alias = await LiteLlmManager._team_alias_for_org(
|
||||
'11111111-1111-1111-1111-111111111111', 'user-1'
|
||||
)
|
||||
assert alias == 'Acme Inc'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_team_alias_for_org_lookup_failure_falls_back(self):
|
||||
# A lookup failure must not crash team creation.
|
||||
with patch(
|
||||
'storage.org_store.OrgStore.get_org_by_id',
|
||||
new_callable=AsyncMock,
|
||||
side_effect=RuntimeError('db down'),
|
||||
):
|
||||
alias = await LiteLlmManager._team_alias_for_org(
|
||||
'11111111-1111-1111-1111-111111111111', 'user-1'
|
||||
)
|
||||
assert alias == 'Organization 11111111-1111-1111-1111-111111111111'
|
||||
|
||||
|
||||
class TestDefaultInitialBudget:
|
||||
"""Test cases for DEFAULT_INITIAL_BUDGET configuration."""
|
||||
|
||||
|
||||
@@ -328,10 +328,15 @@ async def test_ensure_api_key_keeps_valid_key():
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_api_key_generates_new_key_when_verification_fails():
|
||||
"""When verification fails, a new key should be generated."""
|
||||
"""When verification fails, a new managed key is minted under the shared
|
||||
alias after deleting any prior key — symmetric across model types so
|
||||
switching to/from an openhands/* model never orphans a key."""
|
||||
from storage.lite_llm_manager import get_openhands_cloud_key_alias
|
||||
|
||||
store = SaasSettingsStore('test-user-id-123')
|
||||
new_key = 'sk-new-key'
|
||||
item = _make_settings(model='openhands/gpt-4', api_key='sk-invalid-key')
|
||||
expected_alias = get_openhands_cloud_key_alias('test-user-id-123', 'org-123')
|
||||
|
||||
with (
|
||||
patch(
|
||||
@@ -339,16 +344,25 @@ async def test_ensure_api_key_generates_new_key_when_verification_fails():
|
||||
new_callable=AsyncMock,
|
||||
return_value=False,
|
||||
),
|
||||
patch(
|
||||
'storage.saas_settings_store.LiteLlmManager.delete_key_by_alias',
|
||||
new_callable=AsyncMock,
|
||||
) as mock_delete,
|
||||
patch(
|
||||
'storage.saas_settings_store.LiteLlmManager.generate_key',
|
||||
new_callable=AsyncMock,
|
||||
return_value=new_key,
|
||||
),
|
||||
) as mock_generate,
|
||||
):
|
||||
await store._ensure_api_key(item, 'org-123', openhands_type=True)
|
||||
|
||||
assert _secret_value(item, 'llm.api_key') is not None
|
||||
assert _secret_value(item, 'llm.api_key') == new_key
|
||||
# The openhands branch now deletes the prior key under the shared alias
|
||||
# before minting (previously it skipped the delete and orphaned keys).
|
||||
mock_delete.assert_awaited_once_with(key_alias=expected_alias)
|
||||
mock_generate.assert_awaited_once_with(
|
||||
'test-user-id-123', 'org-123', expected_alias, {'type': 'openhands'}
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
|
||||
Reference in New Issue
Block a user