mirror of
https://github.com/OpenHands/OpenHands.git
synced 2025-12-25 21:36:52 +08:00
refactor: add description field support for secrets
This commit is contained in:
parent
2ce6c9836e
commit
b5e83679af
@ -557,6 +557,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
continue
|
||||
|
||||
secret_name = f'{provider_type.name}_TOKEN'
|
||||
description = f'{provider_type.name} authentication token'
|
||||
|
||||
if self.web_url:
|
||||
# Create an access token for web-based authentication
|
||||
@ -576,12 +577,15 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
||||
secrets[secret_name] = LookupSecret(
|
||||
url=self.web_url + '/api/v1/webhooks/secrets',
|
||||
headers=headers,
|
||||
description=description,
|
||||
)
|
||||
else:
|
||||
# Use static token for environments without web URL access
|
||||
static_token = await self.user_context.get_latest_token(provider_type)
|
||||
if static_token:
|
||||
secrets[secret_name] = StaticSecret(value=static_token)
|
||||
secrets[secret_name] = StaticSecret(
|
||||
value=static_token, description=description
|
||||
)
|
||||
|
||||
return secrets
|
||||
|
||||
|
||||
@ -81,7 +81,12 @@ class AuthUserContext(UserContext):
|
||||
secrets = await self.user_auth.get_secrets()
|
||||
if secrets:
|
||||
for name, custom_secret in secrets.custom_secrets.items():
|
||||
results[name] = StaticSecret(value=custom_secret.secret)
|
||||
results[name] = StaticSecret(
|
||||
value=custom_secret.secret,
|
||||
description=custom_secret.description
|
||||
if custom_secret.description
|
||||
else None,
|
||||
)
|
||||
|
||||
return results
|
||||
|
||||
|
||||
@ -4,6 +4,7 @@ from unittest.mock import AsyncMock, Mock, patch
|
||||
from uuid import UUID, uuid4
|
||||
|
||||
import pytest
|
||||
from pydantic import SecretStr
|
||||
|
||||
from openhands.agent_server.models import SendMessageRequest, StartConversationRequest
|
||||
from openhands.app_server.app_conversation.app_conversation_models import (
|
||||
@ -21,7 +22,7 @@ from openhands.app_server.sandbox.sandbox_models import (
|
||||
)
|
||||
from openhands.app_server.sandbox.sandbox_spec_models import SandboxSpecInfo
|
||||
from openhands.app_server.user.user_context import UserContext
|
||||
from openhands.integrations.provider import ProviderType
|
||||
from openhands.integrations.provider import ProviderToken, ProviderType
|
||||
from openhands.sdk import Agent
|
||||
from openhands.sdk.llm import LLM
|
||||
from openhands.sdk.secret import LookupSecret, StaticSecret
|
||||
@ -104,10 +105,6 @@ class TestLiveStatusAppConversationService:
|
||||
async def test_setup_secrets_for_git_providers_with_web_url(self):
|
||||
"""Test _setup_secrets_for_git_providers with web URL (creates access token)."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
from openhands.integrations.provider import ProviderToken
|
||||
|
||||
base_secrets = {}
|
||||
self.mock_user_context.get_secrets.return_value = base_secrets
|
||||
self.mock_jwt_service.create_jws_token.return_value = 'test_access_token'
|
||||
@ -134,6 +131,9 @@ class TestLiveStatusAppConversationService:
|
||||
== 'https://test.example.com/api/v1/webhooks/secrets'
|
||||
)
|
||||
assert result['GITHUB_TOKEN'].headers['X-Access-Token'] == 'test_access_token'
|
||||
# Verify descriptions are included
|
||||
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
|
||||
assert result['GITLAB_TOKEN'].description == 'GITLAB authentication token'
|
||||
|
||||
# Should be called twice, once for each provider
|
||||
assert self.mock_jwt_service.create_jws_token.call_count == 2
|
||||
@ -142,10 +142,6 @@ class TestLiveStatusAppConversationService:
|
||||
async def test_setup_secrets_for_git_providers_with_saas_mode(self):
|
||||
"""Test _setup_secrets_for_git_providers with SaaS mode (includes keycloak cookie)."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
from openhands.integrations.provider import ProviderToken
|
||||
|
||||
self.service.app_mode = 'saas'
|
||||
self.service.keycloak_auth_cookie = 'test_cookie'
|
||||
base_secrets = {}
|
||||
@ -169,15 +165,13 @@ class TestLiveStatusAppConversationService:
|
||||
assert isinstance(lookup_secret, LookupSecret)
|
||||
assert 'Cookie' in lookup_secret.headers
|
||||
assert lookup_secret.headers['Cookie'] == 'keycloak_auth=test_cookie'
|
||||
# Verify description is included
|
||||
assert lookup_secret.description == 'GITLAB authentication token'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_setup_secrets_for_git_providers_without_web_url(self):
|
||||
"""Test _setup_secrets_for_git_providers without web URL (uses static token)."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
from openhands.integrations.provider import ProviderToken
|
||||
|
||||
self.service.web_url = None
|
||||
base_secrets = {}
|
||||
self.mock_user_context.get_secrets.return_value = base_secrets
|
||||
@ -198,6 +192,8 @@ class TestLiveStatusAppConversationService:
|
||||
assert 'GITHUB_TOKEN' in result
|
||||
assert isinstance(result['GITHUB_TOKEN'], StaticSecret)
|
||||
assert result['GITHUB_TOKEN'].value.get_secret_value() == 'static_token_value'
|
||||
# Verify description is included
|
||||
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
|
||||
self.mock_user_context.get_latest_token.assert_called_once_with(
|
||||
ProviderType.GITHUB
|
||||
)
|
||||
@ -206,10 +202,6 @@ class TestLiveStatusAppConversationService:
|
||||
async def test_setup_secrets_for_git_providers_no_static_token(self):
|
||||
"""Test _setup_secrets_for_git_providers when no static token is available."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
from openhands.integrations.provider import ProviderToken
|
||||
|
||||
self.service.web_url = None
|
||||
base_secrets = {}
|
||||
self.mock_user_context.get_secrets.return_value = base_secrets
|
||||
@ -230,6 +222,148 @@ class TestLiveStatusAppConversationService:
|
||||
assert 'GITHUB_TOKEN' not in result
|
||||
assert result == base_secrets
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_setup_secrets_for_git_providers_descriptions_included(self):
|
||||
"""Test _setup_secrets_for_git_providers includes descriptions for all provider types."""
|
||||
# Arrange
|
||||
base_secrets = {}
|
||||
self.mock_user_context.get_secrets.return_value = base_secrets
|
||||
self.mock_jwt_service.create_jws_token.return_value = 'test_access_token'
|
||||
|
||||
# Mock provider tokens for multiple providers
|
||||
provider_tokens = {
|
||||
ProviderType.GITHUB: ProviderToken(token=SecretStr('github_token')),
|
||||
ProviderType.GITLAB: ProviderToken(token=SecretStr('gitlab_token')),
|
||||
ProviderType.BITBUCKET: ProviderToken(token=SecretStr('bitbucket_token')),
|
||||
}
|
||||
self.mock_user_context.get_provider_tokens = AsyncMock(
|
||||
return_value=provider_tokens
|
||||
)
|
||||
|
||||
# Act
|
||||
result = await self.service._setup_secrets_for_git_providers(self.mock_user)
|
||||
|
||||
# Assert - verify all secrets have correct descriptions
|
||||
assert 'GITHUB_TOKEN' in result
|
||||
assert isinstance(result['GITHUB_TOKEN'], LookupSecret)
|
||||
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
|
||||
|
||||
assert 'GITLAB_TOKEN' in result
|
||||
assert isinstance(result['GITLAB_TOKEN'], LookupSecret)
|
||||
assert result['GITLAB_TOKEN'].description == 'GITLAB authentication token'
|
||||
|
||||
assert 'BITBUCKET_TOKEN' in result
|
||||
assert isinstance(result['BITBUCKET_TOKEN'], LookupSecret)
|
||||
assert result['BITBUCKET_TOKEN'].description == 'BITBUCKET authentication token'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_setup_secrets_for_git_providers_static_secret_description(self):
|
||||
"""Test _setup_secrets_for_git_providers includes description for StaticSecret."""
|
||||
# Arrange
|
||||
self.service.web_url = None
|
||||
base_secrets = {}
|
||||
self.mock_user_context.get_secrets.return_value = base_secrets
|
||||
self.mock_user_context.get_latest_token.return_value = 'static_token_value'
|
||||
|
||||
# Mock provider tokens for multiple providers
|
||||
provider_tokens = {
|
||||
ProviderType.GITHUB: ProviderToken(token=SecretStr('github_token')),
|
||||
ProviderType.GITLAB: ProviderToken(token=SecretStr('gitlab_token')),
|
||||
}
|
||||
self.mock_user_context.get_provider_tokens = AsyncMock(
|
||||
return_value=provider_tokens
|
||||
)
|
||||
|
||||
# Act
|
||||
result = await self.service._setup_secrets_for_git_providers(self.mock_user)
|
||||
|
||||
# Assert - verify StaticSecret objects have descriptions
|
||||
assert 'GITHUB_TOKEN' in result
|
||||
assert isinstance(result['GITHUB_TOKEN'], StaticSecret)
|
||||
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
|
||||
|
||||
assert 'GITLAB_TOKEN' in result
|
||||
assert isinstance(result['GITLAB_TOKEN'], StaticSecret)
|
||||
assert result['GITLAB_TOKEN'].description == 'GITLAB authentication token'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_setup_secrets_for_git_providers_preserves_custom_secret_descriptions(
|
||||
self,
|
||||
):
|
||||
"""Test _setup_secrets_for_git_providers preserves descriptions from custom secrets."""
|
||||
# Arrange
|
||||
# Mock custom secrets with descriptions
|
||||
custom_secret_with_desc = StaticSecret(
|
||||
value=SecretStr('custom_secret_value'),
|
||||
description='Custom API key for external service',
|
||||
)
|
||||
custom_secret_no_desc = StaticSecret(
|
||||
value=SecretStr('another_secret_value'),
|
||||
description=None,
|
||||
)
|
||||
base_secrets = {
|
||||
'CUSTOM_API_KEY': custom_secret_with_desc,
|
||||
'ANOTHER_SECRET': custom_secret_no_desc,
|
||||
}
|
||||
self.mock_user_context.get_secrets.return_value = base_secrets
|
||||
self.mock_jwt_service.create_jws_token.return_value = 'test_access_token'
|
||||
|
||||
# Mock provider tokens
|
||||
provider_tokens = {
|
||||
ProviderType.GITHUB: ProviderToken(token=SecretStr('github_token')),
|
||||
}
|
||||
self.mock_user_context.get_provider_tokens = AsyncMock(
|
||||
return_value=provider_tokens
|
||||
)
|
||||
|
||||
# Act
|
||||
result = await self.service._setup_secrets_for_git_providers(self.mock_user)
|
||||
|
||||
# Assert - verify custom secrets are preserved with their descriptions
|
||||
assert 'CUSTOM_API_KEY' in result
|
||||
assert isinstance(result['CUSTOM_API_KEY'], StaticSecret)
|
||||
assert (
|
||||
result['CUSTOM_API_KEY'].description
|
||||
== 'Custom API key for external service'
|
||||
)
|
||||
assert (
|
||||
result['CUSTOM_API_KEY'].value.get_secret_value() == 'custom_secret_value'
|
||||
)
|
||||
|
||||
assert 'ANOTHER_SECRET' in result
|
||||
assert isinstance(result['ANOTHER_SECRET'], StaticSecret)
|
||||
assert result['ANOTHER_SECRET'].description is None
|
||||
assert (
|
||||
result['ANOTHER_SECRET'].value.get_secret_value() == 'another_secret_value'
|
||||
)
|
||||
|
||||
# Verify git provider token is also included
|
||||
assert 'GITHUB_TOKEN' in result
|
||||
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_setup_secrets_for_git_providers_custom_secret_empty_description(
|
||||
self,
|
||||
):
|
||||
"""Test _setup_secrets_for_git_providers handles custom secrets with empty descriptions."""
|
||||
# Arrange
|
||||
custom_secret_empty_desc = StaticSecret(
|
||||
value=SecretStr('secret_value'),
|
||||
description='', # Empty string description
|
||||
)
|
||||
base_secrets = {'MY_SECRET': custom_secret_empty_desc}
|
||||
self.mock_user_context.get_secrets.return_value = base_secrets
|
||||
self.mock_user_context.get_provider_tokens = AsyncMock(return_value=None)
|
||||
|
||||
# Act
|
||||
result = await self.service._setup_secrets_for_git_providers(self.mock_user)
|
||||
|
||||
# Assert - empty description should be preserved as-is
|
||||
assert 'MY_SECRET' in result
|
||||
assert isinstance(result['MY_SECRET'], StaticSecret)
|
||||
# Empty string description is preserved
|
||||
assert result['MY_SECRET'].description == ''
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_configure_llm_and_mcp_with_custom_model(self):
|
||||
"""Test _configure_llm_and_mcp with custom LLM model."""
|
||||
@ -360,8 +494,6 @@ class TestLiveStatusAppConversationService:
|
||||
async def test_configure_llm_and_mcp_tavily_with_user_search_api_key(self):
|
||||
"""Test _configure_llm_and_mcp adds tavily when user has search_api_key."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
self.mock_user.search_api_key = SecretStr('user_search_key')
|
||||
self.mock_user_context.get_mcp_api_key.return_value = 'mcp_api_key'
|
||||
|
||||
@ -406,8 +538,6 @@ class TestLiveStatusAppConversationService:
|
||||
async def test_configure_llm_and_mcp_tavily_user_key_takes_precedence(self):
|
||||
"""Test _configure_llm_and_mcp user search_api_key takes precedence over env key."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
self.mock_user.search_api_key = SecretStr('user_search_key')
|
||||
self.service.tavily_api_key = 'env_tavily_key'
|
||||
self.mock_user_context.get_mcp_api_key.return_value = None
|
||||
@ -476,8 +606,6 @@ class TestLiveStatusAppConversationService:
|
||||
Even in SAAS mode, if the user has their own search_api_key, tavily should be added.
|
||||
"""
|
||||
# Arrange - simulate SAAS mode with user having their own search key
|
||||
from pydantic import SecretStr
|
||||
|
||||
self.service.app_mode = AppMode.SAAS.value
|
||||
self.service.tavily_api_key = None # In SAAS mode, this should be None
|
||||
self.mock_user.search_api_key = SecretStr('user_search_key')
|
||||
@ -502,8 +630,6 @@ class TestLiveStatusAppConversationService:
|
||||
async def test_configure_llm_and_mcp_tavily_with_empty_user_search_key(self):
|
||||
"""Test _configure_llm_and_mcp handles empty user search_api_key correctly."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
self.mock_user.search_api_key = SecretStr('') # Empty string
|
||||
self.service.tavily_api_key = 'env_tavily_key'
|
||||
self.mock_user_context.get_mcp_api_key.return_value = None
|
||||
@ -527,8 +653,6 @@ class TestLiveStatusAppConversationService:
|
||||
async def test_configure_llm_and_mcp_tavily_with_whitespace_user_search_key(self):
|
||||
"""Test _configure_llm_and_mcp handles whitespace-only user search_api_key correctly."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
self.mock_user.search_api_key = SecretStr(' ') # Whitespace only
|
||||
self.service.tavily_api_key = 'env_tavily_key'
|
||||
self.mock_user_context.get_mcp_api_key.return_value = None
|
||||
@ -1072,8 +1196,6 @@ class TestLiveStatusAppConversationService:
|
||||
async def test_configure_llm_and_mcp_merges_system_and_custom_servers(self):
|
||||
"""Test _configure_llm_and_mcp merges both system and custom MCP servers."""
|
||||
# Arrange
|
||||
from pydantic import SecretStr
|
||||
|
||||
from openhands.core.config.mcp_config import (
|
||||
MCPConfig,
|
||||
MCPSSEServerConfig,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user