diff --git a/enterprise/storage/saas_settings_store.py b/enterprise/storage/saas_settings_store.py index b2fa3cff26..d2fe4104e0 100644 --- a/enterprise/storage/saas_settings_store.py +++ b/enterprise/storage/saas_settings_store.py @@ -11,9 +11,10 @@ from pydantic import SecretStr from server.auth.token_manager import TokenManager from server.constants import LITE_LLM_API_URL from server.logger import logger -from sqlalchemy import select +from sqlalchemy import select, update from sqlalchemy.orm import joinedload from storage.database import a_session_maker +from storage.encrypt_utils import encrypt_value from storage.lite_llm_manager import LiteLlmManager, get_openhands_cloud_key_alias from storage.org import Org from storage.org_member import OrgMember @@ -186,6 +187,30 @@ class SaasSettingsStore(SettingsStore): if hasattr(model, key): setattr(model, key, value) + # Propagate LLM settings to all org members + # This ensures all members see the same LLM configuration when an admin saves + # Note: Concurrent saves by multiple admins will result in last-write-wins. + # Consider adding optimistic locking if this becomes a problem. + member_update_values: dict = {} + if item.llm_model is not None: + member_update_values['llm_model'] = item.llm_model + if item.llm_base_url is not None: + member_update_values['llm_base_url'] = item.llm_base_url + if item.max_iterations is not None: + member_update_values['max_iterations'] = item.max_iterations + if item.llm_api_key is not None: + member_update_values['_llm_api_key'] = encrypt_value( + item.llm_api_key.get_secret_value() + ) + + if member_update_values: + stmt = ( + update(OrgMember) + .where(OrgMember.org_id == org_id) + .values(**member_update_values) + ) + await session.execute(stmt) + await session.commit() @classmethod diff --git a/enterprise/tests/unit/test_saas_settings_store.py b/enterprise/tests/unit/test_saas_settings_store.py index 91a55e858a..b0b634b880 100644 --- a/enterprise/tests/unit/test_saas_settings_store.py +++ b/enterprise/tests/unit/test_saas_settings_store.py @@ -1,3 +1,4 @@ +import uuid from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -233,3 +234,165 @@ async def test_ensure_api_key_generates_new_key_when_verification_fails( assert item.llm_api_key is not None assert item.llm_api_key.get_secret_value() == new_key + + +@pytest.fixture +def org_with_multiple_members_fixture(session_maker): + """Set up an organization with multiple members for testing LLM settings propagation. + + Uses sync session to avoid UUID conversion issues with async SQLite. + """ + from storage.encrypt_utils import decrypt_value + from storage.org import Org + from storage.org_member import OrgMember + from storage.role import Role + from storage.user import User + + # Use realistic UUIDs that work well with SQLite + org_id = uuid.UUID('5594c7b6-f959-4b81-92e9-b09c206f5081') + admin_user_id = uuid.UUID('5594c7b6-f959-4b81-92e9-b09c206f5082') + member1_user_id = uuid.UUID('5594c7b6-f959-4b81-92e9-b09c206f5083') + member2_user_id = uuid.UUID('5594c7b6-f959-4b81-92e9-b09c206f5084') + + with session_maker() as session: + # Create role + role = Role(id=10, name='member', rank=3) + session.add(role) + + # Create org + org = Org( + id=org_id, + name='test-org', + org_version=1, + enable_default_condenser=True, + enable_proactive_conversation_starters=True, + ) + session.add(org) + + # Create users + admin_user = User( + id=admin_user_id, current_org_id=org_id, user_consents_to_analytics=True + ) + session.add(admin_user) + + member1_user = User( + id=member1_user_id, current_org_id=org_id, user_consents_to_analytics=True + ) + session.add(member1_user) + + member2_user = User( + id=member2_user_id, current_org_id=org_id, user_consents_to_analytics=True + ) + session.add(member2_user) + + # Create org members with DIFFERENT initial LLM settings + admin_member = OrgMember( + org_id=org_id, + user_id=admin_user_id, + role_id=10, + llm_api_key='admin-initial-key', + llm_model='old-model-v1', + llm_base_url='http://old-url-1.com', + max_iterations=10, + status='active', + ) + session.add(admin_member) + + member1 = OrgMember( + org_id=org_id, + user_id=member1_user_id, + role_id=10, + llm_api_key='member1-initial-key', + llm_model='old-model-v2', + llm_base_url='http://old-url-2.com', + max_iterations=20, + status='active', + ) + session.add(member1) + + member2 = OrgMember( + org_id=org_id, + user_id=member2_user_id, + role_id=10, + llm_api_key='member2-initial-key', + llm_model='old-model-v3', + llm_base_url='http://old-url-3.com', + max_iterations=30, + status='active', + ) + session.add(member2) + + session.commit() + + return { + 'org_id': org_id, + 'admin_user_id': admin_user_id, + 'member1_user_id': member1_user_id, + 'member2_user_id': member2_user_id, + 'decrypt_value': decrypt_value, + } + + +@pytest.mark.asyncio +async def test_store_propagates_llm_settings_to_all_org_members( + session_maker, async_session_maker, mock_config, org_with_multiple_members_fixture +): + """When admin saves LLM settings, all org members should receive the updated settings. + + This test verifies using a real database that: + 1. The bulk UPDATE targets the correct organization (WHERE clause is correct) + 2. All LLM fields are correctly set (llm_model, llm_base_url, max_iterations, llm_api_key) + 3. The llm_api_key is properly encrypted + 4. All members in the org receive the same updated values + """ + from sqlalchemy import select + from storage.org_member import OrgMember + + # Arrange + fixture = org_with_multiple_members_fixture + org_id = fixture['org_id'] + admin_user_id = str(fixture['admin_user_id']) + decrypt_value = fixture['decrypt_value'] + + store = SaasSettingsStore(admin_user_id, mock_config) + + new_settings = DataSettings( + llm_model='new-shared-model/gpt-4', + llm_base_url='http://new-shared-url.com', + max_iterations=100, + llm_api_key=SecretStr('new-shared-api-key'), + ) + + # Act - call store() with async session + with patch('storage.saas_settings_store.a_session_maker', async_session_maker): + await store.store(new_settings) + + # Assert - verify ALL org members have the updated LLM settings using sync session + with session_maker() as session: + result = session.execute(select(OrgMember).where(OrgMember.org_id == org_id)) + members = result.scalars().all() + + # Verify we have all 3 members + assert len(members) == 3, f'Expected 3 org members, got {len(members)}' + + for member in members: + # Verify LLM model is updated + assert ( + member.llm_model == 'new-shared-model/gpt-4' + ), f'Expected llm_model to be updated for member {member.user_id}' + + # Verify LLM base URL is updated + assert ( + member.llm_base_url == 'http://new-shared-url.com' + ), f'Expected llm_base_url to be updated for member {member.user_id}' + + # Verify max_iterations is updated + assert ( + member.max_iterations == 100 + ), f'Expected max_iterations to be 100 for member {member.user_id}' + + # Verify the API key is encrypted and decrypts to the correct value + decrypted_key = decrypt_value(member._llm_api_key) + assert ( + decrypted_key == 'new-shared-api-key' + ), f'Expected llm_api_key to decrypt to new-shared-api-key for member {member.user_id}'