Complete Square GDPR erasure: customer deletion and stale-guest scrub
Account deletion now snapshots card and customer IDs before the local anonymize transaction and dispatches the Square cleanup goroutine only after the tx commits, deleting each distinct Square customer once and skipping any customer still referenced by another user's card. AnonymizeStaleGuestAccounts also disables cards and deletes the guest's Square customer profile (PII) before NULLing references locally. Token redaction applied to all error logs.
This commit is contained in:
@@ -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")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user