Files
Crussell/backend/handlers/payments/concurrency_test.go
T
popertots 7439fa86c1 Fix payment review round 3: saved-card idempotency, stale-pending sweep, webhook fail-closed
R1/R4: saved_card branch in CreateTerminalPayment now mirrors CreateTipPayment
- advisory lock (crussell:payment:<bookingID>) serializes concurrent double-clicks
- deterministic key bookingID-sc-type-amount-cardID (<=45 chars) so a lost-response
  retry derives the same key and dedups instead of double-charging
- idempotency switch inside the lock: completed -> dedup, pending -> reuse with
  pence amount-guard, failed -> clean 409
- success response includes card_brand/card_last4 (frontend already reads them)

R2: add 'failed' case to all four retry switches (tip, booking, gift card, till)
- a swept/definitively-rejected record returns 409 instead of 500-ing on the
  idempotency_key UNIQUE constraint

R3: extend SweepStalePendingPayments to till_sales card rows
- sweeps pending till_sales (online_square/in_person_card) past Square's ~24h
  key retention, closing the double-charge window for till sales
- swept rows logged with the same CRITICAL manual-reconciliation marker as the
  refund sweep

Webhook fail-closed: reject 503 when SQUARE_WEBHOOK_SIGNATURE_KEY unset, 403 on
bad signature (was: skip verification in dev)

Refund status resolution: refunds now resolve by Square status
(COMPLETED/PENDING/FAILED/REJECTED) instead of assuming completed; real error
codes (REFUND_AMOUNT_INVALID, PAYMENT_NOT_REFUNDABLE, REFUND_ALREADY_PENDING)
added to the definitive/processed classification

HTTP client: CreateCard key truncated to <=45 chars, device_options always sent
(env SQUARE_TERMINAL_DEVICE_ID fallback), processing_fee reads amount_money,
ListCards cursor loop, refund keys hashed to <=45 chars

Other fixes: payment/till/gift-card advisory-lock + FOR UPDATE asymmetries,
GetPaymentByID NULL scans, loyalty redemption lock, card upsert on conflict,
mock ccof: prefix parity, IsValidSquareCheckoutID for real Square IDs,
isAdminRequest defense-in-depth on all 6 admin payment handlers, webhook
signature docs, M8/L5 debug markers removed

Docs: README/FC/TM/Overview updated (22 jobs, 20 CRITICAL sites, 23-section
GDPR export, sweep jobs, webhook fail-closed); P11 plan marks remaining items
(sandbox smoke test, M-8 customer_id, saved-card key dedup trade-off) as
deferred with rationale; gap backlog pruned of completed items
2026-08-22 00:34:49 +01:00

426 lines
14 KiB
Go
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
//go:build test && dev
package payments
import (
"context"
"net/http"
"net/http/httptest"
"sync"
"testing"
"time"
"crussell/db"
"crussell/internal/square"
"crussell/mw"
"crussell/testutils"
"crussell/testutils/fixtures"
"crussell/testutils/jwt"
"github.com/go-chi/chi/v5"
)
// slowCreatePaymentClient delays the Square charge so each handler holds its
// advisory lock long enough that a concurrent same-key request would race it
// without the lock (two goroutines both reading "no record", both charging).
type slowCreatePaymentClient struct {
square.SquareClient
delay time.Duration
}
func (c *slowCreatePaymentClient) CreatePayment(ctx context.Context, req square.CreatePaymentReq) (*square.PaymentResult, error) {
time.Sleep(c.delay)
return c.SquareClient.CreatePayment(ctx, req)
}
// cleanupConcurrentTestRows deletes the rows a concurrency test committed at
// pool level. These tests must COMMIT their setup so the advisory locks work
// across independent connections, which leaves committed rows in the shared
// test DB — without cleanup they leak into parallel tests (e.g. GetGiftCards
// counts gift_cards/user_giftcard_balances globally). FK-safe deletion order.
func cleanupConcurrentTestRows(t *testing.T, pool context.Context, userID, bookingID string) {
t.Helper()
t.Cleanup(func() {
var gcIDs []string
rows, err := db.Conn.Query(pool, `SELECT id FROM gift_cards WHERE created_by = $1`, userID)
if err == nil {
for rows.Next() {
var id string
if rows.Scan(&id) == nil {
gcIDs = append(gcIDs, id)
}
}
rows.Close()
}
for _, gcID := range gcIDs {
_, _ = db.Conn.Exec(pool, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, gcID)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM user_giftcard_balances WHERE user_id = $1`, userID)
if bookingID != "" {
_, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM booking_services WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM bookings WHERE id = $1`, bookingID)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE created_by = $1 OR idempotency_key LIKE 'concurrent-%'`, userID)
for _, gcID := range gcIDs {
_, _ = db.Conn.Exec(pool, `DELETE FROM gift_cards WHERE id = $1`, gcID)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM user_saved_cards WHERE user_id = $1`, userID)
_, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, userID)
})
}
// TestBuyGiftCard_ConcurrentSameKey_SingleRecord proves the BuyGiftCard
// advisory lock (giftcards.go): two goroutines POSTing the same idempotency key
// must produce exactly ONE payment record and ONE funded gift card — never
// 2× value for 1 charge. Without the lock, both goroutines pass the
// idempotency check, both reuse/insert pending records, and both fund the card.
func TestBuyGiftCard_ConcurrentSameKey_SingleRecord(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create user: %v", err)
}
token := jwt.GenerateUserToken(userID)
cleanupConcurrentTestRows(t, context.Background(), userID, "")
// Commit the setup so both goroutines operate at pool level — the advisory
// locks only serialize across independent connections, and a per-test tx
// would route both sides through a single shared connection.
innerTx := db.TxFromContext(ctx)
if innerTx == nil {
t.Fatal("no transaction in context")
}
if err := innerTx.Commit(ctx); err != nil {
t.Fatalf("failed to commit setup tx: %v", err)
}
origClient := SquareClient
slow := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond}
SquareClient = slow
defer func() { SquareClient = origClient }()
pool := context.Background()
key := "buy-gc-concurrent-same-key"
reqBody := map[string]interface{}{
"amount": 2000,
"recipient_type": "self",
"new_card_token": "cnon:concurrent-card",
"idempotency_key": key,
}
var wg sync.WaitGroup
startBoth := make(chan struct{})
recs := make([]*httptest.ResponseRecorder, 2)
for i := 0; i < 2; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
<-startBoth
recs[idx] = makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", reqBody, token, pool)
}(i)
}
close(startBoth)
wg.Wait()
// Both requests must succeed — the lock serializes them and the second
// finds the completed record (idempotent dedup, HTTP 200), so neither
// double-charges nor errors.
for i, rec := range recs {
if rec.Code != http.StatusCreated && rec.Code != http.StatusOK {
t.Errorf("request %d expected 201 (create) or 200 (dedup), got %d. body: %s", i, rec.Code, rec.Body.String())
}
}
// Exactly one payment record for this key.
var payCount int
err = db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM payments WHERE idempotency_key = $1`, key).Scan(&payCount)
if err != nil {
t.Fatalf("failed to count payments: %v", err)
}
if payCount != 1 {
t.Errorf("expected exactly 1 payment record, got %d (double-charge!)", payCount)
}
// Exactly one funded gift card for this user's self-purchase.
var gcCount int
err = db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM gift_cards WHERE total_funds_added = 20.00 AND created_by = $1`, userID).Scan(&gcCount)
if err != nil {
t.Fatalf("failed to count gift cards: %v", err)
}
if gcCount != 1 {
t.Errorf("expected exactly 1 funded gift card, got %d (2× value!)", gcCount)
}
// User balance credited exactly once.
var balance float64
err = db.Conn.QueryRow(pool,
`SELECT balance FROM user_giftcard_balances WHERE user_id = $1`, userID).Scan(&balance)
if err != nil {
t.Fatalf("failed to query balance: %v", err)
}
if balance != 20.00 {
t.Errorf("expected balance 20.00, got %.2f (double-credit!)", balance)
}
}
// TestTipPayment_ConcurrentSameKey_SingleRecord proves the tip advisory lock
// (handlers.go): two goroutines POSTing the same booking with the same key must
// produce exactly ONE tip payment record — never two charges for one booking.
func TestTipPayment_ConcurrentSameKey_SingleRecord(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create user: %v", err)
}
serviceID, err := fixtures.CreateTestService(tx)
if err != nil {
t.Fatalf("failed to create service: %v", err)
}
start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)
bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, start)
if err != nil {
t.Fatalf("failed to create booking: %v", err)
}
if _, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed"); err != nil {
t.Fatalf("failed to create prior payment: %v", err)
}
token := jwt.GenerateUserToken(userID)
cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
innerTx := db.TxFromContext(ctx)
if innerTx == nil {
t.Fatal("no transaction in context")
}
if err := innerTx.Commit(ctx); err != nil {
t.Fatalf("failed to commit setup tx: %v", err)
}
origClient := SquareClient
slow := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond}
SquareClient = slow
defer func() { SquareClient = origClient }()
pool := context.Background()
key := "tip-concurrent-same-key"
cardToken := "cnon:concurrent-tip-card"
reqBody := CreateTipPaymentRequest{
Amount: 500,
NewCardToken: &cardToken,
IdempotencyKey: key,
}
var wg sync.WaitGroup
startBoth := make(chan struct{})
recs := make([]*httptest.ResponseRecorder, 2)
for i := 0; i < 2; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
<-startBoth
recs[idx] = makePaymentRequest(CreateTipPayment, "POST", "/api/bookings/"+bookingID+"/tip", reqBody, token, pool)
}(i)
}
close(startBoth)
wg.Wait()
for i, rec := range recs {
if rec.Code != http.StatusOK {
t.Errorf("request %d expected 200, got %d. body: %s", i, rec.Code, rec.Body.String())
}
}
var tipCount int
err = db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_type = 'tip' AND idempotency_key = $2`, bookingID, key).Scan(&tipCount)
if err != nil {
t.Fatalf("failed to count tip payments: %v", err)
}
if tipCount != 1 {
t.Errorf("expected exactly 1 tip payment record, got %d (double-charge!)", tipCount)
}
}
// TestBookingPayment_ConcurrentSameKey_SingleRecord proves the booking-payment
// advisory lock (handlers.go): two goroutines paying the same booking with the
// same key must produce exactly ONE payment record.
func TestBookingPayment_ConcurrentSameKey_SingleRecord(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
token := jwt.GenerateUserToken(userID)
cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
innerTx := db.TxFromContext(ctx)
if innerTx == nil {
t.Fatal("no transaction in context")
}
if err := innerTx.Commit(ctx); err != nil {
t.Fatalf("failed to commit setup tx: %v", err)
}
origClient := SquareClient
slow := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond}
SquareClient = slow
defer func() { SquareClient = origClient }()
pool := context.Background()
key := "booking-pay-concurrent-same-key"
cardToken := "cnon:concurrent-booking-card"
reqBody := CreateBookingPaymentRequest{
Amount: 5000,
PaymentType: "full",
NewCardToken: &cardToken,
IdempotencyKey: key,
}
var wg sync.WaitGroup
startBoth := make(chan struct{})
recs := make([]*httptest.ResponseRecorder, 2)
for i := 0; i < 2; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
<-startBoth
recs[idx] = makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", reqBody, token, pool)
}(i)
}
close(startBoth)
wg.Wait()
for i, rec := range recs {
if rec.Code != http.StatusOK {
t.Errorf("request %d expected 200, got %d. body: %s", i, rec.Code, rec.Body.String())
}
}
var payCount int
err := db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND idempotency_key = $2`, bookingID, key).Scan(&payCount)
if err != nil {
t.Fatalf("failed to count payments: %v", err)
}
if payCount != 1 {
t.Errorf("expected exactly 1 payment record, got %d (double-charge!)", payCount)
}
}
// completedCheckoutClient forces GetCheckout to return a fixed COMPLETED
// payment for any checkout id, deterministically exercising the terminal
// completion dedup+insert path (the mock's real async goroutine would be
// non-deterministic in a race test).
type completedCheckoutClient struct {
square.SquareClient
result *square.PaymentResult
}
func (c *completedCheckoutClient) GetCheckout(ctx context.Context, checkoutID string) (*square.PaymentResult, error) {
return c.result, nil
}
// TestGetCheckoutStatus_ConcurrentPolls_SingleRecord proves the P3 fix: the
// terminal-completion path serializes on a per-payment advisory lock, so two
// concurrent polls of the same completed checkout produce exactly ONE payment
// record. Without the lock, both goroutines pass the dedup SELECT, both INSERT,
// and the second dies on the idempotency_key UNIQUE constraint after the
// customer already paid.
func TestGetCheckoutStatus_ConcurrentPolls_SingleRecord(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create user: %v", err)
}
serviceID, err := fixtures.CreateTestService(tx)
if err != nil {
t.Fatalf("failed to create service: %v", err)
}
start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)
bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, start)
if err != nil {
t.Fatalf("failed to create booking: %v", err)
}
cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
innerTx := db.TxFromContext(ctx)
if innerTx == nil {
t.Fatal("no transaction in context")
}
if err := innerTx.Commit(ctx); err != nil {
t.Fatalf("failed to commit setup tx: %v", err)
}
origClient := SquareClient
SquareClient = &completedCheckoutClient{
SquareClient: square.NewDevClient(),
result: &square.PaymentResult{
ID: "pay_terminal_race",
Status: "COMPLETED",
Amount: 5000,
Fees: 88,
SquarePayID: "pay_terminal_race",
CardBrand: "VISA",
CardLast4: "4242",
ReceiptURL: "https://receipt.example/pay_terminal_race",
EntryMethod: "EMV",
LocationID: "loc",
ReferenceID: bookingID,
CreatedAt: "2026-07-31T00:00:00Z",
UpdatedAt: "2026-07-31T00:00:00Z",
},
}
defer func() { SquareClient = origClient }()
pool := context.Background()
checkoutID := "abcd1234ef56" // 12 hex chars, passes the checkout-id validation
var wg sync.WaitGroup
startBoth := make(chan struct{})
recs := make([]*httptest.ResponseRecorder, 2)
for i := 0; i < 2; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
<-startBoth
req := httptest.NewRequest("GET", "/api/checkout/"+checkoutID+"/status?booking_id="+bookingID, nil)
rctx := chi.NewRouteContext()
rctx.URLParams.Add("checkout_id", checkoutID)
reqCtx := context.WithValue(req.Context(), chi.RouteCtxKey, rctx)
// GetCheckoutStatus is admin-only (defense-in-depth S-1 check).
reqCtx = context.WithValue(reqCtx, mw.UserRoleKey, "admin")
req = req.WithContext(reqCtx)
w := httptest.NewRecorder()
GetCheckoutStatus(w, req)
recs[idx] = w
}(i)
}
close(startBoth)
wg.Wait()
for i, rec := range recs {
if rec.Code != http.StatusOK {
t.Errorf("request %d expected 200, got %d. body: %s", i, rec.Code, rec.Body.String())
}
}
// Exactly one completed terminal payment record for this booking.
var payCount int
err = db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_method = 'in_person_card' AND square_payment_id = $2`,
bookingID, "pay_terminal_race").Scan(&payCount)
if err != nil {
t.Fatalf("failed to count terminal payments: %v", err)
}
if payCount != 1 {
t.Errorf("expected exactly 1 terminal payment record, got %d (double-record race!)", payCount)
}
}