From c55be6d3fd5483353fa1fefbe9b0022c6857a2c1 Mon Sep 17 00:00:00 2001 From: qzl Date: Mon, 13 Apr 2026 11:28:58 +0800 Subject: [PATCH] fix: preserve points balance across account re-registration Persist a per-email balance snapshot before account deletion and restore it on same-email re-registration, preventing both unintended balance reset and repeated signup bonus grants. --- ...413_0004_register_bonus_claims_snapshot.py | 50 +++++++++++++++++ backend/src/models/register_bonus_claims.py | 9 ++-- backend/src/v1/points/repository.py | 29 +++++++++- backend/src/v1/points/service.py | 19 +++++-- backend/src/v1/users/service.py | 28 ++++++++++ .../test_register_run_delete_reregister.py | 53 ++++++++++++++++++- .../tests/unit/test_points_service_audit.py | 41 +++++++++++++- .../unit/test_user_service_delete_account.py | 8 +-- .../common/user-points-chat-data-protocol.md | 7 +-- 9 files changed, 223 insertions(+), 21 deletions(-) create mode 100644 backend/alembic/versions/20260413_0004_register_bonus_claims_snapshot.py diff --git a/backend/alembic/versions/20260413_0004_register_bonus_claims_snapshot.py b/backend/alembic/versions/20260413_0004_register_bonus_claims_snapshot.py new file mode 100644 index 0000000..14979d9 --- /dev/null +++ b/backend/alembic/versions/20260413_0004_register_bonus_claims_snapshot.py @@ -0,0 +1,50 @@ +"""store register bonus balance snapshot and remove first_user_id fk + +Revision ID: 20260413_0004 +Revises: 20260411_0005 +Create Date: 2026-04-13 00:10:00 +""" + +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +revision: str = "20260413_0004" +down_revision: Union[str, Sequence[str], None] = "20260411_0005" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.execute( + "ALTER TABLE register_bonus_claims DROP CONSTRAINT IF EXISTS register_bonus_claims_first_user_id_fkey" + ) + op.drop_column("register_bonus_claims", "first_user_id") + op.add_column( + "register_bonus_claims", + sa.Column("first_user_id_snapshot", sa.UUID(), nullable=True), + ) + op.add_column( + "register_bonus_claims", + sa.Column("balance_snapshot", sa.BigInteger(), nullable=True), + ) + + +def downgrade() -> None: + op.drop_column("register_bonus_claims", "balance_snapshot") + op.drop_column("register_bonus_claims", "first_user_id_snapshot") + op.add_column( + "register_bonus_claims", + sa.Column("first_user_id", sa.UUID(), nullable=True), + ) + op.create_foreign_key( + "register_bonus_claims_first_user_id_fkey", + "register_bonus_claims", + "users", + ["first_user_id"], + ["id"], + source_schema="public", + referent_schema="auth", + ondelete="SET NULL", + ) diff --git a/backend/src/models/register_bonus_claims.py b/backend/src/models/register_bonus_claims.py index 585b2e1..a86f040 100644 --- a/backend/src/models/register_bonus_claims.py +++ b/backend/src/models/register_bonus_claims.py @@ -2,7 +2,7 @@ from __future__ import annotations import uuid -from sqlalchemy import ForeignKey, String, Text, UniqueConstraint +from sqlalchemy import BigInteger, String, Text, UniqueConstraint from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import Mapped, mapped_column @@ -25,9 +25,8 @@ class RegisterBonusClaims(TimestampMixin, Base): ) email_hash: Mapped[str] = mapped_column(String(64), nullable=False) user_email_snapshot: Mapped[str] = mapped_column(Text, nullable=False) - first_user_id: Mapped[uuid.UUID | None] = mapped_column( - UUID(as_uuid=True), - ForeignKey("auth.users.id", ondelete="SET NULL"), - nullable=True, + first_user_id_snapshot: Mapped[uuid.UUID | None] = mapped_column( + UUID(as_uuid=True), nullable=True ) + balance_snapshot: Mapped[int | None] = mapped_column(BigInteger, nullable=True) grant_event_id: Mapped[str] = mapped_column(String(64), nullable=False) diff --git a/backend/src/v1/points/repository.py b/backend/src/v1/points/repository.py index 9ce6bbc..bd2b800 100644 --- a/backend/src/v1/points/repository.py +++ b/backend/src/v1/points/repository.py @@ -148,7 +148,7 @@ class PointsRepository: *, email_hash: str, user_email_snapshot: str, - first_user_id: UUID, + first_user_id_snapshot: UUID, grant_event_id: str, ) -> bool: stmt = ( @@ -156,7 +156,7 @@ class PointsRepository: .values( email_hash=email_hash, user_email_snapshot=user_email_snapshot, - first_user_id=first_user_id, + first_user_id_snapshot=first_user_id_snapshot, grant_event_id=grant_event_id, ) .on_conflict_do_nothing(index_elements=[RegisterBonusClaims.email_hash]) @@ -164,3 +164,28 @@ class PointsRepository: ) inserted_id = (await self._session.execute(stmt)).scalar_one_or_none() return inserted_id is not None + + async def get_register_bonus_claim( + self, + *, + email_hash: str, + ) -> RegisterBonusClaims | None: + stmt = ( + select(RegisterBonusClaims) + .where(RegisterBonusClaims.email_hash == email_hash) + .limit(1) + ) + return (await self._session.execute(stmt)).scalar_one_or_none() + + async def update_register_bonus_balance_snapshot( + self, + *, + email_hash: str, + balance_snapshot: int, + ) -> bool: + claim = await self.get_register_bonus_claim(email_hash=email_hash) + if claim is None: + return False + claim.balance_snapshot = int(balance_snapshot) + await self._session.flush() + return True diff --git a/backend/src/v1/points/service.py b/backend/src/v1/points/service.py index ba8b0db..61f4f4e 100644 --- a/backend/src/v1/points/service.py +++ b/backend/src/v1/points/service.py @@ -92,15 +92,26 @@ class PointsService: ).hexdigest() event_id = f"register.bonus:{event_hash}" + claim = await self._repository.get_register_bonus_claim(email_hash=email_hash) + account = await self._repository.get_or_create_user_points_for_update( + user_id=user_id + ) + if claim is not None and claim.balance_snapshot is not None: + account.balance = max(int(claim.balance_snapshot), 0) + account.version = int(account.version) + 1 + return RegisterBonusResult( + granted=False, + amount=0, + balance_after=int(account.balance), + event_id=event_id, + ) + claimed = await self._repository.claim_register_bonus( email_hash=email_hash, user_email_snapshot=normalized_email, - first_user_id=user_id, + first_user_id_snapshot=user_id, grant_event_id=event_id, ) - account = await self._repository.get_or_create_user_points_for_update( - user_id=user_id - ) if not claimed: return RegisterBonusResult( granted=False, diff --git a/backend/src/v1/users/service.py b/backend/src/v1/users/service.py index 557ea4b..41cbac7 100644 --- a/backend/src/v1/users/service.py +++ b/backend/src/v1/users/service.py @@ -12,6 +12,8 @@ from core.auth.models import CurrentUser from core.http.errors import ApiProblemError, problem_payload from services.base.supabase import SupabaseService from schemas.shared.user import UserContext, parse_profile_settings +from v1.points.repository import PointsRepository +from v1.points.service import PointsService from v1.users.repository import SQLAlchemyUserRepository from v1.users.schemas import ( AvatarUploadUrlRequest, @@ -294,6 +296,7 @@ class UserService: user_id = str(self.current_user.id) avatar_bucket = config.storage.avatar.bucket avatar_prefix = f"{self.current_user.id}/" + points_repository = PointsRepository(self.repository.session) try: await self.attachment_storage.delete_prefix( @@ -315,6 +318,31 @@ class UserService: ), ) from exc + try: + user_email = (self.current_user.email or "").strip().lower() + if user_email: + email_hash = PointsService._build_register_bonus_email_hash(user_email) + account = await points_repository.get_user_points( + user_id=self.current_user.id + ) + await points_repository.update_register_bonus_balance_snapshot( + email_hash=email_hash, + balance_snapshot=int(account.balance), + ) + await self.repository.session.commit() + except Exception as exc: + logger.exception( + "Account deletion failed while persisting points snapshot", + user_id=user_id, + ) + raise ApiProblemError( + status_code=502, + detail=problem_payload( + code="PROFILE_DELETE_FAILED", + detail="Failed to delete account data", + ), + ) from exc + try: await self.attachment_storage.delete_auth_user(user_id=user_id) except Exception as exc: diff --git a/backend/tests/integration/test_register_run_delete_reregister.py b/backend/tests/integration/test_register_run_delete_reregister.py index 95531fa..9180c25 100644 --- a/backend/tests/integration/test_register_run_delete_reregister.py +++ b/backend/tests/integration/test_register_run_delete_reregister.py @@ -170,7 +170,7 @@ async def test_register_run_delete_reregister_keeps_bonus_single_use( ) reregister_balance.raise_for_status() re_data = reregister_balance.json() - assert int(re_data["balance"]) == 0 + assert int(re_data["balance"]) == int(after_data["balance"]) async with AsyncSessionLocal() as session: points2 = ( @@ -217,3 +217,54 @@ async def test_register_run_delete_reregister_keeps_bonus_single_use( ).scalars() ) assert len(claim_rows) == 1 + assert claim_rows[0].balance_snapshot == int(after_data["balance"]) + + +@pytest.mark.asyncio +async def test_register_delete_reregister_restores_unspent_balance( + api_client: httpx.AsyncClient, + test_identity: IdentityData, +) -> None: + email = str(test_identity["email"]).strip().lower() + bonus = int(config.points_policy.register_bonus_points) + + first = await _create_email_session( + api_client, + email=email, + code=str(test_identity["code"]), + ) + user1 = first.get("user") + assert isinstance(user1, dict) + user1_id = str(user1["id"]) + token1 = str(first["access_token"]) + headers1 = {"Authorization": f"Bearer {token1}"} + + first_balance = await api_client.get("/api/v1/points/balance", headers=headers1) + first_balance.raise_for_status() + first_data = first_balance.json() + assert int(first_data["balance"]) == bonus + + delete_resp = await api_client.delete("/api/v1/users/me", headers=headers1) + assert delete_resp.status_code == 204 + + second = await _create_email_session( + api_client, + email=email, + code=str(test_identity["code"]), + ) + user2 = second.get("user") + assert isinstance(user2, dict) + user2_id = str(user2["id"]) + assert user1_id != user2_id + token2 = str(second["access_token"]) + headers2 = {"Authorization": f"Bearer {token2}"} + + reregister_balance = await api_client.get( + "/api/v1/points/balance", headers=headers2 + ) + reregister_balance.raise_for_status() + re_data = reregister_balance.json() + assert int(re_data["balance"]) == bonus + + cleanup_resp = await api_client.delete("/api/v1/users/me", headers=headers2) + assert cleanup_resp.status_code == 204 diff --git a/backend/tests/unit/test_points_service_audit.py b/backend/tests/unit/test_points_service_audit.py index 03110f4..b357beb 100644 --- a/backend/tests/unit/test_points_service_audit.py +++ b/backend/tests/unit/test_points_service_audit.py @@ -7,6 +7,7 @@ from uuid import UUID, uuid4 import pytest from core.config.settings import config +from models.register_bonus_claims import RegisterBonusClaims from schemas.domain.points import AppendAuditLedgerCommand, ApplyPointsChangeCommand from schemas.domain.points import PointsChargeSnapshot from v1.points.service import PointsService @@ -28,6 +29,7 @@ class _FakePointsRepository: self.appended_ledger: list[ApplyPointsChangeCommand] = [] self.appended_audit: list[AppendAuditLedgerCommand] = [] self.claimed: bool = False + self.claim: RegisterBonusClaims | None = None async def get_or_create_user_points_for_update( self, *, user_id: UUID @@ -69,15 +71,23 @@ class _FakePointsRepository: *, email_hash: str, user_email_snapshot: str, - first_user_id: UUID, + first_user_id_snapshot: UUID, grant_event_id: str, ) -> bool: - del email_hash, user_email_snapshot, first_user_id, grant_event_id + del email_hash, user_email_snapshot, first_user_id_snapshot, grant_event_id if self.claimed: return False self.claimed = True return True + async def get_register_bonus_claim( + self, + *, + email_hash: str, + ) -> RegisterBonusClaims | None: + del email_hash + return self.claim + @pytest.mark.asyncio async def test_consume_successful_run_points_writes_real_usage_to_audit() -> None: @@ -188,3 +198,30 @@ async def test_grant_register_bonus_if_eligible_second_time_skips() -> None: assert result.amount == 0 assert len(repository.appended_ledger) == 0 assert len(repository.appended_audit) == 0 + + +@pytest.mark.asyncio +async def test_grant_register_bonus_if_eligible_restores_balance_snapshot() -> None: + repository = _FakePointsRepository(usage_snapshot=None) + repository.account.balance = 0 + repository.claim = RegisterBonusClaims( + email_hash="abc", + user_email_snapshot="restore@example.com", + first_user_id_snapshot=uuid4(), + balance_snapshot=35, + grant_event_id="event", + ) + service = PointsService(repository=repository) # type: ignore[arg-type] + + result = await service.grant_register_bonus_if_eligible( + user_id=uuid4(), + user_email="restore@example.com", + ) + + assert result.granted is False + assert result.amount == 0 + assert result.balance_after == 35 + assert repository.account.balance == 35 + assert repository.claimed is False + assert len(repository.appended_ledger) == 0 + assert len(repository.appended_audit) == 0 diff --git a/backend/tests/unit/test_user_service_delete_account.py b/backend/tests/unit/test_user_service_delete_account.py index e83abac..0f5b1b5 100644 --- a/backend/tests/unit/test_user_service_delete_account.py +++ b/backend/tests/unit/test_user_service_delete_account.py @@ -10,7 +10,7 @@ from v1.users.service import UserService class _NoopRepository: - pass + session = None class _FakeStorage: @@ -28,7 +28,7 @@ class _FakeStorage: @pytest.mark.asyncio async def test_delete_account_success_calls_storage_cleanup_and_auth_delete() -> None: - user = CurrentUser(id=uuid4(), email="test@example.com") + user = CurrentUser(id=uuid4(), email=None) storage = _FakeStorage() service = UserService( current_user=user, @@ -46,7 +46,7 @@ async def test_delete_account_success_calls_storage_cleanup_and_auth_delete() -> async def test_delete_account_raises_profile_delete_failed_on_storage_cleanup_error() -> ( None ): - user = CurrentUser(id=uuid4(), email="test@example.com") + user = CurrentUser(id=uuid4(), email=None) class _FailingStorage(_FakeStorage): async def delete_prefix(self, *, bucket: str, prefix: str) -> int: @@ -72,7 +72,7 @@ async def test_delete_account_raises_profile_delete_failed_on_storage_cleanup_er async def test_delete_account_raises_profile_delete_failed_on_auth_delete_error() -> ( None ): - user = CurrentUser(id=uuid4(), email="test@example.com") + user = CurrentUser(id=uuid4(), email=None) class _FailingStorage(_FakeStorage): async def delete_auth_user(self, *, user_id: str) -> None: diff --git a/docs/protocols/common/user-points-chat-data-protocol.md b/docs/protocols/common/user-points-chat-data-protocol.md index 90cd557..abbf5df 100644 --- a/docs/protocols/common/user-points-chat-data-protocol.md +++ b/docs/protocols/common/user-points-chat-data-protocol.md @@ -4,7 +4,7 @@ This protocol defines the canonical data contract for user profile, points accou Protocol verification status: -- Last audited migration: `backend/alembic/versions/20260411_0003_points_audit_and_register_bonus_claims.py` +- Last audited migration: `backend/alembic/versions/20260413_0004_register_bonus_claims_snapshot.py` - Last audited models: `backend/src/models/profile.py`, `backend/src/models/user_points.py`, `backend/src/models/points_ledger.py`, `backend/src/models/points_audit_ledger.py`, `backend/src/models/register_bonus_claims.py`, `backend/src/models/agent_chat_session.py`, `backend/src/models/agent_chat_message.py` - Current status: aligned with register bonus moved to application service @@ -95,13 +95,14 @@ Protocol verification status: ### register_bonus_claims - PK: `id` -- Core fields: `email_hash`, `user_email_snapshot`, `first_user_id`, `grant_event_id`, `created_at`, `updated_at` +- Core fields: `email_hash`, `user_email_snapshot`, `first_user_id_snapshot`, `balance_snapshot`, `grant_event_id`, `created_at`, `updated_at` - Constraints: - `email_hash` unique - `grant_event_id` unique - Notes: - `email_hash` must be HMAC-SHA256 over normalized email (`trim + lower`) - key source: backend config `points_policy.register_bonus_hmac_key` + - `balance_snapshot` stores the latest pre-delete account balance for same-email re-registration recovery #### points_ledger.metadata (schema_version=1) @@ -145,7 +146,7 @@ JSON constraints: - Function: `public.initialize_profile_and_invite_code_on_signup()` - Side effects: profile init + invite code init - Application service (in `POST /auth/email-session`): - - `grant_register_bonus_if_eligible()` grants register bonus via `register_bonus_claims` ledger + - `grant_register_bonus_if_eligible()` restores `balance_snapshot` first when present; otherwise grants register bonus via `register_bonus_claims` - Bonus amount from `config.points_policy.register_bonus_points` ### sessions