mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-01-09 14:57:59 -05:00
Refactor: rename user secrets table to custom secrets (#11525)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
@@ -31,7 +31,7 @@ from server.utils.conversation_callback_utils import register_callback_processor
|
||||
from openhands.core.logger import openhands_logger as logger
|
||||
from openhands.integrations.provider import ProviderToken, ProviderType
|
||||
from openhands.server.types import LLMAuthenticationError, MissingSettingsError
|
||||
from openhands.storage.data_models.user_secrets import UserSecrets
|
||||
from openhands.storage.data_models.secrets import Secrets
|
||||
from openhands.utils.async_utils import call_sync_from_async
|
||||
|
||||
|
||||
@@ -250,7 +250,7 @@ class GithubManager(Manager):
|
||||
f'[GitHub] Creating new conversation for user {user_info.username}'
|
||||
)
|
||||
|
||||
secret_store = UserSecrets(
|
||||
secret_store = Secrets(
|
||||
provider_tokens=MappingProxyType(
|
||||
{
|
||||
ProviderType.GITHUB: ProviderToken(
|
||||
|
||||
@@ -25,7 +25,7 @@ from openhands.core.logger import openhands_logger as logger
|
||||
from openhands.integrations.gitlab.gitlab_service import GitLabServiceImpl
|
||||
from openhands.integrations.provider import ProviderToken, ProviderType
|
||||
from openhands.server.types import LLMAuthenticationError, MissingSettingsError
|
||||
from openhands.storage.data_models.user_secrets import UserSecrets
|
||||
from openhands.storage.data_models.secrets import Secrets
|
||||
|
||||
|
||||
class GitlabManager(Manager):
|
||||
@@ -198,7 +198,7 @@ class GitlabManager(Manager):
|
||||
f'[GitLab] Creating new conversation for user {user_info.username}'
|
||||
)
|
||||
|
||||
secret_store = UserSecrets(
|
||||
secret_store = Secrets(
|
||||
provider_tokens=MappingProxyType(
|
||||
{
|
||||
ProviderType.GITLAB: ProviderToken(
|
||||
|
||||
@@ -57,7 +57,7 @@ class JiraNewConversationView(JiraViewInterface):
|
||||
raise StartingConvoException('No repository selected for this conversation')
|
||||
|
||||
provider_tokens = await self.saas_user_auth.get_provider_tokens()
|
||||
user_secrets = await self.saas_user_auth.get_user_secrets()
|
||||
user_secrets = await self.saas_user_auth.get_secrets()
|
||||
instructions, user_msg = self._get_instructions(jinja_env)
|
||||
|
||||
try:
|
||||
|
||||
@@ -60,7 +60,7 @@ class JiraDcNewConversationView(JiraDcViewInterface):
|
||||
raise StartingConvoException('No repository selected for this conversation')
|
||||
|
||||
provider_tokens = await self.saas_user_auth.get_provider_tokens()
|
||||
user_secrets = await self.saas_user_auth.get_user_secrets()
|
||||
user_secrets = await self.saas_user_auth.get_secrets()
|
||||
instructions, user_msg = self._get_instructions(jinja_env)
|
||||
|
||||
try:
|
||||
|
||||
@@ -57,7 +57,7 @@ class LinearNewConversationView(LinearViewInterface):
|
||||
raise StartingConvoException('No repository selected for this conversation')
|
||||
|
||||
provider_tokens = await self.saas_user_auth.get_provider_tokens()
|
||||
user_secrets = await self.saas_user_auth.get_user_secrets()
|
||||
user_secrets = await self.saas_user_auth.get_secrets()
|
||||
instructions, user_msg = self._get_instructions(jinja_env)
|
||||
|
||||
try:
|
||||
|
||||
@@ -186,7 +186,7 @@ class SlackNewConversationView(SlackViewInterface):
|
||||
self._verify_necessary_values_are_set()
|
||||
|
||||
provider_tokens = await self.saas_user_auth.get_provider_tokens()
|
||||
user_secrets = await self.saas_user_auth.get_user_secrets()
|
||||
user_secrets = await self.saas_user_auth.get_secrets()
|
||||
user_instructions, conversation_instructions = self._get_instructions(jinja)
|
||||
|
||||
# Determine git provider from repository
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
"""rename user_secrets table to custom_secrets
|
||||
|
||||
Revision ID: 079
|
||||
Revises: 078
|
||||
Create Date: 2025-10-27 00:00:00.000000
|
||||
|
||||
"""
|
||||
|
||||
from typing import Sequence, Union
|
||||
|
||||
from alembic import op
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision: str = '079'
|
||||
down_revision: Union[str, None] = '078'
|
||||
branch_labels: Union[str, Sequence[str], None] = None
|
||||
depends_on: Union[str, Sequence[str], None] = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
# Rename the table from user_secrets to custom_secrets
|
||||
op.rename_table('user_secrets', 'custom_secrets')
|
||||
|
||||
# Rename the index to match the new table name
|
||||
op.drop_index('idx_user_secrets_keycloak_user_id', 'custom_secrets')
|
||||
op.create_index(
|
||||
'idx_custom_secrets_keycloak_user_id', 'custom_secrets', ['keycloak_user_id']
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
# Rename the index back to the original name
|
||||
op.drop_index('idx_custom_secrets_keycloak_user_id', 'custom_secrets')
|
||||
op.create_index(
|
||||
'idx_user_secrets_keycloak_user_id', 'custom_secrets', ['keycloak_user_id']
|
||||
)
|
||||
|
||||
# Rename the table back from custom_secrets to user_secrets
|
||||
op.rename_table('custom_secrets', 'user_secrets')
|
||||
@@ -31,7 +31,7 @@ from openhands.integrations.provider import (
|
||||
)
|
||||
from openhands.server.settings import Settings
|
||||
from openhands.server.user_auth.user_auth import AuthType, UserAuth
|
||||
from openhands.storage.data_models.user_secrets import UserSecrets
|
||||
from openhands.storage.data_models.secrets import Secrets
|
||||
from openhands.storage.settings.settings_store import SettingsStore
|
||||
|
||||
token_manager = TokenManager()
|
||||
@@ -52,7 +52,7 @@ class SaasUserAuth(UserAuth):
|
||||
settings_store: SaasSettingsStore | None = None
|
||||
secrets_store: SaasSecretsStore | None = None
|
||||
_settings: Settings | None = None
|
||||
_user_secrets: UserSecrets | None = None
|
||||
_secrets: Secrets | None = None
|
||||
accepted_tos: bool | None = None
|
||||
auth_type: AuthType = AuthType.COOKIE
|
||||
|
||||
@@ -119,13 +119,13 @@ class SaasUserAuth(UserAuth):
|
||||
self.secrets_store = secrets_store
|
||||
return secrets_store
|
||||
|
||||
async def get_user_secrets(self):
|
||||
user_secrets = self._user_secrets
|
||||
async def get_secrets(self):
|
||||
user_secrets = self._secrets
|
||||
if user_secrets:
|
||||
return user_secrets
|
||||
secrets_store = await self.get_secrets_store()
|
||||
user_secrets = await secrets_store.load()
|
||||
self._user_secrets = user_secrets
|
||||
self._secrets = user_secrets
|
||||
return user_secrets
|
||||
|
||||
async def get_access_token(self) -> SecretStr | None:
|
||||
@@ -148,7 +148,7 @@ class SaasUserAuth(UserAuth):
|
||||
if not access_token:
|
||||
raise AuthError()
|
||||
|
||||
user_secrets = await self.get_user_secrets()
|
||||
user_secrets = await self.get_secrets()
|
||||
|
||||
try:
|
||||
# TODO: I think we can do this in a single request if we refactor
|
||||
|
||||
@@ -7,11 +7,11 @@ from dataclasses import dataclass
|
||||
from cryptography.fernet import Fernet
|
||||
from sqlalchemy.orm import sessionmaker
|
||||
from storage.database import session_maker
|
||||
from storage.stored_user_secrets import StoredUserSecrets
|
||||
from storage.stored_custom_secrets import StoredCustomSecrets
|
||||
|
||||
from openhands.core.config.openhands_config import OpenHandsConfig
|
||||
from openhands.core.logger import openhands_logger as logger
|
||||
from openhands.storage.data_models.user_secrets import UserSecrets
|
||||
from openhands.storage.data_models.secrets import Secrets
|
||||
from openhands.storage.secrets.secrets_store import SecretsStore
|
||||
|
||||
|
||||
@@ -21,20 +21,20 @@ class SaasSecretsStore(SecretsStore):
|
||||
session_maker: sessionmaker
|
||||
config: OpenHandsConfig
|
||||
|
||||
async def load(self) -> UserSecrets | None:
|
||||
async def load(self) -> Secrets | None:
|
||||
if not self.user_id:
|
||||
return None
|
||||
|
||||
with self.session_maker() as session:
|
||||
# Fetch all secrets for the given user ID
|
||||
settings = (
|
||||
session.query(StoredUserSecrets)
|
||||
.filter(StoredUserSecrets.keycloak_user_id == self.user_id)
|
||||
session.query(StoredCustomSecrets)
|
||||
.filter(StoredCustomSecrets.keycloak_user_id == self.user_id)
|
||||
.all()
|
||||
)
|
||||
|
||||
if not settings:
|
||||
return UserSecrets()
|
||||
return Secrets()
|
||||
|
||||
kwargs = {}
|
||||
for secret in settings:
|
||||
@@ -45,14 +45,14 @@ class SaasSecretsStore(SecretsStore):
|
||||
|
||||
self._decrypt_kwargs(kwargs)
|
||||
|
||||
return UserSecrets(custom_secrets=kwargs) # type: ignore[arg-type]
|
||||
return Secrets(custom_secrets=kwargs) # type: ignore[arg-type]
|
||||
|
||||
async def store(self, item: UserSecrets):
|
||||
async def store(self, item: Secrets):
|
||||
with self.session_maker() as session:
|
||||
# Incoming secrets are always the most updated ones
|
||||
# Delete all existing records and override with incoming ones
|
||||
session.query(StoredUserSecrets).filter(
|
||||
StoredUserSecrets.keycloak_user_id == self.user_id
|
||||
session.query(StoredCustomSecrets).filter(
|
||||
StoredCustomSecrets.keycloak_user_id == self.user_id
|
||||
).delete()
|
||||
|
||||
# Prepare the new secrets data
|
||||
@@ -74,7 +74,7 @@ class SaasSecretsStore(SecretsStore):
|
||||
|
||||
# Add the new secrets
|
||||
for secret_name, secret_value, description in secret_tuples:
|
||||
new_secret = StoredUserSecrets(
|
||||
new_secret = StoredCustomSecrets(
|
||||
keycloak_user_id=self.user_id,
|
||||
secret_name=secret_name,
|
||||
secret_value=secret_value,
|
||||
|
||||
@@ -2,8 +2,8 @@ from sqlalchemy import Column, Identity, Integer, String
|
||||
from storage.base import Base
|
||||
|
||||
|
||||
class StoredUserSecrets(Base): # type: ignore
|
||||
__tablename__ = 'user_secrets'
|
||||
class StoredCustomSecrets(Base): # type: ignore
|
||||
__tablename__ = 'custom_secrets'
|
||||
id = Column(Integer, Identity(), primary_key=True)
|
||||
keycloak_user_id = Column(String, nullable=True, index=True)
|
||||
secret_name = Column(String, nullable=False)
|
||||
@@ -309,7 +309,7 @@ class TestJiraViewEdgeCases:
|
||||
mock_agent_loop_info,
|
||||
):
|
||||
"""Test conversation creation when user has no secrets"""
|
||||
new_conversation_view.saas_user_auth.get_user_secrets.return_value = None
|
||||
new_conversation_view.saas_user_auth.get_secrets.return_value = None
|
||||
mock_create_conversation.return_value = mock_agent_loop_info
|
||||
mock_store.create_conversation = AsyncMock()
|
||||
|
||||
|
||||
@@ -309,7 +309,7 @@ class TestJiraDcViewEdgeCases:
|
||||
mock_agent_loop_info,
|
||||
):
|
||||
"""Test conversation creation when user has no secrets"""
|
||||
new_conversation_view.saas_user_auth.get_user_secrets.return_value = None
|
||||
new_conversation_view.saas_user_auth.get_secrets.return_value = None
|
||||
mock_create_conversation.return_value = mock_agent_loop_info
|
||||
mock_store.create_conversation = AsyncMock()
|
||||
|
||||
|
||||
@@ -309,7 +309,7 @@ class TestLinearViewEdgeCases:
|
||||
mock_agent_loop_info,
|
||||
):
|
||||
"""Test conversation creation when user has no secrets"""
|
||||
new_conversation_view.saas_user_auth.get_user_secrets.return_value = None
|
||||
new_conversation_view.saas_user_auth.get_secrets.return_value = None
|
||||
mock_create_conversation.return_value = mock_agent_loop_info
|
||||
mock_store.create_conversation = AsyncMock()
|
||||
|
||||
|
||||
@@ -5,11 +5,11 @@ from unittest.mock import MagicMock
|
||||
import pytest
|
||||
from pydantic import SecretStr
|
||||
from storage.saas_secrets_store import SaasSecretsStore
|
||||
from storage.stored_user_secrets import StoredUserSecrets
|
||||
from storage.stored_custom_secrets import StoredCustomSecrets
|
||||
|
||||
from openhands.core.config.openhands_config import OpenHandsConfig
|
||||
from openhands.integrations.provider import CustomSecret
|
||||
from openhands.storage.data_models.user_secrets import UserSecrets
|
||||
from openhands.storage.data_models.secrets import Secrets
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -27,8 +27,8 @@ def secrets_store(session_maker, mock_config):
|
||||
class TestSaasSecretsStore:
|
||||
@pytest.mark.asyncio
|
||||
async def test_store_and_load(self, secrets_store):
|
||||
# Create a UserSecrets object with some test data
|
||||
user_secrets = UserSecrets(
|
||||
# Create a Secrets object with some test data
|
||||
user_secrets = Secrets(
|
||||
custom_secrets=MappingProxyType(
|
||||
{
|
||||
'api_token': CustomSecret.from_value(
|
||||
@@ -60,8 +60,8 @@ class TestSaasSecretsStore:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_encryption_decryption(self, secrets_store):
|
||||
# Create a UserSecrets object with sensitive data
|
||||
user_secrets = UserSecrets(
|
||||
# Create a Secrets object with sensitive data
|
||||
user_secrets = Secrets(
|
||||
custom_secrets=MappingProxyType(
|
||||
{
|
||||
'api_token': CustomSecret.from_value(
|
||||
@@ -87,8 +87,8 @@ class TestSaasSecretsStore:
|
||||
# Verify the data is encrypted in the database
|
||||
with secrets_store.session_maker() as session:
|
||||
stored = (
|
||||
session.query(StoredUserSecrets)
|
||||
.filter(StoredUserSecrets.keycloak_user_id == 'user-id')
|
||||
session.query(StoredCustomSecrets)
|
||||
.filter(StoredCustomSecrets.keycloak_user_id == 'user-id')
|
||||
.first()
|
||||
)
|
||||
|
||||
@@ -154,7 +154,7 @@ class TestSaasSecretsStore:
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_existing_secrets(self, secrets_store):
|
||||
# Create and store initial secrets
|
||||
initial_secrets = UserSecrets(
|
||||
initial_secrets = Secrets(
|
||||
custom_secrets=MappingProxyType(
|
||||
{
|
||||
'api_token': CustomSecret.from_value(
|
||||
@@ -169,7 +169,7 @@ class TestSaasSecretsStore:
|
||||
await secrets_store.store(initial_secrets)
|
||||
|
||||
# Create and store updated secrets
|
||||
updated_secrets = UserSecrets(
|
||||
updated_secrets = Secrets(
|
||||
custom_secrets=MappingProxyType(
|
||||
{
|
||||
'api_token': CustomSecret.from_value(
|
||||
|
||||
Reference in New Issue
Block a user