Merge pull request #2689 from langbot-app/test/quality-unit-api-identity-20261017

test: verify identity persistence and credential boundaries

tests/unit_tests/api/service/test_space_service.py

@@ -15,6 +15,7 @@ from __future__ import annotations
 
 from collections import OrderedDict
 import json
+import sqlalchemy
 from urllib.parse import parse_qs, urlsplit
 
 import pytest
@@ -25,6 +26,7 @@ import time
 
 from langbot.pkg.api.http.service.space import SpaceService
 from langbot.pkg.entity.persistence.user import User
+from langbot.pkg.entity.persistence.model import LLMModel, ModelProvider
 from langbot.pkg.utils import constants
 
 
@@ -157,6 +159,8 @@ class TestSpaceServiceGetUserByEmail:
         # Verify
         assert result is not None
         assert result.user == '[email protected]'
+        statement = ap.persistence_mgr.execute_async.await_args.args[0]
+        assert statement.whereclause.compare(User.user == '[email protected]')
 
     async def test_get_user_by_email_not_found(self):
         """Returns None when user not found."""
@@ -478,6 +482,9 @@ class TestSpaceServiceRefreshToken:
 
         # Verify
         assert result['access_token'] == 'new_access_token'
+        mock_session_obj.post.assert_called_once_with(
+            'https://space.langbot.app/api/v1/accounts/token/refresh', json={'refresh_token': 'old_refresh_token'}
+        )
 
     async def test_refresh_token_api_error(self):
         """Raises ValueError on API error."""
@@ -662,6 +669,9 @@ class TestSpaceServiceGetUserInfoRaw:
         # Verify
         assert result['email'] == '[email protected]'
         assert result['credits'] == 100
+        mock_session_obj.get.assert_called_once_with(
+            'https://space.langbot.app/api/v1/accounts/me', headers={'Authorization': 'Bearer access_token'}
+        )
 
     async def test_get_user_info_raw_api_error(self):
         """Raises ValueError on API error."""
@@ -737,7 +747,8 @@ class TestSpaceServiceGetUserInfo:
         result = await service.get_user_info('[email protected]')
 
         # Verify
-        assert result['email'] == '[email protected]'
+        assert result == {'email': '[email protected]', 'credits': 100}
+        service.get_user_info_raw.assert_awaited_once_with('valid_token')
 
 
 class TestSpaceServiceGetModels:
@@ -980,6 +991,20 @@ class TestSpaceServiceGetModelSelection:
         service.get_model_selection.assert_awaited_once_with('chat')
         model_mgr.sync_new_models_from_space.assert_awaited_once()
         assert persistence.execute_async.await_count == 2
+        execution = model_mgr.sync_new_models_from_space.await_args.args[0]
+        assert (execution.instance_uuid, execution.workspace_uuid, execution.placement_generation) == (
+            'instance',
+            'workspace',
+            1,
+        )
+        for call in persistence.execute_async.await_args_list:
+            assert call.args[0].whereclause.compare(
+                sqlalchemy.and_(
+                    LLMModel.workspace_uuid == 'workspace',
+                    ModelProvider.requester == 'space-chat-completions',
+                    sqlalchemy.or_(LLMModel.uuid == 'best-upstream-uuid', LLMModel.name == 'best-chat-model'),
+                )
+            )
 
 
 class TestSpaceServiceCreditsCache:

tests/unit_tests/api/service/test_user_service.py

@@ -13,6 +13,7 @@ Source: src/langbot/pkg/api/http/service/user.py
 
 from __future__ import annotations
 
+import asyncio
 import pytest
 import jwt
 import datetime
@@ -28,6 +29,7 @@ from langbot.pkg.api.http.service.user import (
     UserService,
 )
 from langbot.pkg.entity.persistence.user import AccountSource, AccountStatus, User
+from langbot.pkg.entity.persistence.workspace import WorkspaceMembership, MembershipRole, MembershipStatus
 from langbot.pkg.entity.errors.account import (
     AccountEmailMismatchError,
     SpaceAccountBindingRequiredError,
@@ -269,6 +271,8 @@ class TestUserServiceGetUserByEmail:
         # Verify
         assert result is not None
         assert result.user == '[email protected]'
+        statement = ap.persistence_mgr.execute_async.await_args.args[0]
+        assert statement.whereclause.compare(User.normalized_email == '[email protected]')
 
     async def test_get_user_by_email_not_found(self):
         """Returns None when user not found."""
@@ -327,6 +331,8 @@ class TestUserServiceGetUserBySpaceAccountUuid:
         # Verify
         assert result is not None
         assert result.space_account_uuid == 'space-uuid-123'
+        statement = ap.persistence_mgr.execute_async.await_args.args[0]
+        assert statement.whereclause.compare(User.space_account_uuid == 'space-uuid-123')
 
     async def test_get_user_by_space_uuid_not_found(self):
         """Returns None when Space UUID not found."""
@@ -621,6 +627,16 @@ class TestUserServiceSetPassword:
             await service.set_password('[email protected]', 'new_password')
 
 
+def _space_user_write(persistence, operation, credentials):
+    persistence.execute_async.assert_awaited_once()
+    statement = persistence.execute_async.await_args.args[0]
+    assert getattr(statement, f'is_{operation}')
+    assert statement.table.name == User.__tablename__
+    values = statement.compile().params
+    assert {key: values.get(key) for key in credentials} == credentials
+    return statement, values
+
+
 class TestUserServiceCreateOrUpdateSpaceUser:
     """Tests for create_or_update_space_user method."""
 
@@ -657,7 +673,17 @@ class TestUserServiceCreateOrUpdateSpaceUser:
         )
 
         # Verify - update was called and user returned
-        ap.persistence_mgr.execute_async.assert_called()
+        statement, values = _space_user_write(
+            ap.persistence_mgr,
+            'update',
+            {
+                'space_access_token': 'new_access_token',
+                'space_refresh_token': 'new_refresh_token',
+                'space_api_key': 'new_api_key',
+            },
+        )
+        assert statement.whereclause.compare(User.space_account_uuid == 'existing-space-uuid')
+        assert isinstance(values['space_access_token_expires_at'], datetime.datetime)
         assert updated_user.space_account_uuid == 'existing-space-uuid'
 
     async def test_cloud_login_updates_only_the_projected_space_account(self):
@@ -690,7 +716,23 @@ class TestUserServiceCreateOrUpdateSpaceUser:
         )
 
         assert result is projected
-        persistence.execute_async.assert_awaited_once()
+        statement, values = _space_user_write(
+            persistence,
+            'update',
+            {
+                'space_access_token': 'access-token',
+                'space_refresh_token': 'refresh-token',
+                'space_api_key': 'api-key',
+            },
+        )
+        assert statement.whereclause.compare(
+            sqlalchemy.and_(
+                User.uuid == 'projected-space-uuid',
+                User.space_account_uuid == 'projected-space-uuid',
+                User.source == AccountSource.CLOUD_PROJECTION.value,
+            )
+        )
+        assert isinstance(values['space_access_token_expires_at'], datetime.datetime)
 
     async def test_cloud_login_never_creates_an_unprojected_account(self):
         persistence = SimpleNamespace(execute_async=AsyncMock())
@@ -770,6 +812,20 @@ class TestUserServiceCreateOrUpdateSpaceUser:
 
         # Verify
         assert result.space_account_uuid == 'new-space-uuid'
+        _space_user_write(
+            ap.persistence_mgr,
+            'insert',
+            {
+                'user': '[email protected]',
+                'normalized_email': '[email protected]',
+                'password': '',
+                'account_type': 'space',
+                'space_account_uuid': 'new-space-uuid',
+                'space_access_token': 'access_token',
+                'space_refresh_token': 'refresh_token',
+                'space_api_key': 'api_key',
+            },
+        )
 
     async def test_create_or_update_space_user_already_initialized_reports_unknown_space_email(self):
         """Unknown Space email is distinct from an existing local Account collision."""
@@ -914,6 +970,17 @@ class TestUserServiceCreateOrUpdateSpaceUser:
         # Verify
         assert result is not None
         assert result.space_account_uuid == 'noexpiry-uuid'
+        _space_user_write(
+            ap.persistence_mgr,
+            'insert',
+            {
+                'space_account_uuid': 'noexpiry-uuid',
+                'space_access_token': 'token',
+                'space_refresh_token': 'refresh',
+                'space_api_key': 'key',
+                'space_access_token_expires_at': None,
+            },
+        )
 
     async def test_bind_space_account_rejects_different_email(self):
         service = UserService(SimpleNamespace())
@@ -941,13 +1008,23 @@ class TestUserServiceCreateOrUpdateSpaceUser:
     async def test_get_workspace_owner_returns_user_object_from_core_connection_result(self):
         service = UserService(SimpleNamespace())
         owner = _create_mock_user('[email protected]', password='pw')
-        service.ap.persistence_mgr = SimpleNamespace(
-            current_session=lambda: SimpleNamespace(scalar=AsyncMock(return_value=owner)),
-        )
+        session = SimpleNamespace(scalar=AsyncMock(return_value=owner))
+        service.ap.persistence_mgr = SimpleNamespace(current_session=lambda: session)
 
         resolved = await service.get_workspace_owner('workspace-1')
 
         assert resolved is owner
+        session.scalar.assert_awaited_once()
+        statement = session.scalar.await_args.args[0]
+        assert statement.whereclause.compare(
+            sqlalchemy.and_(
+                WorkspaceMembership.workspace_uuid == 'workspace-1',
+                WorkspaceMembership.role == MembershipRole.OWNER.value,
+                WorkspaceMembership.status == MembershipStatus.ACTIVE.value,
+                User.status == AccountStatus.ACTIVE.value,
+            )
+        )
+        assert statement.get_final_froms()[0].onclause.compare(WorkspaceMembership.account_uuid == User.uuid)
 
 
 class TestUserServiceLoginCapabilities:
@@ -980,4 +1057,5 @@ class TestUserServiceCreateUserLock:
 
         # Verify lock exists
         assert hasattr(service, '_create_user_lock')
-        assert service._create_user_lock is not None
+        assert isinstance(service._create_user_lock, asyncio.Lock)
+        assert not service._create_user_lock.locked()

tests/unit_tests/api/test_apikey_service.py

@@ -3,10 +3,12 @@ from __future__ import annotations
 from types import SimpleNamespace
 from unittest.mock import AsyncMock, Mock
 
+import hashlib
+import sqlalchemy
 import pytest
 
 from langbot.pkg.api.http.service.apikey import ApiKeyService
-from langbot.pkg.entity.persistence.apikey import ApiKeyStatus
+from langbot.pkg.entity.persistence.apikey import ApiKey, ApiKeyStatus
 
 
 @pytest.mark.asyncio
@@ -76,6 +78,27 @@ async def test_verify_api_key_keeps_db_validation_for_lbk_keys(key_exists):
     result = await service.verify_api_key('lbk_valid_format')
 
     assert result is key_exists
+    digest = hashlib.sha256(b'lbk_valid_format').hexdigest()
+    statements = [call.args[0] for call in persistence_mgr.execute_async.await_args_list]
+    assert statements[0].whereclause.compare(ApiKey.key_hash == digest)
+    if key_exists:
+        assert statements[1].whereclause.compare(
+            sqlalchemy.and_(
+                ApiKey.id == 1,
+                ApiKey.workspace_uuid == 'workspace-a',
+                ApiKey.key_hash == digest,
+            )
+        )
+        assert statements[2].is_update
+        assert statements[2].whereclause.compare(
+            sqlalchemy.and_(
+                ApiKey.id == 1,
+                ApiKey.workspace_uuid == 'workspace-a',
+                ApiKey.key_hash == digest,
+                ApiKey.status == ApiKeyStatus.ACTIVE.value,
+            )
+        )
+        assert statements[2].compile().params['last_used_at'] is not None
     assert persistence_mgr.execute_async.await_count == (3 if key_exists else 1)
     if key_exists:
         workspace_service.get_execution_binding.assert_awaited_once_with('workspace-a')

tests/unit_tests/api/test_user_reset_password.py

@@ -16,7 +16,7 @@ import asyncio
 import logging
 import time
 from types import SimpleNamespace
-from unittest.mock import AsyncMock
+from unittest.mock import AsyncMock, Mock, call
 
 import pytest
 import quart
@@ -65,13 +65,17 @@ def _make_genkeys_ap(existing_key: str) -> SimpleNamespace:
     )
 
 
-async def test_recovery_key_generation_is_short_and_unambiguous():
+async def test_recovery_key_generation_is_short_and_unambiguous(monkeypatch):
     """Eight random base32 characters balance manual entry and online throttling."""
     ap = _make_genkeys_ap(existing_key='')
+    choice = Mock(side_effect=list('ZYXW9876'))
+    monkeypatch.setattr('langbot.pkg.core.stages.genkeys.secrets.choice', choice)
 
     await GenKeysStage().run(ap)
 
     key = ap.instance_config.data['system']['recovery_key']
+    assert key == 'ZYXW9876'
+    assert choice.call_args_list == [call('23456789ABCDEFGHJKLMNPQRSTUVWXYZ')] * 8
     assert len(key) == 8
     assert set(key) <= set('23456789ABCDEFGHJKLMNPQRSTUVWXYZ')
     assert ap.instance_config.dump_config.called
@@ -198,7 +202,7 @@ async def test_non_string_recovery_key_does_not_crash():
 @pytest.mark.parametrize('key', ['奇数密钥不是ASCII', '\ud800', '\udfff'])
 async def test_non_ascii_recovery_key_does_not_crash(key):
     """Non-ASCII keys must compare safely (encode-based constant-time compare)."""
-    client, _, _ = await _create_client()
+    client, reset_password, _ = await _create_client()
 
     resp = await client.post(
         '/api/v1/user/reset-password',
@@ -206,6 +210,7 @@ async def test_non_ascii_recovery_key_does_not_crash(key):
     )
 
     assert resp.status_code == 403
+    reset_password.assert_not_awaited()
 
 
 async def test_quota_exhausted_after_max_attempts():