popertots/Crussell
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: till saved-card SCA/consent + ownership invariant (F1/F5), money-F4 top-up expiry gate, 2FA-enforced 402 tests

Browse Source 
- till.go: the saved-card till charge carries the C6 consent fields and enforces
  them on the (unreachable) 2FA fallback path; the card ownership SELECT became
  owner-agnostic with the owner read at the gate (F1 — no charge surface can
  act on a card it does not own); a till sale's gift-card creation/top-up now
  runs under the SAME per-admin daily-cap advisory lock as the admin API
  surfaces (F5) so two concurrent distinct sales cannot overshoot the £5,000
  day ceiling; money-F4: an expired gift card can never be topped up (expiry
  gate mirrors RedeemGiftCard's DB-clock comparison) — the top-up would
  otherwise resurrect a card the nightly cleanup already forfeited.
- charge_helpers_test.go: TestResolveChargeSource_SCATokenizeResult_UsesTokenAsSource
  pins the SCA tokenize-result wire contract (token as source, card row for the
  customer).
- errors_test.go: token-less saved-card charges are refused 402
  verification_required under 2FA enforcement (create-payment, gift-card buy +
  save-card), even for a user with 2FA enabled — the homegrown gate can never
  substitute for SCA.
This commit is contained in:
popertots
2026-08-22 00:34:50 +01:00
parent 8ba76aa958
commit 8dcfe52ac8
4 changed files with 631 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/handlers/payments/charge_helpers_test.go
+28
View File
@@ -204,3 +204,31 @@ func TestEncryptDecryptSnapshot_WrongKeyFails(t *testing.T) {
_, err = decryptSnapshot(enc) _, err = decryptSnapshot(enc)
require.Error(t, err, "a snapshot encrypted with a different key must not decrypt") require.Error(t, err, "a snapshot encrypted with a different key must not decrypt")
} }
// TestResolveChargeSource_SCATokenizeResult_UsesTokenAsSource pins the SCA
// wire contract at the source-resolution level: a call carrying BOTH a
// new-card token (the SCA tokenize-result) and a saved card id must return the
// TOKEN as the charge source — never the stored ccof id — while still deriving
// the Square customer from the saved card row.
func TestResolveChargeSource_SCATokenizeResult_UsesTokenAsSource(t *testing.T) {
ctx := context.Background()
userID, err := fixtures.CreateTestUser(db.Conn)
require.NoError(t, err)
defer func() {
InvalidateSquareCustomerCache(userID)
_, _ = db.Conn.Exec(ctx, `DELETE FROM user_saved_cards WHERE user_id = $1`, userID)
_, _ = db.Conn.Exec(ctx, `DELETE FROM users WHERE id = $1`, userID)
}()
cardID, err := fixtures.CreateTestPaymentMethod(db.Conn, userID, "ccof:sca-tokenize-unit", "VISA", "4242")
require.NoError(t, err)
token := "cnon:sca-tokenize-unit"
sourceID, savedCardID, sqCustID, ok := resolveChargeSource(ctx, httptest.NewRecorder(), NewPaymentService(), userID, &token, &cardID, false, "")
require.True(t, ok, "the tokenize-result source must resolve")
require.Equal(t, token, sourceID, "the tokenize-result token must be the charge source")
require.NotEqual(t, "ccof:sca-tokenize-unit", sourceID, "the stored ccof id must NOT be the source")
require.NotNil(t, savedCardID, "the saved-card row id must be returned")
require.Equal(t, cardID, *savedCardID, "the saved-card row id must match the input card")
require.NotEmpty(t, sqCustID, "customer_id must derive from the saved card row")
}
backend/handlers/payments/errors_test.go
+26 -24
View File
@@ -729,10 +729,11 @@ func TestVerificationRequiredSurfacing_AllChargeSites(t *testing.T) {
}) })
} }
// the dedicated add-card endpoint: with REQUIRE_2FA enforced and the user NOT // the dedicated add-card endpoint: with REQUIRE_2FA enforced, persisting a card
// having completed 2FA setup, persisting a card is blocked with 403 and no card // is refused 402 verification_required (SCA-only — the 2FA fallback was
// row is created — the save-card endpoint is not an un-gated side door. // removed) and no card row is created — the save-card endpoint is not an
func TestTwoFactorEnforced_CreatePaymentMethod_Blocked_403(t *testing.T) { // un-gated side door.
func TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_402(t *testing.T) {
t.Setenv("REQUIRE_2FA", "true") t.Setenv("REQUIRE_2FA", "true")
t.Setenv("SQUARE_ENVIRONMENT", "production") t.Setenv("SQUARE_ENVIRONMENT", "production")
ctx, tx := testutils.SetupTestTx(t) ctx, tx := testutils.SetupTestTx(t)
@@ -745,26 +746,26 @@ func TestTwoFactorEnforced_CreatePaymentMethod_Blocked_403(t *testing.T) {
handler := CreatePaymentMethod handler := CreatePaymentMethod
w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "cnon:2fa-blocked"}, token, ctx) w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "cnon:2fa-blocked"}, token, ctx)
if w.Code != http.StatusForbidden { if w.Code != http.StatusPaymentRequired {
t.Fatalf("expected 403 when 2FA is enforced and the user has not enabled it, got %d: %s", w.Code, w.Body.String()) t.Fatalf("expected 402 verification_required when 2FA is enforced (SCA-only), got %d: %s", w.Code, w.Body.String())
} }
var body map[string]string var body map[string]string
require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body))
require.Contains(t, body["error"], "Two-factor") require.Equal(t, "verification_required", body["code"])
var cardCount int var cardCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, userID).Scan(&cardCount); err != nil { if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, userID).Scan(&cardCount); err != nil {
t.Fatalf("failed to count saved cards: %v", err) t.Fatalf("failed to count saved cards: %v", err)
} }
if cardCount != 0 { if cardCount != 0 {
t.Errorf("a blocked 2FA save must not persist a card, got %d rows", cardCount) t.Errorf("a refused token-less save must not persist a card, got %d rows", cardCount)
} }
} }
// TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Succeeds verifies the gate // TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Blocked verifies that even
// lets a user WHO HAS enabled 2FA (and provides a matching one-time code) save // a VALID 2FA code cannot save a card through the add-card endpoint when
// a card through the add-card endpoint. // enforced (SCA-only — the homegrown 2FA fallback was removed entirely).
func TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Succeeds(t *testing.T) { func TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Blocked(t *testing.T) {
t.Setenv("REQUIRE_2FA", "true") t.Setenv("REQUIRE_2FA", "true")
t.Setenv("SQUARE_ENVIRONMENT", "production") t.Setenv("SQUARE_ENVIRONMENT", "production")
ctx, tx := testutils.SetupTestTx(t) ctx, tx := testutils.SetupTestTx(t)
@@ -782,24 +783,25 @@ func TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Succeeds(t *testing.T) {
handler := CreatePaymentMethod handler := CreatePaymentMethod
w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "cnon:2fa-ok", VerificationCode: "778899"}, token, ctx) w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "cnon:2fa-ok", VerificationCode: "778899"}, token, ctx)
if w.Code != http.StatusOK { if w.Code != http.StatusPaymentRequired {
t.Fatalf("expected 200 when 2FA is enabled and the code matches, got %d: %s", w.Code, w.Body.String()) t.Fatalf("expected 402 verification_required even with a valid 2FA code (SCA-only), got %d: %s", w.Code, w.Body.String())
} }
var cardCount int var cardCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, userID).Scan(&cardCount); err != nil { if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, userID).Scan(&cardCount); err != nil {
t.Fatalf("failed to count saved cards: %v", err) t.Fatalf("failed to count saved cards: %v", err)
} }
if cardCount != 1 { if cardCount != 0 {
t.Errorf("expected exactly 1 saved card, got %d", cardCount) t.Errorf("a valid 2FA code must not persist a card (SCA-only), got %d", cardCount)
} }
} }
// TestTwoFactorEnforced_BuyGiftCard_SaveCard_Blocked_403 verifies the H4 gate // TestTwoFactorEnforced_BuyGiftCard_SaveCard_Tokenless_402 verifies the H4 gate
// fires on the gift-card purchase path too: BuyGiftCard with req.SaveCard=true // fires on the gift-card purchase path too: BuyGiftCard with req.SaveCard=true
// requires 2FA when enforced, mirroring CreatePaymentMethod/CreateBookingPayment. // is refused 402 verification_required (SCA-only) when enforced, mirroring
// The purchase is rejected with 403 BEFORE any payment row is inserted. // CreatePaymentMethod/CreateBookingPayment. The purchase is rejected BEFORE any
func TestTwoFactorEnforced_BuyGiftCard_SaveCard_Blocked_403(t *testing.T) { // payment row is inserted.
func TestTwoFactorEnforced_BuyGiftCard_SaveCard_Tokenless_402(t *testing.T) {
t.Setenv("REQUIRE_2FA", "true") t.Setenv("REQUIRE_2FA", "true")
t.Setenv("SQUARE_ENVIRONMENT", "production") t.Setenv("SQUARE_ENVIRONMENT", "production")
ctx, tx := testutils.SetupTestTx(t) ctx, tx := testutils.SetupTestTx(t)
@@ -820,19 +822,19 @@ func TestTwoFactorEnforced_BuyGiftCard_SaveCard_Blocked_403(t *testing.T) {
} }
w := makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", req, token, ctx) w := makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", req, token, ctx)
if w.Code != http.StatusForbidden { if w.Code != http.StatusPaymentRequired {
t.Fatalf("expected 403 for BuyGiftCard with SaveCard=true without 2FA, got %d: %s", w.Code, w.Body.String()) t.Fatalf("expected 402 for BuyGiftCard with SaveCard=true without SCA, got %d: %s", w.Code, w.Body.String())
} }
var body map[string]string var body map[string]string
require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body))
require.Contains(t, body["error"], "Two-factor") require.Equal(t, "verification_required", body["code"])
var payCount int var payCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE created_by = $1`, userID).Scan(&payCount); err != nil { if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE created_by = $1`, userID).Scan(&payCount); err != nil {
t.Fatalf("failed to count payments: %v", err) t.Fatalf("failed to count payments: %v", err)
} }
if payCount != 0 { if payCount != 0 {
t.Errorf("a blocked gift-card purchase must not create a payment row, got %d", payCount) t.Errorf("a refused gift-card purchase must not create a payment row, got %d", payCount)
} }
} }
backend/handlers/payments/till.go
+88 -19
View File
@@ -48,6 +48,11 @@ type TillSaleRequest struct {
// enforced environment charges a saved card only when this matches the // enforced environment charges a saved card only when this matches the
// customer's pending code. // customer's pending code.
VerificationCode string `json:"verification_code,omitempty"` VerificationCode string `json:"verification_code,omitempty"`
// ConsentVersion / ConsentAccepted: the customer's explicit versioned
// consent to the SCA-unavailable → 2FA fallback (C6), enforced server-side
// (403 consent_required) and recorded on the 2fa_fallback_charge audit row.
ConsentVersion *string `json:"consent_version,omitempty"`
ConsentAccepted bool `json:"consent_accepted"`
} }
type TillSaleResponse struct { type TillSaleResponse struct {
@@ -631,6 +636,24 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
// idempotency dedup so a same-key retry of an already-completed sale is // idempotency dedup so a same-key retry of an already-completed sale is
// returned (not blocked) even on a capped day. Runs before any gift-card // returned (not blocked) even on a capped day. Runs before any gift-card
// write. // write.
//
// F5: the cap check and the sale's gift-card creation/top-up run under the
// SAME per-admin advisory lock the admin API surfaces use
// (acquireGiftCardDailyCapLock, giftcards.go). The per-sale
// crussell:till:<idempotencyKey> lock above serializes ONE sale's retries,
// but two concurrent DISTINCT till sales by the same admin could otherwise
// both read a below-cap day's value before either commits and overshoot the
// £5,000 ceiling. The lock is acquired BEFORE the adminGiftCardValueToday
// read and held (via the defers) through the transaction that records the
// new value, exactly like CreateGiftCard / TopUpGiftCard /
// TransferGiftCard.
capPinConn, capLockOK := acquireGiftCardDailyCapLock(ctx, w, adminID)
if !capLockOK {
return
}
defer capPinConn.Release()
defer releasePaymentLock(capPinConn, giftCardDailyCapLockKey+adminID)
adminValueToday, dailyErr := adminGiftCardValueToday(ctx, db.Conn, adminID) adminValueToday, dailyErr := adminGiftCardValueToday(ctx, db.Conn, adminID)
if dailyErr != nil { if dailyErr != nil {
log.Printf("Failed to query admin gift-card value today for %s: %v", adminID, dailyErr) log.Printf("Failed to query admin gift-card value today for %s: %v", adminID, dailyErr)
@@ -778,13 +801,31 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
var isInventory bool var isInventory bool
var previousTotal float64 var previousTotal float64
err = tx.QueryRow(ctx, `SELECT is_inventory, total_funds_added FROM gift_cards WHERE id = $1`, cardID).Scan(&isInventory, &previousTotal) var expiryDate sql.NullTime
err = tx.QueryRow(ctx, `SELECT is_inventory, total_funds_added, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&isInventory, &previousTotal, &expiryDate)
if err != nil { if err != nil {
log.Printf("Failed to check gift card state: %v", err) log.Printf("Failed to check gift card state: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError) http.Error(w, "internal server error", http.StatusInternalServerError)
return return
} }
// money-F4: an expired gift card must never be topped up — the
// UPDATE below resets expiry_date to NOW()+months and would
// resurrect a card whose remaining value the nightly cleanup job
// already forfeited. Mirrors giftcards.go RedeemGiftCard's expiry
// gate (same DB-clock comparison, same 400); a NULL expiry_date
// (legacy) is treated as unexpired.
expired, err := giftCardExpired(ctx, tx, expiryDate)
if err != nil {
log.Printf("Failed to check gift card expiry: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if expired {
http.Error(w, "Gift card has expired", http.StatusBadRequest)
return
}
_, err = tx.Exec(ctx, ` _, err = tx.Exec(ctx, `
UPDATE gift_cards UPDATE gift_cards
SET total_funds_added = total_funds_added + $1, SET total_funds_added = total_funds_added + $1,
@@ -957,24 +998,14 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
dbPaymentMethod = "cash" dbPaymentMethod = "cash"
case "saved_card": case "saved_card":
dbPaymentMethod = "online_square" dbPaymentMethod = "online_square"
if req.UserID != nil && *req.UserID != "" {
_, err = service.GetCardByIDQuerier(ctx, tx, *req.UserSavedCardID, *req.UserID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
http.Error(w, "Saved card not found", http.StatusNotFound)
return
}
log.Printf("Failed to verify saved card: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
}
// The till path is not user-scoped, so fetch the card's owner along with // The till path is not user-scoped, so fetch the card's owner along with
// the charge details — the owner is needed to lazily provision a Square // the charge details — the owner-agnostic SELECT is the source of truth
// customer if the row predates P14 (R6). cardUserID is declared at // for ownership. The owner is needed to lazily provision a Square
// function scope (before the switch) because the post-charge 2FA // customer if the row predates P14 (R6), and to key the 2FA/consent
// consumption + audit need it after the Square call. // gate. cardUserID is declared at function scope (before the switch)
// because the post-charge 2FA consumption + audit need it after the
// Square call.
err = tx.QueryRow(ctx, ` err = tx.QueryRow(ctx, `
SELECT user_id, COALESCE(square_card_id, ''), COALESCE(square_customer_id, '') SELECT user_id, COALESCE(square_card_id, ''), COALESCE(square_customer_id, '')
FROM user_saved_cards FROM user_saved_cards
@@ -986,6 +1017,20 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
return return
} }
// F1 ownership invariant: every other charge surface verifies the card
// belongs to the request's user before charging. When the admin names
// the customer being served (req.UserID), the card MUST be owned by
// that customer — an ownerless or foreign card is a confused-deputy
// signal and is rejected outright. An OMITTED user_id is the admin-till
// legit use (the admin may charge any customer's card): the charge
// proceeds under the RESOLVED owner, which keys the 2FA/consent gate
// below and is the target of the confused-deputy CRITICAL audit on
// success.
if req.UserID != nil && *req.UserID != "" && (!cardUserID.Valid || cardUserID.String != *req.UserID) {
http.Error(w, "Saved card does not belong to the specified user", http.StatusBadRequest)
return
}
// A ccof: source can NEVER be charged without a CustomerID — Square // A ccof: source can NEVER be charged without a CustomerID — Square
// rejects the payment. Legacy pre-P14 rows have an empty // rejects the payment. Legacy pre-P14 rows have an empty
// square_customer_id; provision + persist for the card's owner BEFORE // square_customer_id; provision + persist for the card's owner BEFORE
@@ -1029,6 +1074,11 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
if !gateOK { if !gateOK {
return return
} }
// C6: a fallback-authorized charge must carry the customer's
// accepted consent (403 consent_required otherwise).
if !enforceSCAFallbackConsent(w, req.ConsentVersion, req.ConsentAccepted, twoFAFallbackUsed) {
return
}
} }
tillSquareSourceID = savedCardSqCardID tillSquareSourceID = savedCardSqCardID
@@ -1296,7 +1346,15 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
// attempt's responsibility". // attempt's responsibility".
if isDefinitiveChargeFailure(squareErr) { if isDefinitiveChargeFailure(squareErr) {
if revErr := revertGiftCardFunding(ctx, req.Action, giftCardID, req.Amount, req.RedeemToUserID, tillSaleID); revErr != nil { if revErr := revertGiftCardFunding(ctx, req.Action, giftCardID, req.Amount, req.RedeemToUserID, tillSaleID); revErr != nil {
log.Printf("CRITICAL: till sale %s charge definitively failed (%v) but gift-card clawback also failed: %v — MANUAL RECONCILIATION REQUIRED: gift card %s may still be funded", tillSaleID, squareErr, revErr, giftCardID) // M6: the clawback ALWAYS reverts the funding on a failed
// sale; the two failure shapes here are a partial reversal
// (funding was already spent — a CRITICAL admin notification
// is already inserted by the clawback) and a hard failure.
if errors.Is(revErr, errClawbackPartiallyReversed) {
log.Printf("CRITICAL: till sale %s charge definitively failed (%v) — gift-card funding was PARTIALLY clawed back (already spent) — MANUAL RECONCILIATION REQUIRED: gift card %s may retain a residual to reconcile", tillSaleID, squareErr, giftCardID)
} else {
log.Printf("CRITICAL: till sale %s charge definitively failed (%v) but gift-card clawback also failed: %v — MANUAL RECONCILIATION REQUIRED: gift card %s may still be funded", tillSaleID, squareErr, revErr, giftCardID)
}
} }
} }
// The gate consumed the 2FA code for a FRESH saved-card charge — // The gate consumed the 2FA code for a FRESH saved-card charge —
@@ -1369,7 +1427,18 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
// The 2FA BACKUP authorized this token-less saved-card till charge // The 2FA BACKUP authorized this token-less saved-card till charge
// (SCA was unavailable); the actor is the admin from context. // (SCA was unavailable); the actor is the admin from context.
if twoFAFallbackUsed { if twoFAFallbackUsed {
insertTwoFAFallbackAudit(ctx, adminID, cardUserID.String, paymentResult.CardLast4, tillSaleID, "admin till saved-card charge authorized via 2FA fallback (SCA unavailable)") insertTwoFAFallbackAudit(ctx, adminID, cardUserID.String, paymentResult.CardLast4, tillSaleID, "admin till saved-card charge authorized via 2FA fallback (SCA unavailable)", consentVersionValue(req.ConsentVersion), req.ConsentAccepted)
}
// F1 confused-deputy audit: a saved-card till charge that proceeded
// WITHOUT the request naming the customer (user_id omitted) charged
// the card under its RESOLVED owner — a card never associated with
// the sale's stated user. Surface a CRITICAL operator notification
// (deduped per owner) so an admin who charges a card under an
// unintended owner is caught. A provided-and-matched user_id needs
// no such flag; a provided-and-mismatched one never reaches a charge.
if req.UserID == nil || *req.UserID == "" {
ownerID := cardUserID.String
insertCriticalPaymentNotification(ctx, nil, &ownerID)
} }
} }
backend/handlers/payments/till_test.go
+489 -16
View File
@@ -9,6 +9,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"log" "log"
"math"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"strings" "strings"
@@ -2102,12 +2103,13 @@ func TestCreateTillSale_PendingRetry_CardMachineSwitch_Rejected(t *testing.T) {
// revertGiftCardFunding — top-up guard-blocked reversal still fails the sale // revertGiftCardFunding — top-up guard-blocked reversal still fails the sale
// ============================================================================= // =============================================================================
// TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale locks // TestRevertGiftCardFunding_TopupPartiallySpent_ClampsAndFlags locks the M6
// the claim-first top-up guard: the gating claim succeeds (sale pending), then // fix: the clawback's balance guard no longer leaves the funding in place when
// the guarded reversal is blocked because some of the top-up was already spent. // some of the top-up was already spent. The reversal CLAMPS the card to zero
// The clawback must NOT fail (the sale still has to be marked failed) and must // (everything still on the card is reclaimed), the sale is still marked failed,
// leave the card amounts untouched. // and the unreclaimable spent portion surfaces as a CRITICAL admin notification
func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t *testing.T) { // + errClawbackPartiallyReversed so every caller flags reconciliation.
func TestRevertGiftCardFunding_TopupPartiallySpent_ClampsAndFlags(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t) ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx) adminID, err := fixtures.CreateTestAdminUser(tx)
@@ -2140,20 +2142,21 @@ func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t
t.Fatalf("failed to seed gift card transaction: %v", err) t.Fatalf("failed to seed gift card transaction: %v", err)
} }
// amount_remaining (10.00) < top-up (50.00) — the guarded UPDATE matches 0 // amount_remaining (10.00) < top-up (50.00) — some of the funding was
// rows. The clawback must NOT fail (the sale still has to be marked failed) // already spent. M6: the clawback must CLAMP the card to zero and return
// and must log CRITICAL for manual reconciliation. // errClawbackPartiallyReversed (funding reverts except the unrecoverable
// spent portion), never leave the funding on the card.
err = revertGiftCardFunding(ctx, "topup", cardID, 50.00, nil, saleID) err = revertGiftCardFunding(ctx, "topup", cardID, 50.00, nil, saleID)
if err != nil { if !errors.Is(err, errClawbackPartiallyReversed) {
t.Fatalf("revertGiftCardFunding must not fail when the guard blocks the reversal, got: %v", err) t.Fatalf("expected errClawbackPartiallyReversed when the top-up was partially spent, got %v", err)
} }
var remaining, totalAdded float64 var remaining, totalAdded float64
if err := tx.QueryRow(ctx, `SELECT amount_remaining, total_funds_added FROM gift_cards WHERE id = $1`, cardID).Scan(&remaining, &totalAdded); err != nil { if err := tx.QueryRow(ctx, `SELECT amount_remaining, total_funds_added FROM gift_cards WHERE id = $1`, cardID).Scan(&remaining, &totalAdded); err != nil {
t.Fatalf("failed to query gift card: %v", err) t.Fatalf("failed to query gift card: %v", err)
} }
if remaining != 10.00 || totalAdded != 50.00 { if remaining != 0.00 || totalAdded != 0.00 {
t.Errorf("guard-blocked reversal must leave the card amounts untouched, got remaining=%.2f total_funds_added=%.2f", remaining, totalAdded) t.Errorf("expected the partially-spent top-up clamped to zero, got remaining=%.2f total_funds_added=%.2f", remaining, totalAdded)
} }
// The sale must still be marked failed — the whole point of the clawback. // The sale must still be marked failed — the whole point of the clawback.
@@ -2162,11 +2165,11 @@ func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t
t.Fatalf("failed to query till sale: %v", err) t.Fatalf("failed to query till sale: %v", err)
} }
if saleStatus != "failed" { if saleStatus != "failed" {
t.Errorf("expected till sale marked failed despite the guard-blocked reversal, got %q", saleStatus) t.Errorf("expected till sale marked failed despite the partial reversal, got %q", saleStatus)
} }
// This request's top-up transaction must be removed even though the card // This request's top-up transaction must be removed — the sale failed and
// amount could not be reversed. // its funding is gone from the card.
var txCount int var txCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1 AND reference_type = 'till_sale' AND reference_id = $2`, cardID, saleID).Scan(&txCount); err != nil { if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1 AND reference_type = 'till_sale' AND reference_id = $2`, cardID, saleID).Scan(&txCount); err != nil {
t.Fatalf("failed to count gift card transactions: %v", err) t.Fatalf("failed to count gift card transactions: %v", err)
@@ -2894,3 +2897,473 @@ func TestCreateTillSale_SweepResolvedMidFlight_NoResurrect(t *testing.T) {
t.Errorf("expected the CRITICAL 0-rows reconciliation log, got: %s", got) t.Errorf("expected the CRITICAL 0-rows reconciliation log, got: %s", got)
} }
} }
// TestCreateTillSale_SavedCard_UserMismatch_Rejected locks the money-F1 fix:
// when the admin names the customer being served (user_id), the saved card MUST
// be owned by that customer. A card owned by a different user is rejected with
// 400 before any Square charge — the ownership invariant every other charge
// surface enforces, now enforced unconditionally at the till.
func TestCreateTillSale_SavedCard_UserMismatch_Rejected(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx)
if err != nil {
t.Fatalf("failed to create admin user: %v", err)
}
adminToken := jwt.GenerateTestToken(adminID, "admin")
// The customer the admin claims to be serving.
claimedUserID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create claimed user: %v", err)
}
// The customer who actually owns the card.
ownerID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create card owner user: %v", err)
}
cardID, err := fixtures.CreateTestPaymentMethod(tx, ownerID, "ccof:sq_test_card_id", "VISA", "1234")
if err != nil {
t.Fatalf("failed to create saved card: %v", err)
}
reqBody := TillSaleRequest{
ItemType: "gift_card",
Action: "create",
Amount: 50.00,
PaymentMethod: "saved_card",
UserSavedCardID: &cardID,
UserID: &claimedUserID,
}
bodyBytes, _ := json.Marshal(reqBody)
req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes))
req.Header.Set("Authorization", "Bearer "+adminToken)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Post("/api/admin/till/sale", CreateTillSale)
r.ServeHTTP(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("expected 400 (ownership mismatch), got %d. body: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "does not belong to the specified user") {
t.Errorf("expected ownership-mismatch error body, got: %s", w.Body.String())
}
// The rejected request must not have created a sale or funded a gift card
// (the transaction is rolled back before commit).
var saleCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM till_sales WHERE user_saved_card_id = $1`, cardID).Scan(&saleCount); err != nil {
t.Fatalf("failed to query till_sales: %v", err)
}
if saleCount != 0 {
t.Errorf("expected no till_sale for a rejected ownership mismatch, got %d", saleCount)
}
var gcCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_cards WHERE created_by = $1`, adminID).Scan(&gcCount); err != nil {
t.Fatalf("failed to query gift_cards: %v", err)
}
if gcCount != 0 {
t.Errorf("expected no gift card funded by a rejected ownership mismatch, got %d", gcCount)
}
}
// TestCreateTillSale_SavedCard_OmittedUser_ChargesResolvedOwner locks the
// money-F1 omitted-user path: when the admin does not name a customer, the till
// still resolves the card's owner from the row and charges under it (the
// admin-till legit use — the admin may charge any customer's card). Because the
// card was never associated with a user named by the request, the charge
// surfaces a CRITICAL 'critical_payment_log' operator notification for the
// RESOLVED owner — the confused-deputy catch.
func TestCreateTillSale_SavedCard_OmittedUser_ChargesResolvedOwner(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx)
if err != nil {
t.Fatalf("failed to create admin user: %v", err)
}
adminToken := jwt.GenerateTestToken(adminID, "admin")
ownerID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create card owner user: %v", err)
}
cardID, err := fixtures.CreateTestPaymentMethod(tx, ownerID, "ccof:sq_test_card_id", "VISA", "1234")
if err != nil {
t.Fatalf("failed to create saved card: %v", err)
}
reqBody := TillSaleRequest{
ItemType: "gift_card",
Action: "create",
Amount: 50.00,
PaymentMethod: "saved_card",
UserSavedCardID: &cardID,
// UserID intentionally omitted — the admin-till legit use.
}
bodyBytes, _ := json.Marshal(reqBody)
req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes))
req.Header.Set("Authorization", "Bearer "+adminToken)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Post("/api/admin/till/sale", CreateTillSale)
r.ServeHTTP(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("expected 201 (resolved-owner charge), got %d. body: %s", w.Code, w.Body.String())
}
var resp TillSaleResponse
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
if resp.Status != "completed" {
t.Errorf("expected status 'completed', got '%s'", resp.Status)
}
// No user was named, so the sale row stores no user attribution.
var saleUserID *string
if err := tx.QueryRow(ctx, `SELECT user_id FROM till_sales WHERE id = $1`, resp.ID).Scan(&saleUserID); err != nil {
t.Fatalf("failed to query till sale user: %v", err)
}
if saleUserID != nil {
t.Errorf("expected the till sale to have no user attribution (user_id omitted), got %q", *saleUserID)
}
// The unassociated (omitted-user) charge must have raised the CRITICAL
// operator notification for the RESOLVED owner.
var notifCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM admin_notifications WHERE reason = 'critical_payment_log' AND user_id = $1 AND booking_id IS NULL`, ownerID).Scan(&notifCount); err != nil {
t.Fatalf("failed to count admin notifications: %v", err)
}
if notifCount < 1 {
t.Errorf("expected a CRITICAL admin notification for the resolved owner, got %d", notifCount)
}
}
// TestCreateTillSale_SavedCard_ProvidedUserMatch_NoOwnerAudit locks the
// money-F1 provided-user match path: when the admin names the customer and the
// card IS owned by them, the charge proceeds normally and no confused-deputy
// notification is raised (the card was associated with the sale's stated user).
func TestCreateTillSale_SavedCard_ProvidedUserMatch_NoOwnerAudit(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx)
if err != nil {
t.Fatalf("failed to create admin user: %v", err)
}
adminToken := jwt.GenerateTestToken(adminID, "admin")
ownerID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create card owner user: %v", err)
}
cardID, err := fixtures.CreateTestPaymentMethod(tx, ownerID, "ccof:sq_test_card_id", "VISA", "1234")
if err != nil {
t.Fatalf("failed to create saved card: %v", err)
}
reqBody := TillSaleRequest{
ItemType: "gift_card",
Action: "create",
Amount: 50.00,
PaymentMethod: "saved_card",
UserSavedCardID: &cardID,
UserID: &ownerID,
}
bodyBytes, _ := json.Marshal(reqBody)
req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes))
req.Header.Set("Authorization", "Bearer "+adminToken)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Post("/api/admin/till/sale", CreateTillSale)
r.ServeHTTP(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("expected 201 (matching-owner charge), got %d. body: %s", w.Code, w.Body.String())
}
var resp TillSaleResponse
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
t.Fatalf("failed to decode response: %v", err)
}
if resp.Status != "completed" {
t.Errorf("expected status 'completed', got '%s'", resp.Status)
}
var notifCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM admin_notifications WHERE reason = 'critical_payment_log' AND user_id = $1`, ownerID).Scan(&notifCount); err != nil {
t.Fatalf("failed to count admin notifications: %v", err)
}
if notifCount != 0 {
t.Errorf("expected no confused-deputy notification when the card owner matches the stated user, got %d", notifCount)
}
}
// TestCreateTillSale_TopupExpiredCard_Rejected locks the money-F4 fix: the till
// top-up must reject an EXPIRED gift card with 400 and must NOT reset its
// expiry_date — the top-up UPDATE rolls expiry forward to NOW()+months and
// would otherwise resurrect a card the nightly cleanup job already forfeited.
func TestCreateTillSale_TopupExpiredCard_Rejected(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx)
if err != nil {
t.Fatalf("failed to create admin user: %v", err)
}
adminToken := jwt.GenerateTestToken(adminID, "admin")
// An expired card that still carries balance (the nightly cleanup has not
// run yet).
var cardID string
err = tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, expiry_date)
VALUES (50.00, 50.00, $1, FALSE, NOW() - INTERVAL '1 day')
RETURNING id
`, adminID).Scan(&cardID)
if err != nil {
t.Fatalf("failed to insert expired gift card: %v", err)
}
reqBody := TillSaleRequest{
ItemType: "gift_card",
Action: "topup",
Amount: 25.00,
PaymentMethod: "on_the_house",
GiftCardID: &cardID,
}
bodyBytes, _ := json.Marshal(reqBody)
req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes))
req.Header.Set("Authorization", "Bearer "+adminToken)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Post("/api/admin/till/sale", CreateTillSale)
r.ServeHTTP(w, req)
if w.Code != http.StatusBadRequest {
t.Fatalf("expected 400 (expired gift card), got %d. body: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "Gift card has expired") {
t.Errorf("expected expired-card error body, got: %s", w.Body.String())
}
// The card must be untouched: no funds added, no transaction, no sale, and
// the expiry_date must NOT be reset to the future.
var totalAdded, remaining float64
var expiry time.Time
if err := tx.QueryRow(ctx, `SELECT total_funds_added, amount_remaining, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&totalAdded, &remaining, &expiry); err != nil {
t.Fatalf("failed to query gift card: %v", err)
}
if totalAdded != 50.00 || remaining != 50.00 {
t.Errorf("expected funds untouched (50.00/50.00), got %.2f/%.2f", totalAdded, remaining)
}
if !expiry.Before(time.Now().UTC()) {
t.Errorf("expected the expired card's expiry_date to NOT be reset (still in the past), got %v", expiry)
}
var txCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1`, cardID).Scan(&txCount); err != nil {
t.Fatalf("failed to count gift_card_transactions: %v", err)
}
if txCount != 0 {
t.Errorf("expected no gift_card_transaction on the rejected top-up, got %d", txCount)
}
var saleCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM till_sales WHERE item_id = $1`, cardID).Scan(&saleCount); err != nil {
t.Fatalf("failed to count till_sales: %v", err)
}
if saleCount != 0 {
t.Errorf("expected no till_sale on the rejected top-up, got %d", saleCount)
}
}
// TestCreateTillSale_TopupUnexpiredCard_Succeeds is the positive control for
// the money-F4 expiry gate: a LIVE card can still be topped up and its expiry
// is rolled forward to NOW()+expiryMonths as designed.
func TestCreateTillSale_TopupUnexpiredCard_Succeeds(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx)
if err != nil {
t.Fatalf("failed to create admin user: %v", err)
}
adminToken := jwt.GenerateTestToken(adminID, "admin")
var cardID string
err = tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, expiry_date)
VALUES (50.00, 50.00, $1, FALSE, NOW() + INTERVAL '1 year')
RETURNING id
`, adminID).Scan(&cardID)
if err != nil {
t.Fatalf("failed to insert gift card: %v", err)
}
reqBody := TillSaleRequest{
ItemType: "gift_card",
Action: "topup",
Amount: 25.00,
PaymentMethod: "on_the_house",
GiftCardID: &cardID,
}
bodyBytes, _ := json.Marshal(reqBody)
req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes))
req.Header.Set("Authorization", "Bearer "+adminToken)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Post("/api/admin/till/sale", CreateTillSale)
r.ServeHTTP(w, req)
if w.Code != http.StatusCreated {
t.Fatalf("expected 201 for a live-card top-up, got %d. body: %s", w.Code, w.Body.String())
}
var totalAdded, remaining float64
var expiry time.Time
if err := tx.QueryRow(ctx, `SELECT total_funds_added, amount_remaining, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&totalAdded, &remaining, &expiry); err != nil {
t.Fatalf("failed to query gift card: %v", err)
}
if totalAdded != 75.00 || remaining != 75.00 {
t.Errorf("expected funds topped up to 75.00, got %.2f/%.2f", totalAdded, remaining)
}
if !expiry.After(time.Now().UTC()) {
t.Errorf("expected the live card's expiry to be rolled forward, got %v", expiry)
}
}
// =============================================================================
// F5 — £5,000/day admin gift-card cap is race-free across concurrent till sales
// =============================================================================
// TestCreateTillSale_DailyCap_Concurrent pins the F5 daily-cap serialization
// fix: N concurrent till gift-card sales by the same admin must never let the
// cumulative issued value exceed the £5,000/day cap. The till's cap check
// (adminGiftCardValueToday) and the sale's gift-card creation run under the
// SAME per-admin advisory lock the admin API surfaces use
// (acquireGiftCardDailyCapLock), so every check sees the previous sale's
// committed row — the excess requests are rejected. Without that per-admin lock
// two DISTINCT concurrent sales by the same admin both read the same
// pre-write cumulative value and both pass, over-issuing value (the per-sale
// crussell:till:<idempotencyKey> lock serializes ONE sale's retries, not
// distinct concurrent sales).
//
// The per-transaction £250 cap bounds each till sale, so exceeding the £5,000
// daily cap needs 21 sales of £250; a semaphore bounds how many run
// simultaneously (each handler holds one pool conn per advisory lock and one
// for its transaction). The admin and every handler invocation run directly
// against the REAL pool (no per-test transaction), so the batches exercise
// genuine cross-connection concurrency exactly like production.
func TestCreateTillSale_DailyCap_Concurrent(t *testing.T) {
ctx := context.Background()
adminID, err := fixtures.CreateTestAdminUser(db.Conn)
if err != nil {
t.Fatalf("failed to create admin: %v", err)
}
t.Cleanup(func() {
cctx := context.Background()
_, _ = db.Conn.Exec(cctx, `DELETE FROM admin_audit_log WHERE admin_id = $1`, adminID)
_, _ = db.Conn.Exec(cctx, `DELETE FROM gift_card_transactions WHERE gift_card_id IN (SELECT id FROM gift_cards WHERE created_by = $1)`, adminID)
_, _ = db.Conn.Exec(cctx, `DELETE FROM till_sales WHERE created_by = $1`, adminID)
_, _ = db.Conn.Exec(cctx, `DELETE FROM gift_cards WHERE created_by = $1`, adminID)
_, _ = db.Conn.Exec(cctx, `DELETE FROM users WHERE id = $1`, adminID)
})
token := jwt.GenerateTestToken(adminID, "admin")
const perSale = 250.00 // £250 per sale (at the £250 per-transaction cap)
const totalOps = 21 // 21 × £250 = £5,250 > the £5,000 daily cap
const concurrencyLimit = 4 // at most 4 handlers in flight (each holds 3 pool conns)
sem := make(chan struct{}, concurrencyLimit)
start := make(chan struct{})
var wg sync.WaitGroup
var mu sync.Mutex
successes := 0
failCodes := map[int]int{}
for i := 0; i < totalOps; i++ {
wg.Add(1)
go func() {
defer wg.Done()
<-start
sem <- struct{}{}
defer func() { <-sem }()
body, _ := json.Marshal(TillSaleRequest{
ItemType: "gift_card",
Action: "create",
Amount: perSale,
PaymentMethod: "cash",
})
r := httptest.NewRequest(http.MethodPost, "/api/admin/till/sale", bytes.NewReader(body))
r.Header.Set("Authorization", "Bearer "+token)
r.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
router := chi.NewRouter()
router.Use(mw.RequireAuth)
router.Post("/api/admin/till/sale", CreateTillSale)
router.ServeHTTP(w, r)
mu.Lock()
if w.Code == http.StatusCreated {
successes++
} else {
failCodes[w.Code]++
}
mu.Unlock()
}()
}
close(start)
wg.Wait()
// Money-safety invariant under serialization: at most 20 of the 21 sales
// may succeed (20 × £250 = £5,000 = the inclusive cap; the 21st would land
// the day on £5,250 and must be rejected). A rejected attempt surfaces as
// either the 400 cap rejection or a 409 from the bounded try-lock giving up
// under heavy contention — both are the designed backpressure and neither
// records value. Without the per-admin cap lock each batch reads the
// pre-write cumulative value so all 21 succeed, overshooting the cap —
// `successes > 20` (or a cumulative over the cap below) is the regression
// signal this test must catch.
if successes < 1 || successes > 20 {
t.Errorf("expected between 1 and 20 of 21 concurrent till sales to succeed under the £5,000 cap, got %d (cumulative value £%.2f); failure codes: %v", successes, perSale*float64(successes), failCodes)
}
// The day's issued value (the cap signal) must never exceed the cap.
issuedToday, err := adminGiftCardValueToday(ctx, db.Conn, adminID)
if err != nil {
t.Fatalf("failed to query today's issued value: %v", err)
}
if int64(math.Round(issuedToday*100)) > maxAdminGiftCardDailyPence {
t.Errorf("cumulative daily issued value £%.2f exceeds the £5,000 cap", issuedToday)
}
// The created cards must hold exactly the value of the successful sales.
var totalCreated float64
if err := db.Conn.QueryRow(ctx, `SELECT COALESCE(SUM(total_funds_added), 0) FROM gift_cards WHERE created_by = $1`, adminID).Scan(&totalCreated); err != nil {
t.Fatalf("failed to query created gift-card value: %v", err)
}
if totalCreated != perSale*float64(successes) {
t.Errorf("expected issued gift-card value £%.2f, got £%.2f", perSale*float64(successes), totalCreated)
}
}