diff --git a/app/cabinet/routes/account_linking.py b/app/cabinet/routes/account_linking.py index 40163463..bc4dacd5 100644 --- a/app/cabinet/routes/account_linking.py +++ b/app/cabinet/routes/account_linking.py @@ -18,7 +18,7 @@ from app.database.crud.user import ( set_user_oauth_provider_id, ) from app.database.models import User -from app.services.account_merge_service import execute_merge, get_merge_preview +from app.services.account_merge_service import _compute_auth_methods, execute_merge, get_merge_preview from ..auth.merge_service import ( MERGE_TOKEN_TTL_SECONDS, @@ -137,15 +137,7 @@ def _get_provider_identifier(user: User, provider: str) -> str | None: def _count_auth_methods(user: User) -> int: """Count how many auth methods the user has linked.""" - count = 0 - if user.telegram_id: - count += 1 - if user.email and user.password_hash: - count += 1 - for column in _OAUTH_PROVIDER_COLUMNS.values(): - if getattr(user, column, None): - count += 1 - return count + return len(_compute_auth_methods(user)) # --------------------------------------------------------------------------- @@ -237,6 +229,20 @@ async def link_provider_callback( detail='Invalid or expired OAuth state', ) + # 1b. Validate that the user who initiated the link flow is the same user completing it + state_user_id = state_data.get('user_id') + if state_user_id and str(user.id) != state_user_id: + logger.warning( + 'OAuth state user_id mismatch in link callback', + state_user_id=state_user_id, + current_user_id=user.id, + provider=provider, + ) + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail='OAuth state was initiated by a different user', + ) + # 2. Get provider instance oauth_provider = get_provider(provider) if not oauth_provider: @@ -256,7 +262,7 @@ async def link_provider_callback( try: token_data = await oauth_provider.exchange_code(request.code, **exchange_kwargs) except Exception as exc: - logger.error('OAuth code exchange failed during linking', provider=provider, exc_info=exc) + logger.error('OAuth code exchange failed during linking', provider=provider, exc_info=True) raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail='Failed to exchange authorization code', @@ -266,7 +272,7 @@ async def link_provider_callback( try: user_info = await oauth_provider.get_user_info(token_data) except Exception as exc: - logger.error('OAuth user info fetch failed during linking', provider=provider, exc_info=exc) + logger.error('OAuth user info fetch failed during linking', provider=provider, exc_info=True) raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail='Failed to fetch user information from provider', @@ -445,11 +451,11 @@ async def execute_merge_endpoint( logger.error('Merge execution failed (ValueError)', error=str(exc)) raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=str(exc), + detail='Account merge cannot be completed. The accounts may have already been merged or deleted.', ) from exc except Exception as exc: await db.rollback() - logger.error('Merge execution failed', exc_info=exc) + logger.error('Merge execution failed', exc_info=True) raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail='Account merge failed due to an internal error', @@ -468,7 +474,7 @@ async def execute_merge_endpoint( auth_response = await _create_auth_response(merged_user, db) await _store_refresh_token(db, merged_user.id, auth_response.refresh_token, device_info='merge') except Exception as exc: - logger.error('Failed to create auth tokens after merge', exc_info=exc) + logger.error('Failed to create auth tokens after merge', exc_info=True) raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail='Merge succeeded but failed to create new session', diff --git a/app/services/account_merge_service.py b/app/services/account_merge_service.py index 98840608..6fcd3490 100644 --- a/app/services/account_merge_service.py +++ b/app/services/account_merge_service.py @@ -1,8 +1,7 @@ -from __future__ import annotations - +from collections.abc import AsyncIterator from contextlib import asynccontextmanager from datetime import UTC, datetime -from typing import Any +from typing import Any, Literal import structlog from sqlalchemy import update @@ -148,7 +147,7 @@ async def get_merge_preview( @asynccontextmanager -async def _get_remnawave_api() -> RemnaWaveAPI: +async def _get_remnawave_api() -> AsyncIterator[RemnaWaveAPI]: """Создаёт экземпляр RemnaWave API клиента (паттерн из RemnaWaveService).""" auth_params = settings.get_remnawave_auth_params() base_url = (auth_params.get('base_url') or '').strip() @@ -308,7 +307,7 @@ async def execute_merge( db: AsyncSession, primary_user_id: int, secondary_user_id: int, - keep_subscription_from: str = 'primary', + keep_subscription_from: Literal['primary', 'secondary'] = 'primary', provider: str | None = None, provider_id: str | None = None, ) -> User: @@ -330,6 +329,9 @@ async def execute_merge( Raises: ValueError: Если пользователь не найден, совпадают ID, или secondary уже удалён. """ + if keep_subscription_from not in ('primary', 'secondary'): + raise ValueError("keep_subscription_from must be 'primary' or 'secondary'") + if primary_user_id == secondary_user_id: raise ValueError('primary_user_id и secondary_user_id не могут совпадать') @@ -338,6 +340,8 @@ async def execute_merge( if not primary: raise ValueError(f'Основной пользователь (id={primary_user_id}) не найден') + if primary.status == UserStatus.DELETED.value: + raise ValueError(f'Основной пользователь (id={primary_user_id}) удалён') if not secondary: raise ValueError(f'Вторичный пользователь (id={secondary_user_id}) не найден') if secondary.status == UserStatus.DELETED.value: @@ -461,13 +465,17 @@ async def execute_merge( value=primary.referral_commission_percent, ) - # 14. Помечаем secondary как удалённый + # 14. Помечаем secondary как удалённый и очищаем ВСЕ unique constraint поля secondary.status = UserStatus.DELETED.value secondary.referral_code = None secondary.remnawave_uuid = None - # email уже очищен выше если был перенесён, иначе очищаем для unique constraint if secondary.email: secondary.email = None + if secondary.telegram_id: + secondary.telegram_id = None + for field in _OAUTH_FIELDS: + if getattr(secondary, field) is not None: + setattr(secondary, field, None) secondary.updated_at = now logger.info( diff --git a/tests/services/test_account_merge_service.py b/tests/services/test_account_merge_service.py index ccfcaf8b..44445191 100644 --- a/tests/services/test_account_merge_service.py +++ b/tests/services/test_account_merge_service.py @@ -273,6 +273,23 @@ class TestExecuteMergeValidation: with pytest.raises(ValueError, match='уже удалён'): await execute_merge(db, 1, 2) + async def test_deleted_primary_raises(self, monkeypatch): + db = _make_db() + primary = _make_user(id=1, status='deleted') + secondary = _make_user(id=2) + monkeypatch.setattr( + account_merge_service, + 'get_user_by_id', + AsyncMock(side_effect=[primary, secondary]), + ) + with pytest.raises(ValueError, match='удалён'): + await execute_merge(db, 1, 2) + + async def test_invalid_keep_subscription_from_raises(self): + db = _make_db() + with pytest.raises(ValueError, match=r'primary.*secondary'): + await execute_merge(db, 1, 2, keep_subscription_from='invalid') + # --------------------------------------------------------------------------- # execute_merge — data transfer @@ -325,6 +342,8 @@ class TestExecuteMergeOAuthTransfer: # Primary keeps its own google_id assert result.google_id == 'g_primary' + # Secondary's conflicting google_id is cleared (unique constraint cleanup) + assert secondary.google_id is None class TestExecuteMergeTelegramTransfer: @@ -513,6 +532,40 @@ class TestExecuteMergeSecondaryDeleted: assert secondary.remnawave_uuid is None assert secondary.email is None + async def test_all_unique_fields_cleared_on_secondary(self, monkeypatch): + """All unique constraint fields must be cleared on secondary after merge.""" + db = _make_db() + # Primary has its own OAuth + telegram, so secondary's won't transfer + primary = _make_user(id=1, telegram_id=111, google_id='g1', yandex_id='y1') + secondary = _make_user( + id=2, + telegram_id=222, + google_id='g2', + yandex_id='y2', + discord_id='d2', + vk_id=999, + email='sec@e.com', + referral_code='REF', + remnawave_uuid='rw-sec', + ) + monkeypatch.setattr( + account_merge_service, + 'get_user_by_id', + AsyncMock(side_effect=[primary, secondary]), + ) + with _patch_remnawave_delete(): + await execute_merge(db, 1, 2) + + # ALL unique fields cleared on secondary + assert secondary.telegram_id is None + assert secondary.google_id is None + assert secondary.yandex_id is None + assert secondary.discord_id is None + assert secondary.vk_id is None + assert secondary.email is None + assert secondary.referral_code is None + assert secondary.remnawave_uuid is None + async def test_db_flush_called(self, monkeypatch): db = _make_db() primary = _make_user(id=1)