diff --git a/backend/handlers/scheduling/time-blockers.go b/backend/handlers/scheduling/time-blockers.go index 50c97d4..025c9cf 100644 --- a/backend/handlers/scheduling/time-blockers.go +++ b/backend/handlers/scheduling/time-blockers.go @@ -2,6 +2,7 @@ package scheduling import ( "context" + "database/sql" "encoding/json" "errors" "fmt" @@ -12,6 +13,8 @@ import ( "crussell/clock" "crussell/db" + "crussell/handlers/payments" + "crussell/internal/square" "crussell/internal/validators" "crussell/mw" @@ -413,6 +416,80 @@ func CleanupOldReservations(ctx context.Context) (int, error) { // Financial records (bookings, payments) remain intact — only PII is scrubbed. // Active/pending bookings are excluded so the salon can still contact the guest. func AnonymizeStaleGuestAccounts(ctx context.Context) (int, error) { + // Best-effort: disable stale-guests' saved cards at Square BEFORE the SQL + // below NULLs square_card_id, so those cards can't keep accepting ccof: + // charges after anonymization (GDPR erasure completeness). A Square failure + // is logged and ignored — the local anonymization must never be blocked by + // Square. Card IDs are selected with the same stale-guest predicate the + // users UPDATE uses, and only when a Square client is configured. + if payments.SquareClient != nil { + // Snapshot the stale-guests' saved cards AND their Square customer IDs + // BEFORE the SQL below NULLs square_card_id/square_customer_id, so the + // external Square references are still available for cleanup (GDPR + // erasure completeness: the local scrub must never strand PII at + // Square). Best-effort: a Square failure is logged and ignored — the + // local anonymization must never be blocked by Square. Rows are + // selected with the same stale-guest predicate the users UPDATE uses. + rows, err := db.Conn.Query(ctx, ` + SELECT usc.square_card_id, usc.square_customer_id + FROM user_saved_cards usc + JOIN users u ON u.id = usc.user_id + WHERE u.account_role = 'guest' + AND NOT EXISTS (SELECT 1 FROM bookings WHERE user_id = u.id AND status IN ('pending', 'confirmed')) + AND EXISTS (SELECT 1 FROM bookings WHERE user_id = u.id GROUP BY user_id HAVING MAX(start_time) < NOW() - INTERVAL '6 months') + AND usc.square_card_id IS NOT NULL + `) + if err != nil { + log.Printf("Warning: Failed to query stale-guest saved cards for Square cleanup: %v", err) + } else { + var cardIDs []string + // Distinct non-null customer IDs only: a guest's saved cards share + // one provisioned Square customer, so DeleteCustomer runs once per + // customer. NULL customer IDs (guests with no provisioned Square + // customer) are skipped. + customerSeen := map[string]bool{} + var customerIDs []string + for rows.Next() { + var cardID, customerID sql.NullString + if err := rows.Scan(&cardID, &customerID); err != nil { + log.Printf("Warning: Failed to scan stale-guest saved card: %v", err) + continue + } + if cardID.Valid && cardID.String != "" { + cardIDs = append(cardIDs, cardID.String) + } + if customerID.Valid && customerID.String != "" && !customerSeen[customerID.String] { + customerSeen[customerID.String] = true + customerIDs = append(customerIDs, customerID.String) + } + } + rows.Close() + if err := rows.Err(); err != nil { + log.Printf("Warning: Row iteration error querying stale-guest saved cards: %v", err) + } + for _, cardID := range cardIDs { + if err := payments.SquareClient.DeleteCardOnFile(ctx, cardID); err != nil { + // TokenPrefix redacts the ccof: card token — the full ID + // must never reach logs. + log.Printf("Warning: Failed to disable stale-guest Square card %s at Square: %v", square.TokenPrefix(cardID), err) + } + } + // GDPR erasure completeness: the guest's Square customer profile + // holds their real name + email PII. Disabling the saved cards and + // NULLing square_customer_id locally is NOT enough — the Square + // customer profile must be deleted too, or the PII persists at + // Square indefinitely after anonymization. Distinct IDs only, so a + // guest with multiple cards on one customer triggers one delete. + for _, customerID := range customerIDs { + if err := payments.SquareClient.DeleteCustomer(ctx, customerID); err != nil { + // TokenPrefix redacts the customer ID — the full ID must + // never reach logs. + log.Printf("Warning: Failed to delete stale-guest Square customer %s at Square: %v", square.TokenPrefix(customerID), err) + } + } + } + } + tx, err := db.Conn.Begin(ctx) if err != nil { return 0, fmt.Errorf("failed to begin transaction: %w", err) diff --git a/backend/handlers/scheduling/time_blockers_test.go b/backend/handlers/scheduling/time_blockers_test.go index 9708f2e..ac62dce 100644 --- a/backend/handlers/scheduling/time_blockers_test.go +++ b/backend/handlers/scheduling/time_blockers_test.go @@ -22,10 +22,13 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "testing" "time" "crussell/clock" + "crussell/handlers/payments" + "crussell/internal/square" "crussell/mw" "crussell/testutils/fixtures" @@ -1322,6 +1325,169 @@ func TestAnonymizeStaleGuestAccounts_ScrubsSavedCards(t *testing.T) { } } +// recordingDisableClient records every DeleteCardOnFile and DeleteCustomer call +// so tests can assert which cards/customers the anonymization cleaned up at +// Square. +type recordingDisableClient struct { + square.SquareClient + mu sync.Mutex + disabled []string + deletedCustomers []string +} + +func (c *recordingDisableClient) DeleteCardOnFile(ctx context.Context, cardID string) error { + c.mu.Lock() + c.disabled = append(c.disabled, cardID) + c.mu.Unlock() + return nil +} + +func (c *recordingDisableClient) DeleteCustomer(ctx context.Context, customerID string) error { + c.mu.Lock() + c.deletedCustomers = append(c.deletedCustomers, customerID) + c.mu.Unlock() + return nil +} + +func (c *recordingDisableClient) disabledIDs() []string { + c.mu.Lock() + defer c.mu.Unlock() + return append([]string(nil), c.disabled...) +} + +func (c *recordingDisableClient) deletedCustomerIDs() []string { + c.mu.Lock() + defer c.mu.Unlock() + return append([]string(nil), c.deletedCustomers...) +} + +// TestAnonymizeStaleGuestAccounts_DisablesCardsAtSquare verifies the +// best-effort Square disable: stale-guests' saved cards are disabled at Square +// BEFORE square_card_id is NULLed locally (GDPR erasure completeness). +// Deliberately NOT t.Parallel: it swaps the package-level payments.SquareClient +// and must not overlap the parallel Anonymize tests that read it. +func TestAnonymizeStaleGuestAccounts_DisablesCardsAtSquare(t *testing.T) { + ctx, tx := resetTestData(t) + + guestID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create guest user: %v", err) + } + if _, err := tx.Exec(ctx, `UPDATE users SET account_role = 'guest' WHERE id = $1`, guestID); err != nil { + t.Fatalf("failed to set guest role: %v", err) + } + if _, err := tx.Exec(ctx, ` + INSERT INTO bookings (user_id, start_time, status, deposit_required) + VALUES ($1, NOW() - INTERVAL '7 months', 'completed', false) + `, guestID); err != nil { + t.Fatalf("failed to create stale booking: %v", err) + } + if _, err := tx.Exec(ctx, ` + INSERT INTO user_saved_cards (user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default) + VALUES ($1, 'ccof:stale_card_123', 'cus_stale_123', 'Visa', '4242', 12, 2030, 'fp1', true) + `, guestID); err != nil { + t.Fatalf("failed to insert saved card: %v", err) + } + + origSquare := payments.SquareClient + rec := &recordingDisableClient{} + payments.SquareClient = rec + defer func() { payments.SquareClient = origSquare }() + + if _, err := AnonymizeStaleGuestAccounts(ctx); err != nil { + t.Fatalf("AnonymizeStaleGuestAccounts failed: %v", err) + } + + got := rec.disabledIDs() + if len(got) != 1 || got[0] != "ccof:stale_card_123" { + t.Fatalf("expected the stale guest's card to be disabled at Square, got %v", got) + } +} + +// TestAnonymizeStaleGuestAccounts_DeletesCustomersAtSquare verifies the +// best-effort Square customer deletion: stale-guests' Square customer profiles +// (real name + email PII) are deleted at Square once per DISTINCT customer ID +// BEFORE square_customer_id is NULLed locally (GDPR erasure completeness). +// Deliberately NOT t.Parallel: it swaps the package-level payments.SquareClient +// and must not overlap the parallel Anonymize tests that read it. +func TestAnonymizeStaleGuestAccounts_DeletesCustomersAtSquare(t *testing.T) { + ctx, tx := resetTestData(t) + + // Guest A: two saved cards sharing one provisioned Square customer. + guestAID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create guest A: %v", err) + } + if _, err := tx.Exec(ctx, `UPDATE users SET account_role = 'guest' WHERE id = $1`, guestAID); err != nil { + t.Fatalf("failed to set guest A role: %v", err) + } + + // Guest B: one saved card on a different Square customer. + guestBID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create guest B: %v", err) + } + if _, err := tx.Exec(ctx, `UPDATE users SET account_role = 'guest' WHERE id = $1`, guestBID); err != nil { + t.Fatalf("failed to set guest B role: %v", err) + } + + // Stale bookings (> 6 months) for both guests. + for _, uid := range []string{guestAID, guestBID} { + if _, err := tx.Exec(ctx, ` + INSERT INTO bookings (user_id, start_time, status, deposit_required) + VALUES ($1, NOW() - INTERVAL '7 months', 'completed', false) + `, uid); err != nil { + t.Fatalf("failed to create stale booking: %v", err) + } + } + + // Guest A's two cards share 'cus_stale_a'; guest B's card is 'cus_stale_b'. + if _, err := tx.Exec(ctx, ` + INSERT INTO user_saved_cards (user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default) + VALUES ($1, 'ccof:stale_card_a1', 'cus_stale_a', 'Visa', '4242', 12, 2030, 'fp_a1', true), + ($1, 'ccof:stale_card_a2', 'cus_stale_a', 'Mastercard', '1111', 12, 2030, 'fp_a2', false) + `, guestAID); err != nil { + t.Fatalf("failed to insert guest A saved cards: %v", err) + } + if _, err := tx.Exec(ctx, ` + INSERT INTO user_saved_cards (user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default) + VALUES ($1, 'ccof:stale_card_b1', 'cus_stale_b', 'Visa', '9999', 12, 2030, 'fp_b1', true) + `, guestBID); err != nil { + t.Fatalf("failed to insert guest B saved card: %v", err) + } + + origSquare := payments.SquareClient + rec := &recordingDisableClient{} + payments.SquareClient = rec + defer func() { payments.SquareClient = origSquare }() + + if _, err := AnonymizeStaleGuestAccounts(ctx); err != nil { + t.Fatalf("AnonymizeStaleGuestAccounts failed: %v", err) + } + + got := rec.deletedCustomerIDs() + if len(got) != 2 { + t.Fatalf("expected DeleteCustomer to run once per DISTINCT customer id (2), got %v", got) + } + gotSet := map[string]bool{} + for _, id := range got { + if gotSet[id] { + t.Errorf("DeleteCustomer called more than once for customer %q: %v", id, got) + } + gotSet[id] = true + } + if !gotSet["cus_stale_a"] || !gotSet["cus_stale_b"] { + t.Errorf("expected both distinct Square customer ids to be deleted, got %v", got) + } + + // The customer delete runs after the card-disable loop, so all cards were + // disabled too. + disabled := rec.disabledIDs() + if len(disabled) != 3 { + t.Errorf("expected all 3 stale cards to be disabled, got %v", disabled) + } +} + // --- Tests for CleanupExpiredFinancialRecords --- // TestCleanupExpiredFinancialRecords_PaymentOlderThan7Years verifies that a diff --git a/backend/handlers/user/account.go b/backend/handlers/user/account.go index d91f941..1f6e305 100644 --- a/backend/handlers/user/account.go +++ b/backend/handlers/user/account.go @@ -15,6 +15,7 @@ import ( "crussell/handlers/payments" "crussell/internal/dav" "crussell/internal/s3" + "crussell/internal/square" "crussell/mw" "github.com/jackc/pgx/v5" @@ -69,38 +70,45 @@ func DeleteAccountHandler(w http.ResponseWriter, r *http.Request) { }(profilePicURL.String) } - if payments.SquareClient != nil { - // #nosec G118 — intentional background goroutine for async account deletion - go func() { - defer func() { - if r := recover(); r != nil { - log.Printf("Panic recovered in Square saved card cleanup: %v", r) - } - }() - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - rows, err := db.Conn.Query(ctx, - `SELECT square_card_id FROM user_saved_cards WHERE user_id = $1 AND deleted_at IS NULL AND square_card_id IS NOT NULL`, userID) - if err != nil { - log.Printf("Warning: Failed to query saved cards for user %s: %v", userID, err) - return - } - defer rows.Close() + // Snapshot the Square card IDs AND customer IDs synchronously BEFORE the + // SQL anonymization below NULLs square_card_id/square_customer_id, so the + // background cleanup still has the external Square references it needs + // (previously the goroutine read the rows itself, racing the anonymize + // step which wiped them mid-flight). Distinct non-null customer IDs only: + // a user's saved cards share one provisioned Square customer, so + // DeleteCustomer runs once per customer. NULL customer IDs (users with + // no saved cards) are skipped. + var cardIDs []string + customerSeen := map[string]bool{} + var customerIDs []string + // Capture the client synchronously so the async cleanup goroutine never + // reads the global payments.SquareClient (which tests swap per-account). + sqClient := payments.SquareClient + if sqClient != nil { + rows, err := db.Conn.Query(r.Context(), + `SELECT square_card_id, square_customer_id FROM user_saved_cards WHERE user_id = $1 AND deleted_at IS NULL`, userID) + if err != nil { + log.Printf("Warning: Failed to query saved cards for user %s: %v", userID, err) + } else { for rows.Next() { - var cardID string - if err := rows.Scan(&cardID); err != nil { + var cardID, customerID sql.NullString + if err := rows.Scan(&cardID, &customerID); err != nil { log.Printf("Warning: Failed to scan card ID for user %s: %v", userID, err) continue } - if err := payments.SquareClient.DeleteCardOnFile(ctx, cardID); err != nil { - log.Printf("Warning: Failed to delete Square card %s for user %s: %v", cardID, userID, err) + if cardID.Valid && cardID.String != "" { + cardIDs = append(cardIDs, cardID.String) + } + if customerID.Valid && customerID.String != "" && !customerSeen[customerID.String] { + customerSeen[customerID.String] = true + customerIDs = append(customerIDs, customerID.String) } } if err := rows.Err(); err != nil { log.Printf("Warning: Row iteration error for user %s: %v", userID, err) - return } - }() + rows.Close() + } } // --- SQL-level anonymization/deletion --- @@ -159,6 +167,52 @@ func DeleteAccountHandler(w http.ResponseWriter, r *http.Request) { // TODO: Create 'user_anonymized' notification for admin audit trail } + // external Square cleanup fires only after local anonymization/deletion + // commits, so a failed local tx leaves external state intact for retry. + if sqClient != nil && (len(cardIDs) > 0 || len(customerIDs) > 0) { + // #nosec G118 — intentional background goroutine for async account deletion + go func(client square.SquareClient, cards, customers []string) { + defer func() { + if r := recover(); r != nil { + log.Printf("Panic recovered in Square cleanup: %v", r) + } + }() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + for _, cardID := range cards { + if err := client.DeleteCardOnFile(ctx, cardID); err != nil { + // TokenPrefix redacts the ccof: card token — the full ID must + // never reach logs. + log.Printf("Warning: Failed to delete Square card %s for user %s: %v", square.TokenPrefix(cardID), userID, err) + } + } + for _, customerID := range customers { + // Square customers are provisioned per-user from a deterministic + // email-derived key, but the UNIQUE(email) index excludes guest + // accounts — so a guest and a registered user sharing an email can + // end up on the SAME Square customer profile (Square dedups within + // its idempotency window). Deleting it would break the other + // account's saved-card charges, so skip the deletion when any OTHER + // non-deleted saved card still references the customer. + var stillReferenced bool + if err := db.Conn.QueryRow(ctx, ` + SELECT EXISTS(SELECT 1 FROM user_saved_cards WHERE square_customer_id = $1 AND user_id <> $2 AND deleted_at IS NULL) + `, customerID, userID).Scan(&stillReferenced); err != nil { + log.Printf("Warning: Failed to check Square customer %s references before deletion: %v", square.TokenPrefix(customerID), err) + continue + } + if stillReferenced { + // PII-redacted customer id — the full id never reaches logs. + log.Printf("Warning: skipping Square customer deletion — customer %s still referenced by another account", square.TokenPrefix(customerID)) + continue + } + if err := client.DeleteCustomer(ctx, customerID); err != nil { + log.Printf("Warning: Failed to delete Square customer %s for user %s: %v", square.TokenPrefix(customerID), userID, err) + } + } + }(sqClient, cardIDs, customerIDs) + } + // Delete CardDAV contact (non-blocking, best-effort) if dav.Service != nil { go func() { diff --git a/backend/handlers/user/user_coverage_test.go b/backend/handlers/user/user_coverage_test.go index a0f3f9a..3d1995e 100644 --- a/backend/handlers/user/user_coverage_test.go +++ b/backend/handlers/user/user_coverage_test.go @@ -14,9 +14,15 @@ package user import ( "bytes" "context" + "database/sql" "encoding/json" + "fmt" + "log" "net/http" "net/http/httptest" + "os" + "strings" + "sync" "testing" "time" @@ -24,6 +30,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "crussell/db" "crussell/handlers/payments" "crussell/internal/square" "crussell/mw" @@ -594,3 +601,277 @@ func TestDeleteAccount_WithSquareClient(t *testing.T) { time.Sleep(50 * time.Millisecond) } +// ============================================================================= +// DeleteAccountHandler — Square ccof redaction + customer deletion +// ============================================================================= + +// syncBuffer is a mutex-guarded log/slog writer so logs written by background +// goroutines can be read safely under -race. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *syncBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *syncBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +// recordingSquareClient records Square erasure calls made by the account +// deletion goroutine so tests can assert what was (and wasn't) called. +type recordingSquareClient struct { + square.SquareClient + mu sync.Mutex + deletedCards []string + deletedCustomers []string +} + +func (c *recordingSquareClient) DeleteCardOnFile(ctx context.Context, cardID string) error { + c.mu.Lock() + c.deletedCards = append(c.deletedCards, cardID) + c.mu.Unlock() + // Token-free error so the log-redaction test isolates redaction of the + // cardID argument rather than the error string. + return fmt.Errorf("square: network error disabling card at Square") +} + +func (c *recordingSquareClient) DeleteCustomer(ctx context.Context, customerID string) error { + c.mu.Lock() + c.deletedCustomers = append(c.deletedCustomers, customerID) + c.mu.Unlock() + return nil +} + +func (c *recordingSquareClient) cardDeletes() []string { + c.mu.Lock() + defer c.mu.Unlock() + return append([]string(nil), c.deletedCards...) +} + +func (c *recordingSquareClient) customerDeletes() []string { + c.mu.Lock() + defer c.mu.Unlock() + return append([]string(nil), c.deletedCustomers...) +} + +// TestDeleteAccount_LogsRedactCardTokens verifies the account-deletion +// goroutine logs a redacted tokenPrefix form of ccof: card IDs, never the full +// token (SECURITY: full ccof tokens must not reach server logs). +func TestDeleteAccount_LogsRedactCardTokens(t *testing.T) { + savedSquareClient := payments.SquareClient + rec := &recordingSquareClient{SquareClient: square.NewDevClient()} + payments.SquareClient = rec + t.Cleanup(func() { payments.SquareClient = savedSquareClient }) + + var sb syncBuffer + log.SetOutput(&sb) + defer log.SetOutput(os.Stderr) + + ctx, tx := testutils.SetupTestTx(t) + userID, err := fixtures.CreateTestUser(tx) + require.NoError(t, err) + + fullToken := "ccof:secret_token_abc123" + _, err = tx.Exec(ctx, ` + INSERT INTO user_saved_cards (user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default) + VALUES ($1, $2, 'cus_secret_abc123', 'Visa', '4242', 12, 2030, 'fp1', true) + `, userID, fullToken) + require.NoError(t, err) + + handler := http.HandlerFunc(DeleteAccountHandler) + req := httptest.NewRequest(http.MethodDelete, "/api/user/account", nil) + req = req.WithContext(context.WithValue(ctx, mw.UserIDKey, userID)) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + require.Equal(t, http.StatusNoContent, w.Code) + + require.Eventually(t, func() bool { + return len(rec.cardDeletes()) > 0 + }, 5*time.Second, 10*time.Millisecond, "expected the Square cleanup goroutine to attempt card deletion") + + // The card-deletion log is written by the async goroutine AFTER the call + // returns — wait for the redacted prefix to appear instead of racing the + // goroutine (the cleanup now dispatches after the local tx commits). + require.Eventually(t, func() bool { + return strings.Contains(sb.String(), "ccof:sec...") + }, 5*time.Second, 10*time.Millisecond, "expected the redacted token prefix to reach the logs") + + logs := sb.String() + if strings.Contains(logs, fullToken) { + t.Errorf("full ccof token %q leaked into logs: %q", fullToken, logs) + } +} + +// TestDeleteAccount_DeletesSquareCustomerOnce verifies the Square customer +// profile (email/name PII) is deleted exactly once even when multiple saved +// cards share the same square_customer_id. +func TestDeleteAccount_DeletesSquareCustomerOnce(t *testing.T) { + savedSquareClient := payments.SquareClient + rec := &recordingSquareClient{SquareClient: square.NewDevClient()} + payments.SquareClient = rec + t.Cleanup(func() { payments.SquareClient = savedSquareClient }) + + ctx, tx := testutils.SetupTestTx(t) + userID, err := fixtures.CreateTestUser(tx) + require.NoError(t, err) + + for _, cardTok := range []string{"ccof:card_one_123", "ccof:card_two_456"} { + _, err := tx.Exec(ctx, ` + INSERT INTO user_saved_cards (user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default) + VALUES ($1, $2, 'cus_shared_123', 'Visa', '4242', 12, 2030, 'fp1', true) + `, userID, cardTok) + require.NoError(t, err) + } + + handler := http.HandlerFunc(DeleteAccountHandler) + req := httptest.NewRequest(http.MethodDelete, "/api/user/account", nil) + req = req.WithContext(context.WithValue(ctx, mw.UserIDKey, userID)) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + require.Equal(t, http.StatusNoContent, w.Code) + + require.Eventually(t, func() bool { + return len(rec.customerDeletes()) > 0 + }, 5*time.Second, 10*time.Millisecond, "expected DeleteCustomer to be called") + + require.Equal(t, []string{"cus_shared_123"}, rec.customerDeletes(), "a shared Square customer must be deleted exactly once") + require.ElementsMatch(t, []string{"ccof:card_one_123", "ccof:card_two_456"}, rec.cardDeletes(), "both saved cards must be disabled at Square") +} + +// ============================================================================= +// DeleteAccountHandler — Square cleanup only after local tx commit +// ============================================================================= + +// TestDeleteAccount_SquareCleanupNotDispatchedOnTxFailure verifies the fix that +// dispatches the Square cleanup goroutine only AFTER the local +// anonymization/deletion transaction commits: when the local tx fails (here an +// admin_notifications row references the guest with a RESTRICT FK, so +// delete_guest_user's DELETE FROM users errors), DeleteCardOnFile/DeleteCustomer +// must NOT be called — a failed local tx leaves external state intact for retry. +func TestDeleteAccount_SquareCleanupNotDispatchedOnTxFailure(t *testing.T) { + savedSquareClient := payments.SquareClient + rec := &recordingSquareClient{SquareClient: square.NewDevClient()} + payments.SquareClient = rec + t.Cleanup(func() { payments.SquareClient = savedSquareClient }) + + ctx, tx := testutils.SetupTestTx(t) + + guestID, err := fixtures.CreateTestGuestUser(tx) + require.NoError(t, err) + + // Saved card so the pre-tx snapshot captures a card/customer to scrub — if + // the goroutine were (wrongly) dispatched before the tx, it would record a + // call here. + _, err = tx.Exec(ctx, ` + INSERT INTO user_saved_cards (user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default) + VALUES ($1, $2, 'cus_fail_tx_123', 'Visa', '4242', 12, 2030, 'fp1', true) + `, guestID, "ccof:card_fail_tx") + require.NoError(t, err) + + // admin_notifications.user_id has a RESTRICT FK on users(id): seeding a row + // makes delete_guest_user's DELETE FROM users fail, so the local tx errors. + _, err = tx.Exec(ctx, ` + INSERT INTO admin_notifications (reason, user_id) + VALUES ('pending_booking', $1) + `, guestID) + require.NoError(t, err) + + handler := http.HandlerFunc(DeleteAccountHandler) + req := httptest.NewRequest(http.MethodDelete, "/api/user/account", nil) + req = req.WithContext(context.WithValue(ctx, mw.UserIDKey, guestID)) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + + require.Equal(t, http.StatusInternalServerError, w.Code, "the failed local tx must surface a 500") + + // Give a (wrongly dispatched) cleanup goroutine time to record its calls. + time.Sleep(100 * time.Millisecond) + require.Equal(t, 0, len(rec.cardDeletes()), "DeleteCardOnFile must NOT be called when the local tx failed") + require.Equal(t, 0, len(rec.customerDeletes()), "DeleteCustomer must NOT be called when the local tx failed") +} + +// ============================================================================= +// DeleteAccountHandler — shared Square customer edge (cross-user email dedup) +// ============================================================================= + +// TestDeleteAccount_SkipsSharedSquareCustomer verifies the fix that guards +// DeleteCustomer behind a "still referenced by another account" check: Square +// customers are provisioned per-user from a deterministic email-derived key, +// but the UNIQUE(email) index excludes guest accounts, so a guest and a +// registered user sharing an email can land on the SAME Square customer profile +// (Square dedups within its idempotency window). Deleting one account must NOT +// delete the shared profile — the other account's saved-card charges would +// break. The other user's saved-card row must be left untouched. +func TestDeleteAccount_SkipsSharedSquareCustomer(t *testing.T) { + savedSquareClient := payments.SquareClient + rec := &recordingSquareClient{SquareClient: square.NewDevClient()} + payments.SquareClient = rec + t.Cleanup(func() { payments.SquareClient = savedSquareClient }) + + ctx, tx := testutils.SetupTestTx(t) + + userA, err := fixtures.CreateTestUser(tx) + require.NoError(t, err) + userB, err := fixtures.CreateTestUser(tx) + require.NoError(t, err) + + for _, tc := range []struct{ userID, cardID string }{ + {userA, "ccof:card_user_a"}, + {userB, "ccof:card_user_b"}, + } { + _, err := tx.Exec(ctx, ` + INSERT INTO user_saved_cards (user_id, square_card_id, square_customer_id, brand, last_4, exp_month, exp_year, fingerprint, is_default) + VALUES ($1, $2, 'cus_shared_cross_user', 'Visa', '4242', 12, 2030, 'fp1', true) + `, tc.userID, tc.cardID) + require.NoError(t, err) + } + + // Commit the setup so the cleanup goroutine's pool-level reference check + // (background context, outside the per-test tx) can see user B's row. + pgxTx := db.TxFromContext(ctx) + require.NotNil(t, pgxTx, "no transaction in context") + require.NoError(t, pgxTx.Commit(ctx)) + + // The committed rows live in the SHARED test pool — clean them up. + t.Cleanup(func() { + _, _ = db.Conn.Exec(context.Background(), `DELETE FROM user_saved_cards WHERE square_customer_id = 'cus_shared_cross_user'`) + _, _ = db.Conn.Exec(context.Background(), `DELETE FROM users WHERE id IN ($1, $2)`, userA, userB) + }) + + handler := http.HandlerFunc(DeleteAccountHandler) + req := httptest.NewRequest(http.MethodDelete, "/api/user/account", nil) + req = req.WithContext(context.WithValue(context.Background(), mw.UserIDKey, userA)) + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + require.Equal(t, http.StatusNoContent, w.Code) + + // The goroutine runs the card loop first — wait for it to record user A's + // card deletion so we know the cleanup ran, then assert the shared customer + // was NOT deleted. + require.Eventually(t, func() bool { + return len(rec.cardDeletes()) > 0 + }, 5*time.Second, 10*time.Millisecond, "expected the Square cleanup goroutine to attempt card deletion") + + require.Never(t, func() bool { + return len(rec.customerDeletes()) > 0 + }, 500*time.Millisecond, 10*time.Millisecond, "DeleteCustomer must NOT be called for a Square customer still referenced by another account") + + // User B's saved card is untouched (still references the shared customer). + var bUserID string + var bCustomerID sql.NullString + var bDeletedAt sql.NullTime + err = db.Conn.QueryRow(context.Background(), ` + SELECT user_id, square_customer_id, deleted_at FROM user_saved_cards WHERE square_card_id = 'ccof:card_user_b' + `).Scan(&bUserID, &bCustomerID, &bDeletedAt) + require.NoError(t, err) + require.Equal(t, userB, bUserID) + require.True(t, bCustomerID.Valid && bCustomerID.String == "cus_shared_cross_user", "user B's row must keep the shared square_customer_id") + require.False(t, bDeletedAt.Valid, "user B's row must not be soft-deleted") +}