From 0eefbfd6a436b5913b54dc11fa00d3c75e50d90a Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Fri, 3 Jul 2026 19:32:52 +0200 Subject: [PATCH] fix(account): dedupe colliding identities on merge, journaling to the dossier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An account merge blanket-reassigned all of the secondary's identities to the primary, so merging two accounts that each had a confirmed email (or telegram/vk) left the survivor with two identities of one kind — which the profile and the retention dossier both treat as singular (the profile showed an arbitrary one; a later change-email journaled only one). The merge now keeps the primary's identity and journals the secondary's colliding one to retained_identities (reason=merge) before dropping it, so the survivor has one identity per kind and the absorbed credential still lands in the legal dossier. - migration 00008: widen retained_identities.reason CHECK to admit 'merge' (expand-contract — Up only widens the set, so an image rollback stays DB-safe). - accountmerge: dedupeIdentities + retainMergedIdentity before the identity reassign. - inttest TestAccountMergeDedupesEmail; docs ARCHITECTURE §4 + retention.go reason note. --- backend/internal/account/retention.go | 8 +- backend/internal/accountmerge/merge.go | 79 +++++++++++++++++++ backend/internal/inttest/merge_test.go | 38 +++++++++ ...00008_retained_identities_merge_reason.sql | 22 ++++++ docs/ARCHITECTURE.md | 7 +- 5 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 backend/internal/postgres/migrations/00008_retained_identities_merge_reason.sql diff --git a/backend/internal/account/retention.go b/backend/internal/account/retention.go index 14c6bab..b6f161e 100644 --- a/backend/internal/account/retention.go +++ b/backend/internal/account/retention.go @@ -21,9 +21,11 @@ import ( const RetentionTTL = 2 * 365 * 24 * time.Hour // Reasons recorded on a retained_identities row: what detached the credential from its -// account. The row is written just before the live identities row is removed, preserving -// the legal dossier (which email/vk/tg was linked, and when) even as the identity frees -// for reuse. See docs/ARCHITECTURE.md §9.1. +// account (unlink / email change / account deletion here; an account merge that drops a +// same-kind colliding identity writes reason "merge" from the accountmerge package). The +// row is written just before the live identities row is removed, preserving the legal +// dossier (which email/vk/tg was linked, and when) even as the identity frees for reuse. +// See docs/ARCHITECTURE.md §9.1. const ( retainUnlink = "unlink" retainChange = "change" diff --git a/backend/internal/accountmerge/merge.go b/backend/internal/accountmerge/merge.go index 216e373..d30d089 100644 --- a/backend/internal/accountmerge/merge.go +++ b/backend/internal/accountmerge/merge.go @@ -27,6 +27,11 @@ import ( // without taking a dependency on the game package. const statusActive = "active" +// retainReasonMerge is the retained_identities.reason for a credential dropped by a merge +// collision (both accounts held the same kind). It mirrors the account package's retain +// reasons, kept local to avoid importing that package's unexported constants. +const retainReasonMerge = "merge" + // Friendship statuses, highest precedence first, mirroring internal/social. const ( friendAccepted = "accepted" @@ -75,6 +80,9 @@ func (m *Merger) Merge(ctx context.Context, primary, secondary uuid.UUID) error if err := mergeAccountFields(ctx, tx, primary, secondary, now); err != nil { return err } + if err := dedupeIdentities(ctx, tx, primary, secondary); err != nil { + return err + } if err := reassignColumn(ctx, tx, table.Identities, table.Identities.AccountID, primary, secondary); err != nil { return fmt.Errorf("accountmerge: identities: %w", err) } @@ -300,6 +308,77 @@ func reassignColumn(ctx context.Context, tx *sql.Tx, tbl postgres.Table, col pos return err } +// dedupeIdentities resolves a same-kind identity collision before the blanket identity +// reassign: when both accounts already hold an identity of the same kind (e.g. each has a +// confirmed email — reachable when two email-bearing accounts merge), the primary keeps +// its own and the secondary's is journaled to retained_identities (reason=merge) and +// removed. Without this the blanket reassign would leave the survivor with two identities +// of one kind (there is no per-account-kind unique on identities), which the profile and +// the retention dossier both treat as singular. Non-colliding identities are untouched and +// move with the blanket reassign. +func dedupeIdentities(ctx context.Context, tx *sql.Tx, primary, secondary uuid.UUID) error { + var prows []model.Identities + if err := postgres.SELECT(table.Identities.Kind). + FROM(table.Identities). + WHERE(table.Identities.AccountID.EQ(postgres.UUID(primary))). + QueryContext(ctx, tx, &prows); err != nil && !errors.Is(err, qrm.ErrNoRows) { + return fmt.Errorf("accountmerge: primary identity kinds: %w", err) + } + occupied := make(map[string]struct{}, len(prows)) + for _, r := range prows { + occupied[r.Kind] = struct{}{} + } + if len(occupied) == 0 { + return nil + } + var srows []model.Identities + if err := postgres.SELECT( + table.Identities.Kind, table.Identities.ExternalID, + table.Identities.Confirmed, table.Identities.CreatedAt, + ).FROM(table.Identities). + WHERE(table.Identities.AccountID.EQ(postgres.UUID(secondary))). + QueryContext(ctx, tx, &srows); err != nil && !errors.Is(err, qrm.ErrNoRows) { + return fmt.Errorf("accountmerge: secondary identities: %w", err) + } + for _, s := range srows { + if _, dup := occupied[s.Kind]; !dup { + continue + } + if err := retainMergedIdentity(ctx, tx, secondary, s); err != nil { + return err + } + del := table.Identities.DELETE().WHERE( + table.Identities.AccountID.EQ(postgres.UUID(secondary)). + AND(table.Identities.Kind.EQ(postgres.String(s.Kind))). + AND(table.Identities.ExternalID.EQ(postgres.String(s.ExternalID))), + ) + if _, err := del.ExecContext(ctx, tx); err != nil { + return fmt.Errorf("accountmerge: drop colliding %s identity: %w", s.Kind, err) + } + } + return nil +} + +// retainMergedIdentity appends a retained_identities row for a secondary identity dropped +// by a merge collision (reason=merge), preserving it in the legal dossier. It mirrors +// account.retainIdentityTx, which is unexported; detached_at falls to the column default. +func retainMergedIdentity(ctx context.Context, tx *sql.Tx, accountID uuid.UUID, id model.Identities) error { + rid, err := uuid.NewV7() + if err != nil { + return fmt.Errorf("accountmerge: new retained id: %w", err) + } + ins := table.RetainedIdentities.INSERT( + table.RetainedIdentities.RetainedID, table.RetainedIdentities.AccountID, + table.RetainedIdentities.Kind, table.RetainedIdentities.ExternalID, + table.RetainedIdentities.Confirmed, table.RetainedIdentities.LinkedAt, + table.RetainedIdentities.Reason, + ).VALUES(rid, accountID, id.Kind, id.ExternalID, id.Confirmed, id.CreatedAt, retainReasonMerge) + if _, err := ins.ExecContext(ctx, tx); err != nil { + return fmt.Errorf("accountmerge: retain merged %s identity: %w", id.Kind, err) + } + return nil +} + // friendRank ranks a friendship status for dedupe precedence (higher wins). func friendRank(status string) int { switch status { diff --git a/backend/internal/inttest/merge_test.go b/backend/internal/inttest/merge_test.go index 83cda98..7547886 100644 --- a/backend/internal/inttest/merge_test.go +++ b/backend/internal/inttest/merge_test.go @@ -251,6 +251,44 @@ func TestAccountMergeFinishedSharedGameKept(t *testing.T) { } } +// TestAccountMergeDedupesEmail keeps the primary's email when both accounts have one and +// journals the secondary's to the dossier (reason=merge), so the survivor never ends up +// with two email identities. +func TestAccountMergeDedupesEmail(t *testing.T) { + ctx := context.Background() + store := account.NewStore(testDB) + merger := accountmerge.NewMerger(testDB) + + primary := provisionAccount(t) + secondary := provisionAccount(t) + primaryEmail := "keep-" + uuid.NewString() + "@example.com" + secondaryEmail := "absorb-" + uuid.NewString() + "@example.com" + bindEmailIdentity(t, primary, primaryEmail) + bindEmailIdentity(t, secondary, secondaryEmail) + + if err := merger.Merge(ctx, primary, secondary); err != nil { + t.Fatalf("merge: %v", err) + } + + // The primary keeps its own email; the secondary's is gone from the live identities. + if owner, ok, _ := store.AccountIDByIdentity(ctx, account.KindEmail, primaryEmail); !ok || owner != primary { + t.Errorf("primary email owner = %s ok=%v, want primary %s", owner, ok, primary) + } + if _, ok, _ := store.AccountIDByIdentity(ctx, account.KindEmail, secondaryEmail); ok { + t.Error("the secondary's email must be removed (no duplicate email on the survivor)") + } + // The absorbed email stays in the legal dossier, tagged reason=merge. + var reason string + if err := testDB.QueryRowContext(ctx, + `SELECT reason FROM backend.retained_identities WHERE account_id=$1 AND kind='email' AND external_id=$2`, + secondary, secondaryEmail).Scan(&reason); err != nil { + t.Fatalf("retained email row: %v", err) + } + if reason != "merge" { + t.Errorf("retained reason = %q, want merge", reason) + } +} + // TestAccountLinkFreeEmail binds a free email and promotes a guest to durable. func TestAccountLinkFreeEmail(t *testing.T) { ctx := context.Background() diff --git a/backend/internal/postgres/migrations/00008_retained_identities_merge_reason.sql b/backend/internal/postgres/migrations/00008_retained_identities_merge_reason.sql new file mode 100644 index 0000000..b2b9ae2 --- /dev/null +++ b/backend/internal/postgres/migrations/00008_retained_identities_merge_reason.sql @@ -0,0 +1,22 @@ +-- Admit the 'merge' reason into retained_identities. An account merge folds a secondary +-- account into a primary; when both hold an identity of the same kind (e.g. each has a +-- confirmed email), the survivor keeps its own and the secondary's is journaled to the +-- legal dossier and removed — so the survivor never ends up with two identities of one +-- kind, and the absorbed credential is still retained. That journal row carries reason +-- 'merge', alongside the existing unlink / change / delete. +-- +-- Expand-contract: the Up only WIDENS the allowed reason set, so an older backend image +-- (which writes only unlink/change/delete) still satisfies the constraint — a rollback +-- stays DB-safe. The Down narrows it again and would reject pre-existing 'merge' rows, so +-- it is a dev-only convenience, not a production rollback path (image rollback runs old +-- code against this schema, not the Down migration). + +-- +goose Up +ALTER TABLE backend.retained_identities DROP CONSTRAINT retained_identities_reason_chk; +ALTER TABLE backend.retained_identities ADD CONSTRAINT retained_identities_reason_chk + CHECK ((reason = ANY (ARRAY['unlink'::text, 'change'::text, 'delete'::text, 'merge'::text]))); + +-- +goose Down +ALTER TABLE backend.retained_identities DROP CONSTRAINT retained_identities_reason_chk; +ALTER TABLE backend.retained_identities ADD CONSTRAINT retained_identities_reason_chk + CHECK ((reason = ANY (ARRAY['unlink'::text, 'change'::text, 'delete'::text]))); diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2fb8a7b..030165e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -292,8 +292,11 @@ arrive from a platform rather than completing a mandatory registration). invitations / friend-codes / drafts / pending-codes. **Chat, feedback and complaints are kept.** The orchestration a layer up resigns the account's active games (so opponents are not stranded), **drops its all-robot games** (no human opponent; children cascade), and - revokes its sessions. Every credential detachment — **unlink, email change and delete** — - writes a `retained_identities` row (`reason`), so the dossier keeps the full timeline. + revokes its sessions. Every credential detachment — **unlink, email change, delete and a + merge collision** — writes a `retained_identities` row (`reason`), so the dossier keeps the + full timeline. (A merge that would otherwise leave the survivor with two identities of one + kind — e.g. each account held a confirmed email — keeps the primary's and journals the + secondary's with `reason=merge` before dropping it.) Step-up is a mailed code (`purpose=delete`, no deeplink — a stray click must not delete; `ConfirmByToken` refuses a delete token) for an email account, else a typed phrase. `last_login_at` / `last_login_ip` are stamped on the cold-load profile fetch (throttled