mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-04-29 03:00:45 -04:00
fix: add flat SDK compat fields to /api/v1/users/me response (#13957)
This commit is contained in:
@@ -5,6 +5,7 @@ user endpoints with organization context (org_id, org_name, role, permissions).
|
||||
"""
|
||||
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from fastapi import APIRouter, FastAPI, Header, HTTPException, Query, status
|
||||
from fastapi.responses import JSONResponse
|
||||
@@ -25,6 +26,29 @@ saas_users_v1_router = APIRouter(
|
||||
user_dependency = depends_user_context()
|
||||
|
||||
|
||||
def _inject_sdk_compat_fields(
|
||||
content: dict[str, Any], *, include_api_key: bool
|
||||
) -> None:
|
||||
"""Inject flat top-level convenience fields for the SDK.
|
||||
|
||||
The SDK's ``get_llm()`` and ``get_mcp_config()`` read ``llm_model``,
|
||||
``llm_api_key``, ``llm_base_url``, and ``mcp_config`` from the top
|
||||
level of the ``/api/v1/users/me`` response. These values live inside
|
||||
the nested ``agent_settings`` structure, so we mirror them at the top
|
||||
level for backward compatibility.
|
||||
|
||||
The canonical representation is ``agent_settings``; these flat fields
|
||||
exist solely for SDK backward compatibility.
|
||||
"""
|
||||
agent_settings = content.get('agent_settings') or {}
|
||||
llm = agent_settings.get('llm') or {}
|
||||
content['llm_model'] = llm.get('model')
|
||||
content['llm_base_url'] = llm.get('base_url')
|
||||
if include_api_key:
|
||||
content['llm_api_key'] = llm.get('api_key')
|
||||
content['mcp_config'] = agent_settings.get('mcp_config')
|
||||
|
||||
|
||||
@saas_users_v1_router.get('/me')
|
||||
async def get_current_user_saas(
|
||||
user_context: UserContext = user_dependency,
|
||||
@@ -63,10 +87,13 @@ async def get_current_user_saas(
|
||||
|
||||
if expose_secrets:
|
||||
await validate_session_key_ownership(user_context, x_session_api_key)
|
||||
return JSONResponse( # type: ignore[return-value]
|
||||
content=user_info.model_dump(mode='json', context={'expose_secrets': True})
|
||||
)
|
||||
return user_info
|
||||
content = user_info.model_dump(mode='json', context={'expose_secrets': True})
|
||||
_inject_sdk_compat_fields(content, include_api_key=True)
|
||||
return JSONResponse(content=content) # type: ignore[return-value]
|
||||
|
||||
content = user_info.model_dump(mode='json')
|
||||
_inject_sdk_compat_fields(content, include_api_key=False)
|
||||
return JSONResponse(content=content) # type: ignore[return-value]
|
||||
|
||||
|
||||
async def _get_org_info_from_context(user_context: UserContext) -> dict | None:
|
||||
|
||||
@@ -4,8 +4,10 @@ Tests:
|
||||
- SaasUserInfo model with org fields
|
||||
- get_current_user_saas endpoint with org info
|
||||
- _get_org_info_from_context helper function
|
||||
- SDK compatibility fields (llm_model, llm_api_key, llm_base_url, mcp_config)
|
||||
"""
|
||||
|
||||
import json
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
@@ -166,16 +168,16 @@ class TestGetCurrentUserSaasEndpoint:
|
||||
async def test_endpoint_returns_saas_user_info_with_org_fields(
|
||||
self, mock_user_context
|
||||
):
|
||||
"""Endpoint should return SaasUserInfo with org fields."""
|
||||
"""Endpoint should return user info with org fields."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from server.models.user_models import SaasUserInfo
|
||||
from fastapi.responses import JSONResponse
|
||||
from server.routes.users_v1 import get_current_user_saas
|
||||
|
||||
from openhands.app_server.user.user_models import UserInfo
|
||||
|
||||
# Mock base user info
|
||||
base_user_info = UserInfo(id='user-123', llm_model='test-model')
|
||||
base_user_info = UserInfo(id='user-123')
|
||||
mock_user_context.get_user_info = AsyncMock(return_value=base_user_info)
|
||||
|
||||
# Mock _get_org_info_from_context to return org info
|
||||
@@ -194,12 +196,13 @@ class TestGetCurrentUserSaasEndpoint:
|
||||
user_context=mock_user_context, expose_secrets=False
|
||||
)
|
||||
|
||||
assert isinstance(result, SaasUserInfo)
|
||||
assert result.id == 'user-123'
|
||||
assert result.org_id == 'org-456'
|
||||
assert result.org_name == 'Test Organization'
|
||||
assert result.role == 'member'
|
||||
assert result.permissions == ['read', 'write']
|
||||
assert isinstance(result, JSONResponse)
|
||||
data = json.loads(result.body)
|
||||
assert data['id'] == 'user-123'
|
||||
assert data['org_id'] == 'org-456'
|
||||
assert data['org_name'] == 'Test Organization'
|
||||
assert data['role'] == 'member'
|
||||
assert data['permissions'] == ['read', 'write']
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_endpoint_returns_saas_user_info_without_org_fields(
|
||||
@@ -208,13 +211,13 @@ class TestGetCurrentUserSaasEndpoint:
|
||||
"""Endpoint should work when org info is not available."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from server.models.user_models import SaasUserInfo
|
||||
from fastapi.responses import JSONResponse
|
||||
from server.routes.users_v1 import get_current_user_saas
|
||||
|
||||
from openhands.app_server.user.user_models import UserInfo
|
||||
|
||||
# Mock base user info
|
||||
base_user_info = UserInfo(id='user-123', llm_model='test-model')
|
||||
base_user_info = UserInfo(id='user-123')
|
||||
mock_user_context.get_user_info = AsyncMock(return_value=base_user_info)
|
||||
|
||||
# Mock _get_org_info_from_context to return None
|
||||
@@ -226,12 +229,13 @@ class TestGetCurrentUserSaasEndpoint:
|
||||
user_context=mock_user_context, expose_secrets=False
|
||||
)
|
||||
|
||||
assert isinstance(result, SaasUserInfo)
|
||||
assert result.id == 'user-123'
|
||||
assert result.org_id is None
|
||||
assert result.org_name is None
|
||||
assert result.role is None
|
||||
assert result.permissions is None
|
||||
assert isinstance(result, JSONResponse)
|
||||
data = json.loads(result.body)
|
||||
assert data['id'] == 'user-123'
|
||||
assert data.get('org_id') is None
|
||||
assert data.get('org_name') is None
|
||||
assert data.get('role') is None
|
||||
assert data.get('permissions') is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_endpoint_raises_401_when_user_info_is_none(self, mock_user_context):
|
||||
@@ -250,6 +254,147 @@ class TestGetCurrentUserSaasEndpoint:
|
||||
assert exc_info.value.detail == 'Not authenticated'
|
||||
|
||||
|
||||
class TestSdkCompatFields:
|
||||
"""Test suite for flat SDK compatibility fields in the response."""
|
||||
|
||||
@pytest.fixture
|
||||
def mock_user_context(self):
|
||||
"""Create a mock user context."""
|
||||
return AsyncMock()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_expose_response_contains_llm_model_and_base_url(
|
||||
self, mock_user_context
|
||||
):
|
||||
"""Non-expose response should include llm_model and llm_base_url."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from server.routes.users_v1 import get_current_user_saas
|
||||
|
||||
from openhands.app_server.user.user_models import UserInfo
|
||||
from openhands.sdk.llm import LLM
|
||||
from openhands.sdk.settings import AgentSettings
|
||||
|
||||
base_user_info = UserInfo(
|
||||
id='user-123',
|
||||
agent_settings=AgentSettings(
|
||||
llm=LLM(model='test-model', base_url='https://test.com')
|
||||
),
|
||||
)
|
||||
mock_user_context.get_user_info = AsyncMock(return_value=base_user_info)
|
||||
|
||||
with patch(
|
||||
'server.routes.users_v1._get_org_info_from_context',
|
||||
return_value=None,
|
||||
):
|
||||
result = await get_current_user_saas(
|
||||
user_context=mock_user_context, expose_secrets=False
|
||||
)
|
||||
|
||||
data = json.loads(result.body)
|
||||
assert data['llm_model'] == 'test-model'
|
||||
assert data['llm_base_url'] == 'https://test.com'
|
||||
assert 'llm_api_key' not in data
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_expose_secrets_response_contains_llm_api_key(
|
||||
self, mock_user_context
|
||||
):
|
||||
"""Expose-secrets response should include llm_api_key."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from server.routes.users_v1 import get_current_user_saas
|
||||
|
||||
from openhands.app_server.user.user_models import UserInfo
|
||||
from openhands.sdk.llm import LLM
|
||||
from openhands.sdk.settings import AgentSettings
|
||||
|
||||
base_user_info = UserInfo(
|
||||
id='user-123',
|
||||
agent_settings=AgentSettings(
|
||||
llm=LLM(
|
||||
model='test-model',
|
||||
api_key='sk-test-secret',
|
||||
base_url='https://test.com',
|
||||
)
|
||||
),
|
||||
)
|
||||
mock_user_context.get_user_info = AsyncMock(return_value=base_user_info)
|
||||
|
||||
with (
|
||||
patch(
|
||||
'server.routes.users_v1._get_org_info_from_context',
|
||||
return_value=None,
|
||||
),
|
||||
patch(
|
||||
'server.routes.users_v1.validate_session_key_ownership',
|
||||
new_callable=AsyncMock,
|
||||
),
|
||||
):
|
||||
result = await get_current_user_saas(
|
||||
user_context=mock_user_context,
|
||||
expose_secrets=True,
|
||||
x_session_api_key='session-key',
|
||||
)
|
||||
|
||||
data = json.loads(result.body)
|
||||
assert data['llm_model'] == 'test-model'
|
||||
assert data['llm_api_key'] == 'sk-test-secret'
|
||||
assert data['llm_base_url'] == 'https://test.com'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_response_contains_mcp_config_at_top_level(self, mock_user_context):
|
||||
"""Response should include mcp_config at top level."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from server.routes.users_v1 import get_current_user_saas
|
||||
|
||||
from openhands.app_server.user.user_models import UserInfo
|
||||
|
||||
base_user_info = UserInfo(id='user-123')
|
||||
mock_user_context.get_user_info = AsyncMock(return_value=base_user_info)
|
||||
|
||||
with patch(
|
||||
'server.routes.users_v1._get_org_info_from_context',
|
||||
return_value=None,
|
||||
):
|
||||
result = await get_current_user_saas(
|
||||
user_context=mock_user_context, expose_secrets=False
|
||||
)
|
||||
|
||||
data = json.loads(result.body)
|
||||
assert 'mcp_config' in data
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_nested_agent_settings_still_present(self, mock_user_context):
|
||||
"""Flat fields should not remove the nested agent_settings."""
|
||||
from unittest.mock import patch
|
||||
|
||||
from server.routes.users_v1 import get_current_user_saas
|
||||
|
||||
from openhands.app_server.user.user_models import UserInfo
|
||||
from openhands.sdk.llm import LLM
|
||||
from openhands.sdk.settings import AgentSettings
|
||||
|
||||
base_user_info = UserInfo(
|
||||
id='user-123',
|
||||
agent_settings=AgentSettings(llm=LLM(model='test-model')),
|
||||
)
|
||||
mock_user_context.get_user_info = AsyncMock(return_value=base_user_info)
|
||||
|
||||
with patch(
|
||||
'server.routes.users_v1._get_org_info_from_context',
|
||||
return_value=None,
|
||||
):
|
||||
result = await get_current_user_saas(
|
||||
user_context=mock_user_context, expose_secrets=False
|
||||
)
|
||||
|
||||
data = json.loads(result.body)
|
||||
assert 'agent_settings' in data
|
||||
assert data['agent_settings']['llm']['model'] == 'test-model'
|
||||
|
||||
|
||||
class TestOverrideUsersEndpoint:
|
||||
"""Test suite for override_users_me_endpoint function."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user