mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-04-29 03:00:45 -04:00
fix: BYOR to OpenHands provider switch auth error (#12725)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -230,19 +230,30 @@ class SaasSettingsStore(SettingsStore):
|
||||
async def _ensure_openhands_api_key(self, item: Settings, org_id: str) -> None:
|
||||
"""Generate and set the OpenHands API key for the given settings.
|
||||
|
||||
First checks if an existing key exists for the user and reuses it
|
||||
if found. Otherwise, generates a new key.
|
||||
First checks if an existing key exists for the user and verifies it
|
||||
is valid in LiteLLM. If valid, reuses it. Otherwise, generates a new key.
|
||||
"""
|
||||
# Check if user already has keys in LiteLLM
|
||||
existing_keys = await LiteLlmManager.get_user_keys(self.user_id)
|
||||
if existing_keys:
|
||||
logger.info(
|
||||
'saas_settings_store:store:user_already_has_keys',
|
||||
extra={'user_id': self.user_id, 'key_count': len(existing_keys)},
|
||||
)
|
||||
return
|
||||
# Verify the first key is actually valid in LiteLLM before reusing
|
||||
# This handles cases where keys exist in our DB but were orphaned in LiteLLM
|
||||
key_to_reuse = existing_keys[0]
|
||||
if await LiteLlmManager.verify_key(key_to_reuse, self.user_id):
|
||||
item.llm_api_key = SecretStr(key_to_reuse)
|
||||
logger.info(
|
||||
'saas_settings_store:store:reusing_verified_key',
|
||||
extra={'user_id': self.user_id, 'key_count': len(existing_keys)},
|
||||
)
|
||||
return
|
||||
else:
|
||||
logger.warning(
|
||||
'saas_settings_store:store:existing_key_invalid',
|
||||
extra={'user_id': self.user_id, 'key_count': len(existing_keys)},
|
||||
)
|
||||
# Fall through to generate a new key
|
||||
|
||||
# Generate new key only if none exists
|
||||
# Generate new key if none exists or existing keys are invalid
|
||||
generated_key = await LiteLlmManager.generate_key(
|
||||
self.user_id,
|
||||
org_id,
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
from unittest.mock import MagicMock, patch
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from pydantic import SecretStr
|
||||
|
||||
from openhands.core.config.openhands_config import OpenHandsConfig
|
||||
from openhands.server.settings import Settings
|
||||
from openhands.storage.data_models.settings import Settings as DataSettings
|
||||
|
||||
# Mock the database module before importing
|
||||
with patch('storage.database.engine'), patch('storage.database.a_engine'):
|
||||
@@ -176,3 +177,61 @@ async def test_encryption(settings_store):
|
||||
# But we should be able to decrypt it when loading
|
||||
loaded_settings = await settings_store.load()
|
||||
assert loaded_settings.llm_api_key.get_secret_value() == 'secret_key'
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_openhands_api_key_sets_key_when_reusing_verified_key(mock_config):
|
||||
"""The old code returned early without setting item.llm_api_key."""
|
||||
store = SaasSettingsStore('test-user-id-123', MagicMock(), mock_config)
|
||||
existing_key = 'sk-existing-key'
|
||||
item = DataSettings(llm_model='openhands/gpt-4')
|
||||
|
||||
with (
|
||||
patch(
|
||||
'storage.saas_settings_store.LiteLlmManager.get_user_keys',
|
||||
new_callable=AsyncMock,
|
||||
return_value=[existing_key],
|
||||
),
|
||||
patch(
|
||||
'storage.saas_settings_store.LiteLlmManager.verify_key',
|
||||
new_callable=AsyncMock,
|
||||
return_value=True,
|
||||
),
|
||||
):
|
||||
await store._ensure_openhands_api_key(item, 'org-123')
|
||||
|
||||
# This assertion failed with the old code
|
||||
assert item.llm_api_key is not None
|
||||
assert item.llm_api_key.get_secret_value() == existing_key
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_openhands_api_key_generates_new_key_when_verification_fails(
|
||||
mock_config,
|
||||
):
|
||||
"""Handles orphaned keys that exist in our DB but not in LiteLLM."""
|
||||
store = SaasSettingsStore('test-user-id-123', MagicMock(), mock_config)
|
||||
new_key = 'sk-new-key'
|
||||
item = DataSettings(llm_model='openhands/gpt-4')
|
||||
|
||||
with (
|
||||
patch(
|
||||
'storage.saas_settings_store.LiteLlmManager.get_user_keys',
|
||||
new_callable=AsyncMock,
|
||||
return_value=['sk-orphaned-key'],
|
||||
),
|
||||
patch(
|
||||
'storage.saas_settings_store.LiteLlmManager.verify_key',
|
||||
new_callable=AsyncMock,
|
||||
return_value=False,
|
||||
),
|
||||
patch(
|
||||
'storage.saas_settings_store.LiteLlmManager.generate_key',
|
||||
new_callable=AsyncMock,
|
||||
return_value=new_key,
|
||||
),
|
||||
):
|
||||
await store._ensure_openhands_api_key(item, 'org-123')
|
||||
|
||||
assert item.llm_api_key is not None
|
||||
assert item.llm_api_key.get_secret_value() == new_key
|
||||
|
||||
Reference in New Issue
Block a user