fix: payments review rounds — money-safety, GDPR, security, gift-card cancel, modal stacking

Money-safety:
- Deterministic till idempotency fallback (Square-charging only); cash/on_the_house keep unique keys; £250 till gift-card cap; 45-char key validation
- Gift-card admin caps £250/tx + £5,000/day; user buy £500/day; BuyGiftCard allowlist unchanged
- CancelGiftCard: CCR 2013 14-day right with partial-spend refund of the unspent balance (spend verified via payments.gift_card_id); atomic vs redeem/transfer; refunds stay pending until reversal commits; admin cancel surface (AdminCancelGiftCard)
- Sweep: cancelled-booking charges failed+notified instead of silently completed; source-override replay uses live square_source_id; legacy square-less refund sweep; snapshot refresh on pending reuse
- Refund lock consolidation; recordTerminalPaymentTx shared recorder; structured Square error codes; terminal checkout CustomerID

GDPR / security:
- Notes retained as de-identified medical/safety record at erasure (single field treated as health data; rest of record wiped, no re-identification map) + comments updated per UK GDPR/Art 9/Equality Act 2010
- square_request_snapshot PII scrubbed on all erasure paths; delete_guest_user FK unlinks; verification codes + dispute reasons handled; idle/stale-guest erasure deletes Square cards/customers + CardDAV/R2
- Durable square-erasure outbox job (retry-square-erasures); 2FA dev/prod build split, pepper fail-closed, no prod code-in-log; prod 2FA delivery fail-loud without a channel
- Webhook unknown-type family split (non-money acked, money retried); untracked dispute notifications; rate-limit CF/X-Real-IP trust gating; nginx CSP nonce + api_limit

Frontend:
- Dynamic z-index stack (ui/dialog/zindex.ts) claimed in open order via data-state observer; re-claims on every reopen; removes stale !z-* overrides — nested modals (booking→user→booking) always paint newest-on-top (browser-verified 3-level + reopen)
- Mobile: iOS zoom fixes, bottom-sheet dialogs, 44px touch targets, inputmode decimal, dvh
- Gift-card buy/cancel UI, admin £250 + daily limits, cancellation/privacy/terms policy accuracy

S3:
- Connect() creates buckets before probing; in-memory fallback only on genuine unreachability; health reports degraded; stale S3_PUBLIC_URL documented (host-specific)

Tests/docs:
- 2263 test functions; all 22 backend packages green; round8/9/10 regression suites; NextEditWindowTime removes wall-clock flake; docs reconciled (notes retention, gift-card partial-use, modal T15 future work)
This commit is contained in:
2026-08-22 00:34:50 +01:00
parent 9111258461
commit 78e6d00dc5
89 changed files with 7702 additions and 853 deletions
@@ -0,0 +1,120 @@
package payments
import (
"context"
"crussell/db"
)
// Gift-card purchase/transaction limits (owner decisions).
//
// - Every admin gift-card value operation (CreateGiftCard, TopUpGiftCard,
// TransferGiftCard) is capped at £250 per transaction — tighter than the
// £10,000 ceiling ValidateAmount enforces on other payment entry points.
// - A customer (BuyGiftCard) may buy at most £500 of online gift cards per
// UTC day.
// - An admin may create/top-up/transfer at most £5,000 of gift-card value
// per UTC day.
//
// till.go keeps its own local `maxTillGiftCardAmountPence` copy of the £250
// transaction cap (see its comment); the shared constant lives here so both
// files can converge on the single owner decision.
const (
// maxAdminGiftCardTransactionPence caps a single admin gift-card
// create/top-up/transfer at £250 (25,000 pence).
maxAdminGiftCardTransactionPence = 25_000
// maxUserGiftCardDailyPence caps one user's online gift-card purchases at
// £500 (50,000 pence) per UTC day.
maxUserGiftCardDailyPence = 500_00
// maxAdminGiftCardDailyPence caps the gift-card value an admin can
// create/top-up/transfer in one UTC day at £5,000 (500,000 pence).
maxAdminGiftCardDailyPence = 500_000
)
// userGiftCardSpentToday returns the total value (in pounds) the user has
// spent on ONLINE gift-card purchases so far today, returned as a float64 so
// the caller can convert to pence with math.Round, matching the repo's
// currency convention.
//
// Signal: gift_card_transactions rows written by BuyGiftCard — the ONLY
// customer-facing online purchase path. Every BuyGiftCard purchase (self and
// friend) inserts a row with transaction_type='purchase', reference_type='api'
// and user_id = the buyer (see giftcards.go). Admin-created cards
// (CreateGiftCard/TopUpGiftCard) also write reference_type='api' but with the
// ADMIN's user id, and till sales write reference_type='till_sale', so neither
// can match a customer. The payments-based alternative (payments rows with
// payment_type='gift_card') does NOT exist in this schema — the payment_type
// enum is ('deposit','full','tip','balance','partial') and BuyGiftCard writes
// payment_type='full' — so the transactions audit log is the correct signal.
//
// "Today" is the UTC day boundary (created_at >= CURRENT_DATE), matching the
// repo's existing time convention: the DB session runs in timezone=UTC and
// completion.go uses the same CURRENT_DATE boundary for its daily loyalty
// stamp cap.
func userGiftCardSpentToday(ctx context.Context, q db.Querier, userID string) (float64, error) {
var spent float64
err := q.QueryRow(ctx, `
SELECT COALESCE(SUM(amount), 0)
FROM gift_card_transactions
WHERE user_id = $1
AND transaction_type = 'purchase'
AND reference_type = 'api'
AND created_at >= CURRENT_DATE
`, userID).Scan(&spent)
if err != nil {
return 0, err
}
return spent, nil
}
// adminGiftCardValueToday returns the total gift-card value (in pounds) the
// admin has created, topped up, or transferred today (UTC day boundary,
// created_at >= CURRENT_DATE), returned as a float64 for pence conversion.
//
// Signal (chosen to be double-count free across the three admin operations):
//
// 1. Cards the admin created today — SUM(total_funds_added). total_funds_added
// is cumulative, so a card created today already reflects any same-day
// top-up or transfer INTO it, and its creation amount.
// 2. API top-ups executed by this admin today on cards created BEFORE today
// (cards created today are excluded — term 1 already includes their
// funding via total_funds_added, so counting the top-up row again would
// double-count). This is the gift_card_transactions rows
// (reference_type='api', user_id=admin) written by CreateGiftCard
// ('purchase') and TopUpGiftCard ('topup', or 'purchase' on an inventory
// card's first top-up).
//
// Transfers INTO pre-existing cards leave no attributable audit row
// (TransferGiftCard deliberately writes no gift_card_transactions entry), so
// they are not directly counted; a transfer also creates no NEW gift-card
// liability, so the daily cap still measures all value this admin has newly
// issued today. Till sales (CreateTillSale) write reference_type='till_sale'
// and are attributed to the CUSTOMER (user_id), so they are excluded here —
// the admin daily cap covers the admin API surface only.
func adminGiftCardValueToday(ctx context.Context, q db.Querier, adminID string) (float64, error) {
var value float64
err := q.QueryRow(ctx, `
SELECT
COALESCE((
SELECT SUM(gc.total_funds_added)
FROM gift_cards gc
WHERE gc.created_by = $1 AND gc.created_at >= CURRENT_DATE
), 0)
+ COALESCE((
SELECT SUM(gct.amount)
FROM gift_card_transactions gct
WHERE gct.user_id = $1
AND gct.reference_type = 'api'
AND gct.transaction_type IN ('purchase', 'topup')
AND gct.created_at >= CURRENT_DATE
AND gct.gift_card_id NOT IN (
SELECT gc2.id FROM gift_cards gc2
WHERE gc2.created_by = $1 AND gc2.created_at >= CURRENT_DATE
)
), 0)
`, adminID).Scan(&value)
if err != nil {
return 0, err
}
return value, nil
}
File diff suppressed because it is too large Load Diff
+117 -191
View File
@@ -680,9 +680,15 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
return
}
} else {
// Refresh square_source_id on a reused pending row — the sweep
// replays the charge from the stored source.
if _, srcErr := tx.Exec(r.Context(), `UPDATE payments SET square_source_id = $1 WHERE id = $2`, sourceID, paymentID); srcErr != nil {
// Reused pending row: same immutability rule as the booking/tip
// reuse paths. square_source_id is refreshed ONLY for snapshot-less
// legacy rows; when the row already carries the original
// square_request_snapshot it is left untouched so the sweep's
// by-key replay keeps matching the FIRST attempt's body. Saved-card
// ccof sources are stable, so this is mostly latent, but keeping
// the snapshot immutable is money-safe (see the booking reuse
// comment above).
if _, srcErr := tx.Exec(r.Context(), `UPDATE payments SET square_source_id = $1 WHERE id = $2 AND (square_request_snapshot IS NULL OR square_request_snapshot = '')`, sourceID, paymentID); srcErr != nil {
log.Printf("Failed to update square_source_id on reused saved-card payment %s: %v", paymentID, srcErr)
}
}
@@ -710,10 +716,14 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
// M1: store the verbatim request JSON so the sweep can replay the charge
// with an IDENTICAL body under the same key — Square compares the whole
// request on key reuse, and a reconstructed body returns
// IDEMPOTENCY_KEY_REUSED, leaving the row pending forever.
// IDEMPOTENCY_KEY_REUSED, leaving the row pending forever. The snapshot
// is written ONLY when the row has none: it records the FIRST attempt's
// body, which stays immutable so a reused pending row never redirects
// the sweep's replay away from the original charge (same rule as the
// booking/tip paths — see the reuse branch above).
if snap, mErr := json.Marshal(paymentReq); mErr != nil {
log.Printf("Failed to marshal square_request_snapshot for saved-card payment %s: %v", paymentID, mErr)
} else if _, sErr := db.Conn.Exec(r.Context(), `UPDATE payments SET square_request_snapshot = $1 WHERE id = $2`, string(snap), paymentID); sErr != nil {
} else if _, sErr := db.Conn.Exec(r.Context(), `UPDATE payments SET square_request_snapshot = $1 WHERE id = $2 AND (square_request_snapshot IS NULL OR square_request_snapshot = '')`, string(snap), paymentID); sErr != nil {
log.Printf("Failed to store square_request_snapshot for saved-card payment %s: %v", paymentID, sErr)
}
@@ -859,6 +869,21 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
return
}
// Attach the booking customer's Square customer id (if any) so the terminal
// checkout is associated with their Square profile. Read-only lookup
// mirroring ensureSquareCustomer's read, but NEVER provisioning — a
// terminal checkout also serves walk-ins, and minting a customer profile
// for a terminal tap would create an unowned customer. No id → empty.
var checkoutCustomerID string
var bookingUserID sql.NullString
if err := db.Conn.QueryRow(r.Context(), `SELECT user_id FROM bookings WHERE id = $1`, bookingID).Scan(&bookingUserID); err == nil && bookingUserID.Valid {
_ = db.Conn.QueryRow(r.Context(), `
SELECT square_customer_id FROM user_saved_cards
WHERE user_id = $1 AND square_customer_id IS NOT NULL AND square_customer_id <> ''
ORDER BY created_at DESC LIMIT 1
`, bookingUserID.String).Scan(&checkoutCustomerID)
}
checkoutReq := square.CreateCheckoutReq{
Amount: amount,
Currency: "GBP",
@@ -868,6 +893,7 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
// (totalWithTip), so the terminal must NOT prompt for a second tip —
// setting AllowTipping here would double-count the tip in production.
AllowTipping: false,
CustomerID: checkoutCustomerID,
}
checkout, err := SquareClient.CreateCheckout(r.Context(), checkoutReq)
@@ -1109,8 +1135,6 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) {
}
if paymentResult.Status == "COMPLETED" {
service := NewPaymentService()
// Serialize terminal-completion records per Square payment ID. Two
// concurrent polls of the same checkout could otherwise BOTH pass the
// dedup SELECT and BOTH INSERT, with the second dying on the
@@ -1136,22 +1160,13 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Payment in progress, try again", http.StatusConflict)
return
}
defer func() {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext('crussell:terminal:' || $1))
`, terminalLockKey); err != nil {
log.Printf("Failed to release terminal-completion serialization lock for %s: %v", terminalLockKey, err)
}
}()
// Deterministic idempotency key derived from booking + amount + Square
// payment ID. The Square payment ID disambiguates two distinct
// equal-amount charges on the same booking, so equal amounts never
// collide on the UNIQUE constraint.
idempotencyKey := bookingID + "-terminal-" + strconv.FormatInt(paymentResult.Amount, 10) + "-" + paymentResult.SquarePayID
defer releasePaymentLock(pinConn, "crussell:terminal:"+terminalLockKey)
// Begin the transaction BEFORE the dedup lookup so it's atomic with the
// payment insert.
// payment insert. The shared money-recording core
// (recordTerminalPaymentTx, sweep.go) runs inside this transaction,
// commits it, and completes a now-fully-paid booking; this handler
// maps the result to the HTTP response.
tx, err := db.Conn.Begin(r.Context())
if err != nil {
log.Printf("Failed to begin transaction: %v", err)
@@ -1164,160 +1179,19 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) {
}
}()
// Dedup by Square payment ID: a double poll of the same terminal
// checkout must return the existing payment row instead of inserting a
// duplicate (which previously 500'd on the idempotency-key UNIQUE
// violation after the customer had already paid).
var existingID string
if err := tx.QueryRow(r.Context(), `
SELECT id FROM payments
WHERE booking_id = $1 AND square_payment_id = $2
`, bookingID, paymentResult.SquarePayID).Scan(&existingID); err == nil {
if err := json.NewEncoder(w).Encode(PaymentStatusResponse{
Status: "COMPLETED",
PaymentID: existingID,
Amount: paymentResult.Amount,
CardBrand: paymentResult.CardBrand,
CardLast4: paymentResult.CardLast4,
ReceiptURL: paymentResult.ReceiptURL,
}); err != nil {
log.Printf("Failed to encode JSON response: %v", err)
}
return
} else if !errors.Is(err, pgx.ErrNoRows) {
log.Printf("Failed to check for existing payment: %v", err)
}
// Re-check the booking status after the advisory lock: a concurrent
// cancellation/eviction can move the booking out of a payable state
// between the terminal charge completing and this poll recording it. A
// charge landing on a cancelled/lapsed/no-show booking must not be
// recorded as a completed payment — the cancellation refund path
// computes refunds from completed payments and would silently exclude
// this charge. Mark the checkout failed and alert ops: money was taken
// at Square and MUST be refunded manually (mirrors CreateBookingPayment's
// post-charge recheck).
var recheckStatus string
// FOR UPDATE (C5): serializes against the cancellation path's lock on
// the same row so a concurrent cancellation cannot commit between this
// recheck and the transaction commit below.
if err := tx.QueryRow(r.Context(), `SELECT status FROM bookings WHERE id = $1 FOR UPDATE`, bookingID).Scan(&recheckStatus); err != nil {
log.Printf("CRITICAL: Square payment %s for checkout %s was processed but re-reading booking %s status failed: %v — manual reconciliation required",
paymentResult.SquarePayID, checkoutID, bookingID, err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if !bookingStatusAllowsCompletedPayment(recheckStatus) {
log.Printf("CRITICAL: Square payment %s for checkout %s was processed but booking %s is now %q — marking checkout failed; money taken at Square MUST be refunded manually",
paymentResult.SquarePayID, checkoutID, bookingID, recheckStatus)
if _, upErr := tx.Exec(r.Context(), `UPDATE terminal_checkouts SET status = 'failed', updated_at = NOW() WHERE checkout_id = $1`, checkoutID); upErr != nil {
log.Printf("CRITICAL: Square payment %s landed on %q booking %s but marking checkout %s failed errored: %v — manual reconciliation required",
paymentResult.SquarePayID, recheckStatus, bookingID, checkoutID, upErr)
}
if cErr := tx.Commit(r.Context()); cErr != nil {
log.Printf("CRITICAL: Square payment %s landed on %q booking %s and committing the checkout-failed mark errored: %v — manual reconciliation required",
paymentResult.SquarePayID, recheckStatus, bookingID, cErr)
}
http.Error(w, "This booking is no longer accepting payments", http.StatusConflict)
return
}
// The payment type the admin charged is recorded on the checkout row
// by CreateTerminalPayment. Fall back to 'full' for legacy checkouts
// created before that record existed.
var checkoutPaymentType string
if err := tx.QueryRow(r.Context(), `
SELECT payment_type FROM terminal_checkouts WHERE checkout_id = $1
`, checkoutID).Scan(&checkoutPaymentType); err != nil {
if !errors.Is(err, pgx.ErrNoRows) {
log.Printf("Failed to read payment type for checkout %s: %v", checkoutID, err)
}
checkoutPaymentType = "full"
}
record := PaymentRecord{
BookingID: bookingID,
PaymentType: checkoutPaymentType,
PaymentMethod: "in_person_card",
Status: "completed",
Amount: float64(paymentResult.Amount) / 100.0,
SquarePaymentID: &paymentResult.SquarePayID,
IdempotencyKey: &idempotencyKey,
Fees: float64(paymentResult.Fees) / 100.0,
CreatedAt: clock.Now(),
UpdatedAt: clock.Now(),
}
// M4: a terminal charge above the remaining booking value is a tip
// (e.g. £100 booking + £10 tip = one £110 Square charge). Split it into
// deposit + balance + tip records so only the booking portion is
// refundable while the tip is recorded for accounting (total_tips
// aggregation) and excluded from cancellation refunds. Without a tip
// the charge stays a single record. The tip is derived from the amount
// exceeding the remaining booking value ("after 100% is tips") — NOT
// from Square's TipAmount field, because the frontend already embeds
// any tip in the amount and AllowTipping is disabled (see
// CreateTerminalPayment), so Square reports TipAmount 0.
var records []PaymentRecord
bookingInfo, bErr := service.GetBookingPaymentInfo(r.Context(), bookingID)
if bErr == nil && bookingInfo != nil {
charged := float64(paymentResult.Amount) / 100.0
remainingBookingValue := math.Max(0, bookingInfo.TotalAmount-bookingInfo.TotalPaid)
bookingPortion := math.Min(charged, remainingBookingValue)
bookingPortion = math.Round(bookingPortion*100) / 100
tipAmount := math.Round((charged-bookingPortion)*100) / 100
if tipAmount > 0.004 {
records = buildTerminalSplitRecords(record, bookingInfo, bookingPortion, tipAmount)
}
}
if len(records) == 0 {
records = []PaymentRecord{record}
}
primary := records[0]
paymentID, err := service.CreatePaymentRecordTx(r.Context(), tx, primary, nil)
if err != nil {
log.Printf("Failed to create payment record: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
ApplyVATToBookingPayment(r.Context(), tx, paymentID)
var splitIDs []string
for _, rec := range records[1:] {
pid, cErr := service.CreatePaymentRecordTx(r.Context(), tx, rec, nil)
if cErr != nil {
log.Printf("Failed to create terminal tip split record: %v", cErr)
http.Error(w, "internal server error", http.StatusInternalServerError)
paymentID, recErr := recordTerminalPaymentTx(r.Context(), tx, checkoutID, bookingID, paymentResult)
if recErr != nil {
if errors.Is(recErr, errTerminalBookingNotPayable) {
// The core already committed the checkout's 'failed' mark:
// money was taken at Square on a booking that is no longer
// payable and MUST be refunded manually.
http.Error(w, "This booking is no longer accepting payments", http.StatusConflict)
return
}
splitIDs = append(splitIDs, pid)
}
for _, pid := range splitIDs {
ApplyVATToBookingPayment(r.Context(), tx, pid)
}
// Release the in-flight guard: this checkout is done, so a subsequent
// charge on the same booking is allowed.
if _, err := tx.Exec(r.Context(), `
UPDATE terminal_checkouts SET status = 'COMPLETED', updated_at = NOW() WHERE checkout_id = $1
`, checkoutID); err != nil {
log.Printf("Failed to mark terminal checkout %s completed: %v", checkoutID, err)
}
if err := tx.Commit(r.Context()); err != nil {
log.Printf("Failed to commit transaction: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
// Fully-paid completion: if this terminal charge (or the accumulated
// total) now covers 100% of the booking total, complete the booking so
// it leaves the admin's Current Appointment view. Runs in its own
// transaction because the payment-recording transaction above has
// already committed.
completeFullyPaidBooking(r.Context(), bookingID)
if err := json.NewEncoder(w).Encode(PaymentStatusResponse{
Status: "COMPLETED",
PaymentID: paymentID,
@@ -1780,11 +1654,24 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
// pattern as CreateTipPayment.
ApplyVATToBookingPayment(r.Context(), tx, paymentID)
} else {
// Refresh square_source_id on a reused pending row: this attempt may
// charge a different token than the failed attempt (one-time cnon:
// nonces are spent), and the sweep replays the charge from the stored
// source.
if _, srcErr := tx.Exec(r.Context(), `UPDATE payments SET square_source_id = $1 WHERE id = $2`, sourceID, paymentID); srcErr != nil {
// Reused pending row. The stored square_request_snapshot is the FIRST
// attempt's charge body and MUST remain immutable across nonce-changing
// retries: if that original charge actually landed at Square (the row
// is pending only because the post-charge outcome is unknown), the
// by-key sweep replay must match the original body so Square's
// idempotency dedup returns the landed payment and the sweep rescues
// the row. Overwriting the snapshot's SourceID with this retry's fresh
// nonce — or refreshing the square_source_id column the sweep overrides
// the replay source with — would make the sweep replay the NEW source,
// Square would return IDEMPOTENCY_KEY_REUSED, and the landed charge
// would never be rescued (stranded until the 24h blind-fail). A retry
// that changed nonce gets IDEMPOTENCY_KEY_REUSED at charge time; the
// sweep's replay/manual-reconcile path (sweep.go:573-584) resolves the
// row's true state from the immutable first-attempt body instead. The
// column is refreshed ONLY for snapshot-less legacy rows, whose
// fallback replay body is rebuilt from it (and which get a fresh
// snapshot from the post-commit write below).
if _, srcErr := tx.Exec(r.Context(), `UPDATE payments SET square_source_id = $1 WHERE id = $2 AND (square_request_snapshot IS NULL OR square_request_snapshot = '')`, sourceID, paymentID); srcErr != nil {
log.Printf("Failed to update square_source_id on reused payment %s: %v", paymentID, srcErr)
}
}
@@ -1824,10 +1711,13 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
// M1: store the verbatim request JSON so the sweep can replay the charge
// with an IDENTICAL body under the same key — Square compares the whole
// request on key reuse, and a reconstructed body returns
// IDEMPOTENCY_KEY_REUSED, leaving the row pending forever.
// IDEMPOTENCY_KEY_REUSED, leaving the row pending forever. The snapshot is
// written ONLY when the row has none: it records the FIRST attempt's body,
// which stays immutable so a nonce-changing retry can never redirect the
// sweep's replay away from the original charge (see the reuse branch above).
if snap, mErr := json.Marshal(paymentReq); mErr != nil {
log.Printf("Failed to marshal square_request_snapshot for payment %s: %v", paymentID, mErr)
} else if _, sErr := db.Conn.Exec(r.Context(), `UPDATE payments SET square_request_snapshot = $1 WHERE id = $2`, string(snap), paymentID); sErr != nil {
} else if _, sErr := db.Conn.Exec(r.Context(), `UPDATE payments SET square_request_snapshot = $1 WHERE id = $2 AND (square_request_snapshot IS NULL OR square_request_snapshot = '')`, string(snap), paymentID); sErr != nil {
log.Printf("Failed to store square_request_snapshot for payment %s: %v", paymentID, sErr)
}
@@ -2359,7 +2249,7 @@ func CreatePaymentMethod(w http.ResponseWriter, r *http.Request) {
service := NewPaymentService()
card, err := service.CreatePaymentMethodFromToken(r.Context(), userID, req.CardToken)
if err != nil {
if strings.Contains(err.Error(), "invalid") || strings.Contains(err.Error(), "expired") {
if isDefinitiveCardSaveFailure(err) {
log.Printf("Failed to process request: %v", err)
http.Error(w, "Invalid request", http.StatusBadRequest)
return
@@ -2374,6 +2264,32 @@ func CreatePaymentMethod(w http.ResponseWriter, r *http.Request) {
}
}
// isDefinitiveCardSaveFailure reports whether a CreatePaymentMethod error is a
// definitive client rejection — an expired/invalid/already-used card source or
// a declined card that can never be saved — as opposed to an ambiguous
// transport/server failure. It matches the structured Square error Code
// (square.ErrorCode) exactly against the codes this codebase already recognizes
// for card failures (till.go's definitivePaymentDeclineCodes via
// isDefinitiveChargeFailure, plus the card-on-file creation codes SOURCE_USED /
// INVALID_REQUEST_ERROR), replacing the old substring match on "invalid" /
// "expired" in the formatted message so a Square wording change can never
// silently flip the 400↔500 classification. Errors carrying no structured code
// (transport errors, the dev mock's plain errors, 5xx) are ambiguous and stay
// 500 — retrying with the same inputs might succeed.
func isDefinitiveCardSaveFailure(err error) bool {
if err == nil {
return false
}
if isDefinitiveChargeFailure(err) {
return true
}
switch square.ErrorCode(err) {
case "SOURCE_USED", "CARD_TOKEN_USED", "CARD_TOKEN_EXPIRED", "INVALID_CARD", "INVALID_REQUEST_ERROR":
return true
}
return false
}
func RefundPayment(w http.ResponseWriter, r *http.Request) {
// Defense-in-depth: the route is mounted under mw.RequireAdmin, but this
// in-handler check keeps refund access admin-only even if the route is ever
@@ -2523,13 +2439,7 @@ func RefundPayment(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Refund in progress, try again", http.StatusConflict)
return
}
defer func() {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext('crussell:refund:' || $1))
`, refundLockKey); err != nil {
log.Printf("Failed to release refund serialization lock for %s: %v", paymentID, err)
}
}()
defer releasePaymentLock(pinConn, "crussell:refund:"+refundLockKey)
// Dedup/resume (inside the lock): a same-key retry of a completed or
// in-flight (pending) refund must not create a second Square refund. Runs
@@ -3662,11 +3572,24 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
}
ApplyVATToBookingPayment(r.Context(), tx, paymentID)
} else {
// Refresh square_source_id on a reused pending row: this attempt may
// charge a different token than the failed attempt (one-time cnon:
// nonces are spent), and the sweep replays the charge from the stored
// source.
if _, srcErr := tx.Exec(r.Context(), `UPDATE payments SET square_source_id = $1 WHERE id = $2`, sourceID, paymentID); srcErr != nil {
// Reused pending row. The stored square_request_snapshot is the FIRST
// attempt's charge body and MUST remain immutable across nonce-changing
// retries: if that original charge actually landed at Square (the row
// is pending only because the post-charge outcome is unknown), the
// by-key sweep replay must match the original body so Square's
// idempotency dedup returns the landed payment and the sweep rescues
// the row. Overwriting the snapshot's SourceID with this retry's fresh
// nonce — or refreshing the square_source_id column the sweep overrides
// the replay source with — would make the sweep replay the NEW source,
// Square would return IDEMPOTENCY_KEY_REUSED, and the landed charge
// would never be rescued (stranded until the 24h blind-fail). A retry
// that changed nonce gets IDEMPOTENCY_KEY_REUSED at charge time; the
// sweep's replay/manual-reconcile path (sweep.go:573-584) resolves the
// row's true state from the immutable first-attempt body instead. The
// column is refreshed ONLY for snapshot-less legacy rows, whose
// fallback replay body is rebuilt from it (and which get a fresh
// snapshot from the post-commit write below).
if _, srcErr := tx.Exec(r.Context(), `UPDATE payments SET square_source_id = $1 WHERE id = $2 AND (square_request_snapshot IS NULL OR square_request_snapshot = '')`, sourceID, paymentID); srcErr != nil {
log.Printf("Failed to update square_source_id on reused tip payment %s: %v", paymentID, srcErr)
}
}
@@ -3712,10 +3635,13 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
// M1: store the verbatim request JSON so the sweep can replay the charge
// with an IDENTICAL body under the same key — Square compares the whole
// request on key reuse, and a reconstructed body returns
// IDEMPOTENCY_KEY_REUSED, leaving the row pending forever.
// IDEMPOTENCY_KEY_REUSED, leaving the row pending forever. The snapshot is
// written ONLY when the row has none: it records the FIRST attempt's body,
// which stays immutable so a nonce-changing retry can never redirect the
// sweep's replay away from the original charge (see the reuse branch above).
if snap, mErr := json.Marshal(paymentReq); mErr != nil {
log.Printf("Failed to marshal square_request_snapshot for tip payment %s: %v", paymentID, mErr)
} else if _, sErr := db.Conn.Exec(r.Context(), `UPDATE payments SET square_request_snapshot = $1 WHERE id = $2`, string(snap), paymentID); sErr != nil {
} else if _, sErr := db.Conn.Exec(r.Context(), `UPDATE payments SET square_request_snapshot = $1 WHERE id = $2 AND (square_request_snapshot IS NULL OR square_request_snapshot = '')`, string(snap), paymentID); sErr != nil {
log.Printf("Failed to store square_request_snapshot for tip payment %s: %v", paymentID, sErr)
}
+18
View File
@@ -2,6 +2,7 @@ package payments
import (
"context"
"log"
"time"
"github.com/jackc/pgx/v5"
@@ -39,6 +40,23 @@ func acquireAdvisoryLock(ctx context.Context, conn *pgxpool.Conn, key string) (b
return tryAdvisoryLock(ctx, conn, key, "pg_try_advisory_lock")
}
// releasePaymentLock releases a session advisory lock acquired by
// acquireAdvisoryLock on the SAME pinned pool connection (pg_advisory_unlock
// only releases locks held by the calling session). It is the generic release
// counterpart to acquireAdvisoryLock, and callers defer it immediately after a
// successful acquire so the unlock runs before the deferred pinConn.Release().
// Errors are logged and otherwise ignored — exactly what the inline
// `pg_advisory_unlock(hashtext('crussell:...:' || $1))` blocks it replaces did
// — and the key must be the FULL "crussell:..." string that was hashed at
// acquire time so the two hashtext() calls produce the same lock bigint.
func releasePaymentLock(pinConn *pgxpool.Conn, lockKey string) {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext($1))
`, lockKey); err != nil {
log.Printf("Failed to release payment serialization lock %s: %v", lockKey, err)
}
}
// acquireAdvisoryXactLockBlocking is the transaction-scoped BLOCKING variant
// of acquireAdvisoryLock: it issues `SELECT pg_advisory_xact_lock(...)`
// ONCE and waits for as long as the key is contended — there is no 3s bound.
+1 -8
View File
@@ -1,7 +1,6 @@
package payments
import (
"context"
"crussell/db"
"crussell/internal/validators"
"crussell/mw"
@@ -111,13 +110,7 @@ func ApplyLoyaltyRedemption(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Another payment operation is in progress, try again", http.StatusConflict)
return
}
defer func() {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext('crussell:payment:' || $1))
`, bookingID); err != nil {
log.Printf("Failed to release loyalty redemption serialization lock for %s: %v", bookingID, err)
}
}()
defer releasePaymentLock(pinConn, "crussell:payment:"+bookingID)
// Re-check inside the lock (the checks above ran before acquiring it) so a
// concurrent redemption that completed while we waited is caught.
@@ -0,0 +1,489 @@
//go:build test && dev
package payments
// =============================================================================
// ROUND 10 — gift-card value limits & the admin cancellation surface
// =============================================================================
//
// This file pins the money-safety behaviours added in round 10:
//
// 1. Per-transaction £250 cap on the admin-funded gift-card entry points
// (CreateGiftCard, TopUpGiftCard, TransferGiftCard): an amount of £251
// (25,100 pence) is rejected with 400 before any row is written, while
// exactly £250 (25,000 pence) stays inside the cap.
//
// 2. User daily cap of £500 on online gift-card purchases (BuyGiftCard): the
// day's spend is the sum of the caller's gift_card_transactions 'purchase'
// rows (reference_type 'api' — the signal BuyGiftCard itself writes, see
// giftcard_limits.go userGiftCardSpentToday); an attempt that would cross
// £500 is rejected 400, and the cap is inclusive (exactly £500 is
// allowed). The cap is calendar-day (created_at >= CURRENT_DATE): rolling
// yesterday's signal rows forward resets it.
//
// 3. Admin daily cap of £5,000 on gift-card value created/top-up'd: the day's
// issued value is the sum of the cards the admin created today
// (total_funds_added) plus the admin's same-day 'purchase'/'topup'
// gift_card_transactions audit rows on cards created before today (see
// giftcard_limits.go adminGiftCardValueToday); an operation that would
// cross £5,000 is rejected 400, and the cap is inclusive.
//
// 4. AdminCancelGiftCard (POST /api/admin/gift-cards/cancel, body
// {code, payment_id?}) reuses the 14-day partial-spend cancellation core:
// for a card whose shortfall is verified till spend it refunds ONLY the
// unspent remainder to the original payment method, zeroes + expires the
// card, and records a 'giftcard_cancel' refunds row. Cards outside the
// 14-day window are rejected 400 with no Square call.
//
// MONEY-SAFETY CONTRACT under test: a rejected operation must never write a
// card/transaction/payment row and never call Square; an accepted cancellation
// must issue EXACTLY ONE Square refund and must never leave the card's balance
// spendable on top of the returned money (amount_remaining zeroed + expiry in
// the past, atomically with the refund row resolution).
//
// BUILD DEPENDENCY: main.go already routes POST /admin/gift-cards/cancel to
// AdminCancelGiftCard, so until that handler (and the round-10 limit checks)
// are defined in this package the package cannot compile.
import (
"bytes"
"context"
"database/sql"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"testing"
"time"
"crussell/clock"
"crussell/db"
"crussell/internal/square"
"crussell/mw"
"crussell/testutils"
"crussell/testutils/fixtures"
"crussell/testutils/jwt"
"github.com/go-chi/chi/v5"
"github.com/jackc/pgx/v5"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// =============================================================================
// Round 10 helpers
// =============================================================================
// round10CreateAdmin creates an admin user the way the existing admin gift-card
// tests do (CreateTestUser + account_role update) and returns the user id and a
// role-claim 'admin' token, matching main.go's admin group (mw.RequireAuth +
// mw.RequireAdmin).
func round10CreateAdmin(t *testing.T, ctx context.Context, q db.Querier) (adminID, token string) {
t.Helper()
adminID, err := fixtures.CreateTestUser(q)
require.NoError(t, err)
_, err = q.Exec(ctx, "UPDATE users SET account_role = 'admin' WHERE id = $1", adminID)
require.NoError(t, err)
return adminID, jwt.GenerateTestToken(adminID, "admin")
}
// round10AdminCreateGiftCard POSTs a CreateGiftCard request through the real
// router with mw.RequireAuth + mw.RequireAdmin (mirroring main.go's admin
// group) and the test transaction embedded in the request context.
func round10AdminCreateGiftCard(t *testing.T, ctx context.Context, tx pgx.Tx, token string, amount float64) *httptest.ResponseRecorder {
t.Helper()
body, _ := json.Marshal(CreateGiftCardRequest{Amount: amount})
r := httptest.NewRequest(http.MethodPost, "/api/admin/gift-cards", bytes.NewReader(body))
r.Header.Set("Authorization", "Bearer "+token)
r.Header.Set("Content-Type", "application/json")
r = r.WithContext(db.ContextWithTx(r.Context(), tx))
w := httptest.NewRecorder()
router := chi.NewRouter()
router.Use(mw.RequireAuth)
router.With(mw.RequireAdmin).Post("/api/admin/gift-cards", CreateGiftCard)
router.ServeHTTP(w, r)
return w
}
// round10AdminTopUpGiftCard PUTs a TopUpGiftCard request through the real
// router with the admin middleware stack and the test transaction embedded in
// the request context.
func round10AdminTopUpGiftCard(t *testing.T, ctx context.Context, tx pgx.Tx, token, cardID string, amount float64) *httptest.ResponseRecorder {
t.Helper()
body, _ := json.Marshal(TopUpGiftCardRequest{Amount: amount, PaymentMethod: "cash"})
r := httptest.NewRequest(http.MethodPut, "/api/admin/gift-cards/"+cardID+"/topup", bytes.NewReader(body))
r.Header.Set("Authorization", "Bearer "+token)
r.Header.Set("Content-Type", "application/json")
r = r.WithContext(db.ContextWithTx(r.Context(), tx))
w := httptest.NewRecorder()
router := chi.NewRouter()
router.Use(mw.RequireAuth)
router.With(mw.RequireAdmin).Put("/api/admin/gift-cards/{id}/topup", TopUpGiftCard)
router.ServeHTTP(w, r)
return w
}
// round10AdminTransferGiftCard POSTs a TransferGiftCard request through the
// real router with the admin middleware stack and the test transaction
// embedded in the request context.
func round10AdminTransferGiftCard(t *testing.T, ctx context.Context, tx pgx.Tx, token, fromCardID, toCardID string, amount float64) *httptest.ResponseRecorder {
t.Helper()
body, _ := json.Marshal(TransferGiftCardRequest{ToCardID: toCardID, Amount: amount})
r := httptest.NewRequest(http.MethodPost, "/api/admin/gift-cards/"+fromCardID+"/transfer", bytes.NewReader(body))
r.Header.Set("Authorization", "Bearer "+token)
r.Header.Set("Content-Type", "application/json")
r = r.WithContext(db.ContextWithTx(r.Context(), tx))
w := httptest.NewRecorder()
router := chi.NewRouter()
router.Use(mw.RequireAuth)
router.With(mw.RequireAdmin).Post("/api/admin/gift-cards/{from}/transfer", TransferGiftCard)
router.ServeHTTP(w, r)
return w
}
// round10AdminCancelGiftCard POSTs a gift-card cancellation through the ADMIN
// endpoint (POST /api/admin/gift-cards/cancel) with the admin middleware stack
// (mw.RequireAuth + mw.RequireAdmin, matching main.go) and the test transaction
// embedded in the request context.
func round10AdminCancelGiftCard(t *testing.T, ctx context.Context, tx pgx.Tx, token, code string) *httptest.ResponseRecorder {
t.Helper()
body, _ := json.Marshal(CancelGiftCardRequest{Code: code})
r := httptest.NewRequest(http.MethodPost, "/api/admin/gift-cards/cancel", bytes.NewReader(body))
r.Header.Set("Authorization", "Bearer "+token)
r.Header.Set("Content-Type", "application/json")
r = r.WithContext(db.ContextWithTx(r.Context(), tx))
w := httptest.NewRecorder()
router := chi.NewRouter()
router.Use(mw.RequireAuth)
router.With(mw.RequireAdmin).Post("/api/admin/gift-cards/cancel", AdminCancelGiftCard)
router.ServeHTTP(w, r)
return w
}
// round10BuyGiftCard POSTs an online gift-card purchase for a friend through
// the real BuyGiftCard handler and returns the full response recorder so the
// daily-limit message can be asserted. Mirrors round9BuyGiftCardForFriend but
// keeps the body (that helper returns only the card id and status).
func round10BuyGiftCard(t *testing.T, ctx context.Context, tx pgx.Tx, token string, amount int) *httptest.ResponseRecorder {
t.Helper()
reqBody, _ := json.Marshal(map[string]interface{}{
"amount": amount,
"recipient_type": "friend",
"new_card_token": "cnon:card-nonce-ok",
"idempotency_key": fmt.Sprintf("round10-buy-%d-%d", amount, time.Now().UnixNano()),
})
r := httptest.NewRequest(http.MethodPost, "/user/giftcards/buy", bytes.NewBuffer(reqBody))
r.Header.Set("Authorization", "Bearer "+token)
r.Header.Set("Content-Type", "application/json")
r = r.WithContext(db.ContextWithTx(r.Context(), tx))
w := httptest.NewRecorder()
router := chi.NewRouter()
router.Use(mw.RequireAuth)
router.With(mw.RequireNonGuest).Post("/user/giftcards/buy", BuyGiftCard)
router.ServeHTTP(w, r)
return w
}
// =============================================================================
// 1. £250 per-transaction cap on admin gift-card value entry points
// =============================================================================
// TestRound10_AdminGiftCardTransaction_250Cap_Rejected pins the per-transaction
// £250 cap on the three admin-funded gift-card entry points. For each of
// CreateGiftCard, TopUpGiftCard and TransferGiftCard an amount of £251
// (25,100 pence) must be rejected 400 with a message citing the cap BEFORE any
// value moves (no card created, no top-up applied, no transfer executed), while
// exactly £250 (25,000 pence) stays INSIDE the cap and succeeds. The cap is the
// money-safety ceiling for a single admin-funded gift-card operation; without
// it a mis-keyed admin entry could fund a card beyond the value the salon can
// justify, so the boundary is pinned exactly.
func TestRound10_AdminGiftCardTransaction_250Cap_Rejected(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
adminID, adminToken := round10CreateAdmin(t, ctx, tx)
// Source card funds the top-up and transfer cases; destination receives
// the transfer. Both are plain unredeemed non-inventory cards.
var sourceID, destID string
require.NoError(t, tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by)
VALUES (300.00, 300.00, $1) RETURNING id`, adminID).Scan(&sourceID))
require.NoError(t, tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by)
VALUES (0, 0, $1) RETURNING id`, adminID).Scan(&destID))
cases := []struct {
name string
at func(t *testing.T, amount float64) *httptest.ResponseRecorder
wantSuccess int
}{
{
name: "CreateGiftCard",
at: func(t *testing.T, amount float64) *httptest.ResponseRecorder {
return round10AdminCreateGiftCard(t, ctx, tx.(pgx.Tx), adminToken, amount)
},
wantSuccess: http.StatusCreated,
},
{
name: "TopUpGiftCard",
at: func(t *testing.T, amount float64) *httptest.ResponseRecorder {
return round10AdminTopUpGiftCard(t, ctx, tx.(pgx.Tx), adminToken, sourceID, amount)
},
wantSuccess: http.StatusOK,
},
{
name: "TransferGiftCard",
at: func(t *testing.T, amount float64) *httptest.ResponseRecorder {
return round10AdminTransferGiftCard(t, ctx, tx.(pgx.Tx), adminToken, sourceID, destID, amount)
},
wantSuccess: http.StatusOK,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
// £251 (25,100 pence) — one penny over the £250 per-transaction cap.
w := tc.at(t, 251.00)
require.Equal(t, http.StatusBadRequest, w.Code, "over-cap body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "£250", "the rejection must cite the £250 per-transaction cap")
// Boundary: exactly £250 (25,000 pence) is INSIDE the cap.
wb := tc.at(t, 250.00)
require.Equal(t, tc.wantSuccess, wb.Code, "boundary body: %s", wb.Body.String())
})
}
}
// =============================================================================
// 2. User daily cap of £500 on online gift-card purchases (BuyGiftCard)
// =============================================================================
// TestRound10_UserGiftCardDailyLimit_500 pins the user-facing daily cap: a
// user who has already purchased £500 of online gift cards today cannot buy any
// more — a £50 purchase that would land the day on £550 is rejected 400 with
// the daily-limit message ("You have reached your £500 daily gift-card purchase
// limit"). A user at £450 today can still buy £50, landing the day on EXACTLY
// £500 — pinning the cap as inclusive. (Per-purchase amounts are fixed at
// £10/£20/£50, and the daily gate sits after that amount validation, so the
// over-cap purchase is exercised at the maximum valid amount rather than a
// £100 request, which the amount validation rejects first.) The day's spend
// signal is the caller's gift_card_transactions 'purchase' rows written by
// BuyGiftCard (reference_type 'api'), seeded here via round9SeedGiftCardPurchase.
func TestRound10_UserGiftCardDailyLimit_500(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
// --- Over-cap rejection: £500 already purchased today ---
overUserID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
overToken := jwt.GenerateTestToken(overUserID, "verified_email")
for i := 0; i < 10; i++ {
round9SeedGiftCardPurchase(t, ctx, tx, overUserID, 50.00, 0)
}
// A £50 purchase would take the day to £550 — over the £500 cap.
w := round10BuyGiftCard(t, ctx, tx.(pgx.Tx), overToken, 5000)
require.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "£500", "the rejection must cite the £500 daily cap")
assert.Contains(t, w.Body.String(), "daily", "the rejection must be the daily-limit message")
// --- Inclusive boundary: £450 purchased today, £50 still allowed ---
boundaryUserID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
boundaryToken := jwt.GenerateTestToken(boundaryUserID, "verified_email")
for i := 0; i < 9; i++ {
round9SeedGiftCardPurchase(t, ctx, tx, boundaryUserID, 50.00, 0)
}
// A £50 purchase takes the day to exactly £500 — inside the cap.
wb := round10BuyGiftCard(t, ctx, tx.(pgx.Tx), boundaryToken, 5000)
require.Equal(t, http.StatusCreated, wb.Code, "boundary body: %s", wb.Body.String())
}
// =============================================================================
// 3. Admin daily cap of £5,000 on gift-card value created/top-up'd
// =============================================================================
// TestRound10_AdminGiftCardDailyLimit_5000 pins the admin daily cap: an admin
// who has issued £4,900 of gift-card value today (CreateGiftCard/TopUpGiftCard
// audit rows — reference_type 'api', user_id = the admin) cannot issue another
// £200 (that would land the day on £5,100 — over the £5,000 cap) and is
// rejected 400 with a message citing the cap, while a £100 issue that lands the
// day on EXACTLY £5,000 is accepted, pinning the cap as inclusive. The day's
// issued-value signal is seeded as both the gift-card row and its 'purchase'/
// 'topup' gift_card_transactions rows so whichever query the limit code uses
// sees £4,900.
func TestRound10_AdminGiftCardDailyLimit_5000(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
adminID, adminToken := round10CreateAdmin(t, ctx, tx)
// £4,900 of admin-issued gift-card value today: one card plus the audit
// rows CreateGiftCard/TopUpGiftCard write (transaction_type 'purchase'/
// 'topup', reference_type 'api', user_id = the admin), both created today.
var cardID string
require.NoError(t, tx.QueryRow(ctx, `
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by)
VALUES (4900.00, 4900.00, $1) RETURNING id`, adminID).Scan(&cardID))
_, err := tx.Exec(ctx, `
INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes, created_at)
VALUES ($1, 'purchase', 2400.00, 'api', NULL, $2, 'seeded daily signal', NOW())`, cardID, adminID)
require.NoError(t, err)
_, err = tx.Exec(ctx, `
INSERT INTO gift_card_transactions (gift_card_id, transaction_type, amount, reference_type, reference_id, user_id, notes, created_at)
VALUES ($1, 'topup', 2500.00, 'api', NULL, $2, 'seeded daily signal', NOW())`, cardID, adminID)
require.NoError(t, err)
// A £200 creation would take the day to £5,100 — over the £5,000 cap.
// (£200 is also inside the £250 per-transaction cap, isolating the daily gate.)
w := round10AdminCreateGiftCard(t, ctx, tx.(pgx.Tx), adminToken, 200.00)
require.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "£5,000", "the rejection must cite the £5,000 daily cap")
// A £100 creation takes the day to exactly £5,000 — inside the cap.
wb := round10AdminCreateGiftCard(t, ctx, tx.(pgx.Tx), adminToken, 100.00)
require.Equal(t, http.StatusCreated, wb.Code, "boundary body: %s", wb.Body.String())
}
// =============================================================================
// 4. Admin cancellation reuses the 14-day partial-spend core
// =============================================================================
// TestRound10_AdminCancelGiftCard_PartiallySpent_RefundsRemaining pins the
// admin cancellation surface's handling of partial spend (CCR 2013 reg 34(9)):
// a £50 online purchase whose balance was genuinely spent down to £30 at the
// till (a completed giftcard payment row carrying the card id) is cancelled via
// POST /api/admin/gift-cards/cancel as an admin → 200, Square refunds EXACTLY
// once for the unspent remainder (3,000 pence), the card is neutralized (zeroed
// + expired so the refunded value can never be spent on top of the returned
// money), and the refunds row carries the 'giftcard_cancel' origin at the
// unspent amount. This proves the admin surface exercises the same
// partial-spend money path as the customer-facing flow.
func TestRound10_AdminCancelGiftCard_PartiallySpent_RefundsRemaining(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
_, adminToken := round10CreateAdmin(t, ctx, tx)
origClient := SquareClient
mock := square.NewDevClient().(*square.MockClient)
counting := &countingRefundClient{SquareClient: mock}
SquareClient = counting
defer func() { SquareClient = origClient }()
// £50 online purchase, £20 genuinely spent at the till (a completed
// giftcard payment row carrying the card id), £30 remaining.
cardID, paymentID := round9SeedGiftCardPurchase(t, ctx, tx, userID, 50.00, 0)
_, err = tx.Exec(ctx, `
UPDATE gift_cards SET amount_remaining = 30.00 WHERE id = $1`, cardID)
require.NoError(t, err)
_, err = tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, status, amount, idempotency_key, created_by, created_at, updated_at, gift_card_id)
VALUES (NULL, 'full', 'giftcard', 'completed', 20.00, 'r10-spend-' || $1::text, $2, NOW(), NOW(), $1)`, cardID, userID)
require.NoError(t, err)
t.Cleanup(func() {
_, _ = db.Conn.Exec(context.Background(), `DELETE FROM payments WHERE idempotency_key = 'r10-spend-' || $1`, cardID)
})
w := round10AdminCancelGiftCard(t, ctx, tx.(pgx.Tx), adminToken, cardID)
require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "£30.00", "the message must state the refunded unspent portion")
assert.Contains(t, w.Body.String(), "£20.00", "the message must state the non-refundable spent portion")
// Exactly ONE Square refund, for the UNSPENT remainder (3000 pence).
calls := counting.refundCalls()
require.Len(t, calls, 1, "exactly one Square refund for the admin cancellation")
assert.Equal(t, int64(3000), calls[0].Amount, "the unspent remainder must be refunded in pence")
// Card neutralized: zero balance, expired (cannot be spent).
var rem float64
var expiry sql.NullTime
require.NoError(t, tx.QueryRow(ctx, `SELECT amount_remaining, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&rem, &expiry))
assert.Equal(t, 0.00, rem, "card balance must be zero after the admin cancellation")
require.True(t, expiry.Valid, "the card must still carry an expiry date")
assert.False(t, expiry.Time.After(clock.Now()), "card expiry must be in the past (neutralized)")
// Refund row recorded at the partial amount with the giftcard_cancel origin.
var refundAmount float64
var refundOrigin string
require.NoError(t, tx.QueryRow(ctx, `
SELECT amount, origin FROM refunds WHERE payment_id = $1`, paymentID).
Scan(&refundAmount, &refundOrigin))
assert.Equal(t, 30.00, refundAmount, "the refunds row must record the unspent remainder")
assert.Equal(t, "giftcard_cancel", refundOrigin, "the refund must carry the gift-card-cancel origin")
}
// TestRound10_AdminCancelGiftCard_NotCancellable_Rejected pins the statutory
// timing gate on the ADMIN cancellation surface: a card purchased outside the
// 14-day cooling-off window (seeded 15 days ago) is rejected 400 with the
// 14-day message and NO Square refund is issued — the cooling-off right is
// time-limited regardless of who invokes it.
func TestRound10_AdminCancelGiftCard_NotCancellable_Rejected(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
_, adminToken := round10CreateAdmin(t, ctx, tx)
cardID, _ := round9SeedGiftCardPurchase(t, ctx, tx, userID, 50.00, 15*24*time.Hour)
origClient := SquareClient
counting := &countingRefundClient{SquareClient: square.NewDevClient()}
SquareClient = counting
defer func() { SquareClient = origClient }()
w := round10AdminCancelGiftCard(t, ctx, tx.(pgx.Tx), adminToken, cardID)
require.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "14-day", "the rejection must cite the 14-day cooling-off window")
require.Empty(t, counting.refundCalls(), "no Square refund for a card outside the 14-day window")
}
// =============================================================================
// 5. The user daily cap resets on the next calendar day
// =============================================================================
// TestRound10_UserDailyLimit_ClearsNextDay pins the daily boundary of the user
// purchase cap: after a user has purchased £500 today (exactly at the cap) a
// further £50 purchase is rejected, but once the seeded purchases' timestamps
// are rolled back to YESTERDAY the same £50 purchase succeeds — proving the cap
// is calendar-day scoped and never counts spend from a previous day. Without
// this, a single heavy day would permanently suppress future purchases (or, if
// the boundary were a rolling window, a purchase at 23:59 would bleed into the
// next day's allowance).
func TestRound10_UserDailyLimit_ClearsNextDay(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
token := jwt.GenerateTestToken(userID, "verified_email")
// £500 of purchases today — exactly at the cap.
for i := 0; i < 10; i++ {
round9SeedGiftCardPurchase(t, ctx, tx, userID, 50.00, 0)
}
// Any further purchase today is over the cap.
w := round10BuyGiftCard(t, ctx, tx.(pgx.Tx), token, 5000)
require.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "£500", "the rejection must cite the £500 daily cap")
// Roll the seeded purchases back to yesterday across every table that
// could carry the daily-spend signal (payments, gift_card_transactions,
// gift_cards) so the day boundary resets regardless of which signal the
// limit code queries.
_, err = tx.Exec(ctx, `
UPDATE payments SET created_at = created_at - INTERVAL '1 day'
WHERE created_by = $1 AND booking_id IS NULL AND payment_method = 'online_square'`, userID)
require.NoError(t, err)
_, err = tx.Exec(ctx, `
UPDATE gift_card_transactions SET created_at = created_at - INTERVAL '1 day'
WHERE user_id = $1 AND reference_type = 'api' AND transaction_type = 'purchase'`, userID)
require.NoError(t, err)
_, err = tx.Exec(ctx, `
UPDATE gift_cards SET created_at = created_at - INTERVAL '1 day'
WHERE created_by = $1`, userID)
require.NoError(t, err)
// The same £50 purchase now succeeds — yesterday's spend does not count
// toward today's cap.
wb := round10BuyGiftCard(t, ctx, tx.(pgx.Tx), token, 5000)
require.Equal(t, http.StatusCreated, wb.Code, "next-day body: %s", wb.Body.String())
}
@@ -0,0 +1,360 @@
//go:build test && dev
package payments
import (
"context"
"database/sql"
"net/http"
"sync"
"testing"
"time"
"crussell/db"
"crussell/internal/square"
"crussell/testutils"
"crussell/testutils/fixtures"
"crussell/testutils/jwt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// =============================================================================
// ROUND 8 — money-safety testing gaps
// =============================================================================
//
// This file pins four behaviors that keep money movements safe:
//
// 1. RefundPayment's manual guard rejects discount/on-the-house ledger rows
// (a discount row is not real money, so refunding it would pay money out
// of nothing) BEFORE any Square refund call or refund row is created.
//
// 2. The sweep's recordUntrackedTerminalPayment splits a COMPLETED terminal
// charge that exceeds the remaining booking balance into deposit/balance/
// tip records, applies per-record VAT via ApplyVATToBookingPayment, and
// completes the now-fully-paid booking.
//
// 3. CreateTerminalPayment always sends AllowTipping: false in the Square
// CreateCheckoutReq — the third leg of the tip double-count fix (the
// frontend embeds the tip in the charge amount, so the terminal must not
// prompt for a second one).
//
// 4. acquireAdvisoryXactLockBlocking (the deliberately-unbounded refund lock)
// blocks a second waiter until the holder's transaction commits — the
// "a refund must never be dropped" rationale for the unbounded wait.
// =============================================================================
// T5 — RefundPayment manual guard rejects discount / on-the-house payments
// =============================================================================
// TestRound8_RefundPayment_DiscountOrOnTheHouse_Rejected pins the T5 manual
// guard: RefundPayment rejects a completed discount/on-the-house payment row
// with 400 and a "Cannot refund a discount or complimentary payment" message,
// BEFORE issuing any Square refund call and BEFORE creating any refund row. A
// discount/on-the-house row is a ledger entry, not real money — the customer
// never paid it, so refunding it would pay money out of nothing. The row is
// seeded WITHOUT a square_payment_id so the rejection can only come from the
// discount guard (the later "Payment has no Square reference" guard would
// fire with a different message if the discount guard were ever removed).
func TestRound8_RefundPayment_DiscountOrOnTheHouse_Rejected(t *testing.T) {
for _, method := range []string{"discount", "on_the_house"} {
t.Run(method, func(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
adminID, err := fixtures.CreateTestAdminUser(tx)
require.NoError(t, err)
_, bookingID, _ := setupTestData(t, ctx, tx)
payID, err := fixtures.CreateTestPayment(tx, bookingID, 50.00, method, "full", "completed")
require.NoError(t, err)
// Swap in a client that records every Square refund call so the
// test can prove the guard fires before any money would move.
origClient := SquareClient
counting := &countingRefundClient{SquareClient: square.NewDevClient()}
SquareClient = counting
defer func() { SquareClient = origClient }()
adminToken := jwt.GenerateTestToken(adminID, "admin")
req := RefundRequest{Amount: 1000, Reason: "round8 guard test"}
w := makePaymentRequest(RefundPayment, "POST", "/api/admin/payments/"+payID+"/refund", req, adminToken, ctx)
require.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "Cannot refund a discount or complimentary payment",
"the message must identify the discount/complimentary rejection")
require.Empty(t, counting.refundCalls(),
"no Square refund call may be issued for a discount/on-the-house payment")
var refundCount int
err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM refunds WHERE payment_id = $1`, payID).Scan(&refundCount)
require.NoError(t, err)
assert.Equal(t, 0, refundCount,
"no refund row may be created for a discount/on-the-house payment")
})
}
}
// =============================================================================
// T6 — recordUntrackedTerminalPayment tip-split / VAT / completion branches
// =============================================================================
// TestRound8_SweepUntrackedTerminal_OverBalance_TipSplit_VAT_CompletesBooking
// pins the T6 money-safety contract of recordUntrackedTerminalPayment: a stale
// "tmp-" terminal checkout that COMPLETED at Square with an amount ABOVE the
// remaining booking balance (£55 on a £50 booking) must be recorded as THREE
// ledger rows (deposit £25 + balance £25 + tip £5), each booking row must get
// its VAT applied through ApplyVATToBookingPayment (the tip record must never
// carry VAT), and the now-fully-paid booking must be transitioned to
// 'completed' via completeFullyPaidBooking. This mirrors the existing
// TestSweepStaleTerminalCheckouts_TmpProvisional_Completed_RecordsPayment but
// exercises the over-balance split that its tipAmount=0 charge never reaches.
func TestRound8_SweepUntrackedTerminal_OverBalance_TipSplit_VAT_CompletesBooking(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
serviceID, err := fixtures.CreateTestService(tx)
require.NoError(t, err)
bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID,
time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
require.NoError(t, err)
// The booking must be in a payable state for the untracked charge to be
// recorded (bookingStatusAllowsCompletedPayment) and completable by
// completeFullyPaidBooking.
if _, err := tx.Exec(ctx, "UPDATE bookings SET status = 'in_progress' WHERE id = $1", bookingID); err != nil {
t.Fatalf("failed to set booking in_progress: %v", err)
}
// VAT-registered so the sweep's per-record ApplyVATToBookingPayment writes
// vat_amount/net_amount on the split booking rows. The update is part of the
// setup tx that is committed below, so the sweep sees it at pool level.
if _, err := tx.Exec(ctx, `UPDATE business_settings SET is_vat_registered = TRUE, default_vat_rate = 20.00`); err != nil {
t.Fatalf("failed to enable VAT registration: %v", err)
}
const tmpID = "tmp-round8-tip-split"
seedStaleProvisionalTerminalCheckout(t, ctx, tx, bookingID, tmpID)
origClient := SquareClient
const sqPayID = "sqp_round8_tip_split"
SquareClient = &provisionalCheckoutClient{
SquareClient: square.NewDevClient(),
checkoutID: tmpID,
// £55 charged on a £50 booking: deposit £25 + balance £25 + tip £5.
result: &square.PaymentResult{Status: "COMPLETED", SquarePayID: sqPayID, Amount: 5500, Fees: 88, CardBrand: "VISA", CardLast4: "4242"},
}
defer func() { SquareClient = origClient }()
pgxTx := db.TxFromContext(ctx)
require.NotNil(t, pgxTx, "no transaction in context")
require.NoError(t, pgxTx.Commit(ctx), "failed to commit setup tx")
pool := context.Background()
t.Cleanup(func() {
_, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE square_payment_id = $1`, sqPayID)
_, _ = db.Conn.Exec(pool, `DELETE FROM bookings WHERE id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM services WHERE id = $1`, serviceID)
_, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, userID)
// Restore the shared business_settings row to the VAT-unregistered
// baseline so parallel tests keep their own VAT expectations.
_, _ = db.Conn.Exec(pool, `UPDATE business_settings SET is_vat_registered = FALSE, default_vat_rate = 20.00`)
})
// Drop any other stale terminal rows left by sequential tests so the count
// is deterministic.
if _, err := db.Conn.Exec(pool, `DELETE FROM terminal_checkouts WHERE status IN ('PENDING', 'IN_PROGRESS') AND checkout_id <> $1`, tmpID); err != nil {
t.Fatalf("failed to clean leftover stale terminal checkouts: %v", err)
}
if _, err := db.Conn.Exec(pool, `DELETE FROM till_sales WHERE status = 'pending' AND square_checkout_id IS NOT NULL`); err != nil {
t.Fatalf("failed to clean leftover stale till sales: %v", err)
}
n, err := SweepStaleTerminalCheckouts(pool)
require.NoError(t, err, "sweep failed")
assert.Equal(t, 1, n, "the COMPLETED over-balance provisional checkout must be resolved by the sweep")
var status string
require.NoError(t, db.Conn.QueryRow(pool, "SELECT status FROM terminal_checkouts WHERE checkout_id = $1", tmpID).Scan(&status))
assert.Equal(t, "COMPLETED", status, "the recorded checkout row must be marked COMPLETED")
// The untracked charge must be split: one £55 Square charge → deposit £25 +
// balance £25 + tip £5 (three ledger rows sharing the square_payment_id).
rows, err := db.Conn.Query(pool, `
SELECT payment_type, amount, is_vat_applicable, vat_amount, net_amount
FROM payments
WHERE booking_id = $1 AND square_payment_id = $2
ORDER BY payment_type
`, bookingID, sqPayID)
require.NoError(t, err, "failed to query recorded split payments")
defer rows.Close()
type splitRow struct {
paymentType string
amount float64
vatApplied bool
vatAmount sql.NullFloat64
netAmount sql.NullFloat64
}
splits := map[string]splitRow{}
for rows.Next() {
var r splitRow
require.NoError(t, rows.Scan(&r.paymentType, &r.amount, &r.vatApplied, &r.vatAmount, &r.netAmount))
splits[r.paymentType] = r
}
require.NoError(t, rows.Err())
require.Len(t, splits, 3, "the over-balance terminal charge must split into deposit + balance + tip records")
assert.InDelta(t, 25.0, splits["deposit"].amount, 0.001, "deposit = 50%% of the £50 booking total")
assert.InDelta(t, 25.0, splits["balance"].amount, 0.001, "balance = the remaining booking total")
assert.InDelta(t, 5.0, splits["tip"].amount, 0.001, "tip = the charged amount above the booking value")
// Per-record VAT (ApplyVATToBookingPayment): the deposit and balance rows
// carry 20% VAT of the £25 gross (£4.17 VAT, £20.83 net); the tip record
// must never have VAT applied.
for _, pt := range []string{"deposit", "balance"} {
r := splits[pt]
assert.True(t, r.vatApplied, "%s record must have VAT applied", pt)
require.True(t, r.vatAmount.Valid, "%s record must have vat_amount set", pt)
assert.InDelta(t, 4.17, r.vatAmount.Float64, 0.001, "%s record VAT (20%% of £25 gross)", pt)
require.True(t, r.netAmount.Valid, "%s record must have net_amount set", pt)
assert.InDelta(t, 20.83, r.netAmount.Float64, 0.001, "%s record net of 20%% VAT", pt)
}
assert.False(t, splits["tip"].vatApplied, "tip record must never have VAT applied")
assert.False(t, splits["tip"].vatAmount.Valid, "tip record must have NULL vat_amount")
assert.False(t, splits["tip"].netAmount.Valid, "tip record must have NULL net_amount")
// The £55 charge covers the full £50 booking (deposit + balance), so
// completeFullyPaidBooking must have transitioned the booking to
// 'completed' — the same completion the poll handler performs.
var bookingStatus string
require.NoError(t, db.Conn.QueryRow(pool, "SELECT status FROM bookings WHERE id = $1", bookingID).Scan(&bookingStatus))
assert.Equal(t, "completed", bookingStatus, "a fully-paid booking must be completed by the sweep")
}
// =============================================================================
// T8 — CreateTerminalPayment sends AllowTipping: false to Square
// =============================================================================
// recordingCheckoutClient records every CreateCheckoutReq so a test can assert
// exactly what the handler sends to Square while delegating the actual call to
// the underlying client (the same recording-client pattern as
// recordingPaymentClient / countingRefundClient in the sibling files).
type recordingCheckoutClient struct {
square.SquareClient
mu sync.Mutex
reqs []square.CreateCheckoutReq
}
func (c *recordingCheckoutClient) CreateCheckout(ctx context.Context, req square.CreateCheckoutReq) (*square.CheckoutResult, error) {
c.mu.Lock()
c.reqs = append(c.reqs, req)
c.mu.Unlock()
return c.SquareClient.CreateCheckout(ctx, req)
}
func (c *recordingCheckoutClient) checkoutReqs() []square.CreateCheckoutReq {
c.mu.Lock()
defer c.mu.Unlock()
return append([]square.CreateCheckoutReq(nil), c.reqs...)
}
// TestRound8_CreateTerminalPayment_AllowTippingFalse pins the T8 leg of the
// tip double-count fix: the frontend embeds the tip in the charge amount
// (totalWithTip), so CreateTerminalPayment must pass AllowTipping: false in
// the Square CreateCheckoutReq even when the client requests TipEnabled —
// otherwise the terminal would prompt for a second tip and the tip would be
// double-counted in production. The request is captured with a recording
// client and asserted verbatim.
func TestRound8_CreateTerminalPayment_AllowTippingFalse(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
_, bookingID, _ := setupTestData(t, ctx, tx)
adminToken := jwt.GenerateAdminToken()
origClient := SquareClient
rec := &recordingCheckoutClient{SquareClient: square.NewDevClient()}
SquareClient = rec
defer func() { SquareClient = origClient }()
handler := CreateTerminalPayment
req := CreateTerminalPaymentRequest{
Amount: 5500,
PaymentType: "full",
TipEnabled: true,
}
w := makePaymentRequest(handler, "POST", "/api/admin/bookings/"+bookingID+"/payment", req, adminToken, ctx)
require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
reqs := rec.checkoutReqs()
require.Len(t, reqs, 1, "exactly one CreateCheckoutReq must be sent to Square")
assert.False(t, reqs[0].AllowTipping,
"AllowTipping must be false even with TipEnabled — the tip is already embedded in the amount")
assert.Equal(t, int64(5500), reqs[0].Amount, "the charge amount (tip embedded) must reach Square verbatim")
assert.Equal(t, bookingID, reqs[0].ReferenceID, "the checkout must be scoped to the booking")
}
// =============================================================================
// T9 — acquireAdvisoryXactLockBlocking blocks waiters until the holder commits
// =============================================================================
// TestRound8_AdvisoryXactLock_BlocksWaiterUntilCommit pins the T9 contract of
// the deliberately-unbounded transaction-scoped refund lock: a second waiter
// on the same "crussell:refund:" key must BLOCK (not time out, not proceed)
// while the holder's transaction is open, and must acquire the lock — returning
// nil — only after the holder commits. This is the "a refund must never be
// dropped" rationale: if the manual RefundPayment holds the key across its
// up-to-30s Square round-trip, a timed-out cancellation would abort and the
// caller would commit a cancellation with ZERO refund rows created (no sweep
// retry is possible because the rows never existed). Uses channels + timeouts
// so the assertion never depends on a sleep; both transactions are rolled back
// when the lock is not acquired.
func TestRound8_AdvisoryXactLock_BlocksWaiterUntilCommit(t *testing.T) {
ctx := context.Background()
key := "crussell:refund:round8-locktest"
// Goroutine A: the holder. Its transaction stays OPEN until we commit it,
// so the lock it holds is never released early.
holderTx, err := db.Conn.Begin(ctx)
require.NoError(t, err, "failed to begin holder tx")
defer func() { _ = holderTx.Rollback(ctx) }()
require.NoError(t, acquireAdvisoryXactLockBlocking(ctx, holderTx, key),
"the uncontended blocking xact lock must be acquired immediately")
// Goroutine B: the waiter. It signals that it has STARTED (its tx is open
// and it is about to issue the blocking acquire) and then reports the
// acquire result on a buffered channel.
started := make(chan struct{})
acquired := make(chan error, 1)
go func() {
waiterTx, err := db.Conn.Begin(ctx)
if err != nil {
acquired <- err
return
}
defer func() { _ = waiterTx.Rollback(ctx) }()
close(started)
acquired <- acquireAdvisoryXactLockBlocking(ctx, waiterTx, key)
}()
<-started
// While the holder's tx is open, the waiter must NOT have returned.
select {
case err := <-acquired:
t.Fatalf("waiter returned %v while the holder tx was still open — the blocking xact lock did not block", err)
case <-time.After(300 * time.Millisecond):
// Expected: the waiter is blocked on the holder's lock.
}
// Release the lock by committing the holder's transaction; the waiter must
// then acquire it and return nil.
require.NoError(t, holderTx.Commit(ctx), "failed to commit holder tx")
select {
case err := <-acquired:
require.NoError(t, err, "the waiter must acquire the lock once the holder commits")
case <-time.After(10 * time.Second):
t.Fatal("waiter never acquired the lock after the holder committed — the blocking xact lock did not release")
}
}
File diff suppressed because it is too large Load Diff
+52 -17
View File
@@ -867,21 +867,13 @@ func processChargeGroup(ctx context.Context, chargeID string, rows []pendingChar
// Give up on this charge (the manual handler may hold the lock). The
// sweep continues to the next charge rather than aborting fatally.
for _, pid := range paymentIDs[:locked] {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext('crussell:refund:' || $1))
`, pid); err != nil {
log.Printf("Failed to release refund lock for payment %s: %v", pid, err)
}
releasePaymentLock(pinConn, "crussell:refund:"+pid)
}
return 0, nil
}
defer func() {
for _, pid := range paymentIDs {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext('crussell:refund:' || $1))
`, pid); err != nil {
log.Printf("Failed to release refund lock for payment %s: %v", pid, err)
}
releasePaymentLock(pinConn, "crussell:refund:"+pid)
}
}()
@@ -1170,8 +1162,57 @@ type manualPendingRow struct {
// re-issued; rows WITHOUT one (the ambiguous-error path) are re-issued with
// their OWN stored idempotency key (Square dedups same-key retries, so the
// retry is idempotent).
//
// A terminal pre-pass first sweeps legacy manual rows whose payment has NO
// square_payment_id (pre-dating the handler's square-less guard at
// handlers.go:2477-2480). Such rows can never be refunded via Square and would
// otherwise stay 'pending' forever, blocking the over-refund guard. They are
// marked 'failed' and surfaced in the admin notification centre, mirroring the
// Square-less cancellation pre-pass (refunds.go:554-583) with a DISTINCT
// origin='manual' filter so the two passes never double-process a row.
func sweepManualPendingSquareRefunds(ctx context.Context) (int, error) {
// (a) Terminal pre-pass: legacy MANUAL card refunds whose payment has no
// Square reference can never be refunded via Square → mark them failed
// so they stop blocking the over-refund guard, and surface the affected
// booking in the admin notification centre for in-person arrangement.
// Mirrors the cancellation Square-less pre-pass (origin='cancellation',
// refunds.go:554-583) with origin='manual' so the filters stay distinct
// and no row is swept by both passes. Must run OUTSIDE any GROUP BY —
// Postgres lumps NULLs together, so these rows can't be handled in the
// per-payment grouping below.
rows, err := db.Conn.Query(ctx, `
UPDATE refunds r SET status = 'failed'
FROM payments p
WHERE p.id = r.payment_id
AND r.status = 'pending' AND r.refund_attempts < 3
AND p.payment_method IN ('online_square', 'in_person_card')
AND p.square_payment_id IS NULL
AND r.origin = 'manual'
RETURNING r.id
`)
if err != nil {
log.Printf("Failed to mark Square-less manual refunds failed: %v", err)
} else {
var failedIDs []string
for rows.Next() {
var id string
if err := rows.Scan(&id); err == nil {
failedIDs = append(failedIDs, id)
}
}
if err := rows.Err(); err != nil {
log.Printf("Failed to iterate Square-less manual refunds during sweep: %v", err)
}
rows.Close()
if len(failedIDs) > 0 {
log.Printf("Marked %d Square-less manual refund(s) failed (in-person arrangement needed)", len(failedIDs))
}
insertRefundFailedNotifications(ctx, failedIDs)
}
// (b) Manual refunds WITH a Square reference — the rows below are the only
// ones the retry/reconcile logic can act on.
rows, err = db.Conn.Query(ctx, `
SELECT r.id, r.payment_id, p.booking_id, r.amount, r.idempotency_key, r.reason,
p.square_payment_id, r.square_refund_id, r.created_at
FROM refunds r
@@ -1292,13 +1333,7 @@ func processManualPaymentGroup(ctx context.Context, paymentID string, rows []man
log.Printf("Refund lock for payment %s not acquired within bound — a manual refund is in progress; leaving rows pending for the next sweep", paymentID)
return 0, nil
}
defer func() {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext('crussell:refund:' || $1))
`, paymentID); err != nil {
log.Printf("Failed to release refund lock for payment %s: %v", paymentID, err)
}
}()
defer releasePaymentLock(pinConn, "crussell:refund:"+paymentID)
ids := make([]string, 0, len(rows))
for _, r := range rows {
+306 -65
View File
@@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"log"
"log/slog"
"math"
"strconv"
"strings"
@@ -61,10 +62,23 @@ const stalePendingPaymentAge = 24 * time.Hour
// trustworthily and fall back to the legacy blind-fail + WARN.
const stalePendingKeyedAge = 22 * time.Hour
// SweepStalePendingPayments resolves stale pending payments and till sales; see the rationale block on stalePendingPaymentAge above.
func SweepStalePendingPayments(ctx context.Context) (int, error) {
cutoff := clock.Now().Add(-stalePendingPaymentAge)
keyedCutoff := clock.Now().Add(-stalePendingKeyedAge)
// Pass 0: stranded Square-less MANUAL refund rows at the 3-attempt cap.
// The Square-less pre-pass inside sweepManualPendingSquareRefunds
// (refunds.go) reconciles only rows with refund_attempts < 3; legacy rows
// that already hit the cap are never reconciled by it and would stay
// 'pending' forever, permanently blocking the over-refund guard. Such rows
// can never be refunded via Square (no square_payment_id), so they are
// marked 'failed' + admin-notified here for in-person arrangement (F10).
sqlessRefundCount, sqlessErr := sweepSquarelessManualRefundsAtAttemptCap(ctx)
if sqlessErr != nil {
log.Printf("Failed to reconcile Square-less manual refunds at the 3-attempt cap: %v", sqlessErr)
}
// Pass 1 (earlier cutoff): rows with a stored idempotency key but NO
// square_payment_id are the lost-response case — the charge may have landed
// at Square with the response lost. They are reconciled at Square by
@@ -97,7 +111,7 @@ func SweepStalePendingPayments(ctx context.Context) (int, error) {
return 0, err
}
total := payKeyedCount + tillKeyedCount + payCount + tillCount
total := sqlessRefundCount + payKeyedCount + tillKeyedCount + payCount + tillCount
payTotal := payKeyedCount + payCount
tillTotal := tillKeyedCount + tillCount
completed := payKeyedCompleted + tillKeyedCompleted + payCompleted + tillCompleted
@@ -143,11 +157,13 @@ type staleRow struct {
// the lost-response case: the sweep replays the key at Square to learn the
// true charge outcome before declaring failure.
IdempotencyKey string
// SquareSourceID is the exact source_id sent in the original CreatePayment
// call, stored on the row so the replay-by-key can rebuild an IDENTICAL
// request body (same key + source + amount). Replaying a different body
// would return IDEMPOTENCY_KEY_REUSED, which proves nothing about whether
// the charge landed.
// SquareSourceID is the row's CURRENT square_source_id — the source_id sent
// in the latest CreatePayment attempt, refreshed whenever a same-key retry
// reuses the pending row. The replay-by-key overrides the stored snapshot's
// embedded source with this value so the replayed body matches the source
// the retained key actually used. Replaying a different body would return
// IDEMPOTENCY_KEY_REUSED, which proves nothing about whether the charge
// landed.
SquareSourceID string
// SquareRequestSnapshot is the verbatim original CreatePayment request JSON
// stored on the row at charge time (square_request_snapshot) — the FULL
@@ -214,6 +230,22 @@ func sweepStaleRows(ctx context.Context, table string, cutoff time.Time) (resolv
leaveGiftCardPurchasePending(ctx, r)
continue
}
// F3: a charge Square reports COMPLETED must never be
// completed on a booking that was cancelled during the pending
// window — the cancellation refund path computes refunds from
// completed payments and would miss it, charging the customer
// with NO automatic refund. Re-read bookings.status (FOR
// UPDATE) and refuse on a cancelled booking. Rows with no
// booking (gift-card purchases, till_sales) are never gated.
if table == "payments" {
switch gateStalePaymentRescueOnBooking(ctx, r) {
case staleBookingGateRefused:
resolved++
continue
case staleBookingGateUnknown:
continue
}
}
if rescueStaleRowCompleted(ctx, table, r.ID) {
resolved++
completed++
@@ -303,6 +335,18 @@ func sweepKeyedStaleRows(ctx context.Context, table string, cutoff time.Time) (r
leaveGiftCardPurchasePending(ctx, r)
continue
}
// F3: same money-safety gate as the by-id rescue above — a charge
// Square reports COMPLETED must never be completed on a booking
// that was cancelled during the pending window.
if table == "payments" {
switch gateStalePaymentRescueOnBooking(ctx, r) {
case staleBookingGateRefused:
resolved++
continue
case staleBookingGateUnknown:
continue
}
}
if rescueKeyedStaleRowCompleted(ctx, table, r.ID, sqPayID) {
resolved++
completed++
@@ -427,8 +471,11 @@ func scanStaleRow(table string, rows pgx.Rows) (staleRow, error) {
// failStaleRow marks one stale pending row 'failed'. Returns true when the row
// was updated (status was still 'pending').
func failStaleRow(ctx context.Context, table, id string) bool {
// table is an internal constant ("payments"/"till_sales"), never user
// input, but the identifier is routed through pgx.Identifier.Sanitize so no
// raw, unquoted table name is ever concatenated into the statement.
tag, err := db.Conn.Exec(ctx, `
UPDATE `+table+` SET status = 'failed', updated_at = NOW()
UPDATE `+pgx.Identifier{table}.Sanitize()+` SET status = 'failed', updated_at = NOW()
WHERE id = $1 AND status = 'pending'
`, id)
if err != nil {
@@ -443,7 +490,7 @@ func failStaleRow(ctx context.Context, table, id string) bool {
// row was updated.
func rescueStaleRowCompleted(ctx context.Context, table, id string) bool {
tag, err := db.Conn.Exec(ctx, `
UPDATE `+table+` SET status = 'completed', updated_at = NOW()
UPDATE `+pgx.Identifier{table}.Sanitize()+` SET status = 'completed', updated_at = NOW()
WHERE id = $1 AND status = 'pending'
`, id)
if err != nil {
@@ -461,7 +508,7 @@ func rescueStaleRowCompleted(ctx context.Context, table, id string) bool {
// true when the row was updated.
func rescueKeyedStaleRowCompleted(ctx context.Context, table, id, squarePaymentID string) bool {
tag, err := db.Conn.Exec(ctx, `
UPDATE `+table+` SET status = 'completed', square_payment_id = $1, updated_at = NOW()
UPDATE `+pgx.Identifier{table}.Sanitize()+` SET status = 'completed', square_payment_id = $1, updated_at = NOW()
WHERE id = $2 AND status = 'pending'
`, squarePaymentID, id)
if err != nil {
@@ -471,6 +518,93 @@ func rescueKeyedStaleRowCompleted(ctx context.Context, table, id, squarePaymentI
return int(tag.RowsAffected()) > 0
}
// staleBookingGate is the outcome of the pre-completion booking-status recheck
// for a stale pending payment row whose charge Square reports COMPLETED.
type staleBookingGate int
const (
// staleBookingGateAllowed — the booking is still payable; the rescue may proceed.
staleBookingGateAllowed staleBookingGate = iota
// staleBookingGateRefused — the booking is cancelled/lapsed/no-show; the row
// was marked FAILED and a critical admin notification raised.
staleBookingGateRefused
// staleBookingGateUnknown — the booking status could not be read; the row was
// left pending (never completed on an unknown state).
staleBookingGateUnknown
)
// gateStalePaymentRescueOnBooking re-reads the booking of a stale pending
// payment row before the sweep rescues it to 'completed' on a Square COMPLETED
// reconcile. It mirrors the live-path guard bookingStatusAllowsCompletedPayment
// (handlers.go): a charge that lands AFTER the booking was cancelled must never
// be silently completed — the cancellation refund path computes refunds from
// completed payments and would miss it, charging a customer for a cancelled
// booking with NO automatic refund (F3).
//
// The booking status is re-read FOR UPDATE inside a transaction so the
// decision serializes against a concurrent cancellation (C5 pattern, mirrors
// recordTerminalPaymentTx). On a cancelled/lapsed/no-show booking the payment
// row is marked FAILED (not completed) and a critical admin notification is
// raised, atomically with the decision, so an operator refunds the customer
// manually. A booking whose status cannot be read is never completed — the row
// is left pending for the next sweep run. Rows with no booking (till_sales;
// gift-card purchases are diverted by the callers before this helper) are
// never gated.
func gateStalePaymentRescueOnBooking(ctx context.Context, r staleRow) staleBookingGate {
if r.BookingID == nil {
return staleBookingGateAllowed
}
tx, err := db.Conn.Begin(ctx)
if err != nil {
log.Printf("CRITICAL: failed to begin booking-status recheck for stale pending payment %s (booking %s): %v — leaving pending — MANUAL RECONCILIATION REQUIRED", r.ID, *r.BookingID, err)
return staleBookingGateUnknown
}
defer func() {
if err := tx.Rollback(ctx); err != nil && !errors.Is(err, pgx.ErrTxClosed) {
log.Printf("Failed to rollback booking-status recheck for stale pending payment %s: %v", r.ID, err)
}
}()
var status string
if err := tx.QueryRow(ctx, `SELECT status FROM bookings WHERE id = $1 FOR UPDATE`, *r.BookingID).Scan(&status); err != nil {
// Booking gone or unreadable: the money state is unknown. Never
// complete on an unknown state — a completed payment on a vanished
// booking would strand the charge outside the refund system.
log.Printf("CRITICAL: Square reports stale pending payment %s COMPLETED but re-reading booking %s status failed (%v) — leaving pending — MANUAL RECONCILIATION REQUIRED", r.ID, *r.BookingID, err)
return staleBookingGateUnknown
}
if bookingStatusAllowsCompletedPayment(status) {
// Payable — release the booking lock; the caller rescues the row.
if cErr := tx.Commit(ctx); cErr != nil {
log.Printf("CRITICAL: failed to commit booking-status recheck for stale pending payment %s (booking %s): %v — leaving pending — MANUAL RECONCILIATION REQUIRED", r.ID, *r.BookingID, cErr)
return staleBookingGateUnknown
}
return staleBookingGateAllowed
}
// Cancelled / lapsed / no-show booking: completing the payment would
// charge a customer for a booking the cancellation flow already closed,
// with NO automatic refund. Mark the row FAILED (not completed) and alert
// ops so an operator refunds the customer manually.
tag, upErr := tx.Exec(ctx, `
UPDATE `+pgx.Identifier{"payments"}.Sanitize()+` SET status = 'failed', updated_at = NOW()
WHERE id = $1 AND status = 'pending'
`, r.ID)
if upErr != nil {
log.Printf("CRITICAL: Square payment for stale pending row %s is COMPLETED but booking %s is %q — marking the row failed errored (%v) — MANUAL RECONCILIATION REQUIRED", r.ID, *r.BookingID, status, upErr)
return staleBookingGateUnknown
}
if cErr := tx.Commit(ctx); cErr != nil {
log.Printf("CRITICAL: Square payment for stale pending row %s is COMPLETED but booking %s is %q — committing the failed mark errored (%v) — MANUAL RECONCILIATION REQUIRED", r.ID, *r.BookingID, status, cErr)
return staleBookingGateUnknown
}
if int(tag.RowsAffected()) > 0 {
insertCriticalPaymentNotification(ctx, r.BookingID, r.CreatedBy)
log.Printf("CRITICAL: stale pending payment %s is COMPLETED at Square but booking %s is %q — payment marked FAILED instead of completed; operator must refund the customer manually", r.ID, *r.BookingID, status)
return staleBookingGateRefused
}
// The row was already resolved concurrently — nothing left to refuse.
return staleBookingGateUnknown
}
// clawbackTillSaleFunding claws back a stale pending till sale's funded gift
// card (atomically with the failed mark) after a reconcile PROVED the charge
// never completed — the funding has no charge behind it. A sale with no gift
@@ -497,7 +631,10 @@ func clawbackTillSaleFunding(ctx context.Context, r staleRow) bool {
// charge made under a stale pending row's idempotency key and returns the
// tri-state result. The replay sends an IDENTICAL body to the original charge:
// the stored square_request_snapshot (the FULL request — source_id, key,
// amount and every field the original carried). Replaying a partial body would
// amount and every field the original carried), with the source_id overridden
// by the row's CURRENT square_source_id column value (authoritative — a
// same-key reuse refreshes the column while the snapshot JSON is stale).
// Replaying a partial body would
// return IDEMPOTENCY_KEY_REUSED for a RETAINED key and strand the row pending
// forever. Square's idempotency guarantee returns the ORIGINAL payment for a
// retained key (never a second charge); a COMPLETED payment rescues the row to
@@ -555,8 +692,34 @@ func reconcileStalePaymentByKey(ctx context.Context, table string, r staleRow) (
}
snapshot = fallback
}
// The replay repeats the stored request snapshot verbatim so Square's
// idempotency dedup returns the original payment for a retained key. The
// The stored snapshot embeds the source_id of the ORIGINAL charge, but a
// pending row REUSED by a same-key retry has its square_source_id column
// refreshed to the retry's source while the snapshot JSON stays stale (the
// write side now refreshes the snapshot too). Replaying the stale embedded
// source under a key Square already retains for the new one would return
// IDEMPOTENCY_KEY_REUSED and strand the row pending — so rebuild the replay
// body with the LIVE column value, which is authoritative (set at charge
// time, refreshed on every reuse). The minimal snapshot-less fallback body
// above is already built from the live source and needs no override. An
// unparseable snapshot must never look like proof of no charge — leave the
// row pending for manual reconciliation.
if !fallbackBody && r.SquareSourceID != "" {
var req square.CreatePaymentReq
if err := json.Unmarshal(snapshot, &req); err != nil {
log.Printf("Stale pending %s reconcile by key: failed to parse stored request snapshot for row %s to override the source_id (%v) — leaving pending", table, r.ID, err)
return staleReconcileLeavePending, ""
}
req.SourceID = r.SquareSourceID
overridden, mErr := json.Marshal(req)
if mErr != nil {
log.Printf("Stale pending %s reconcile by key: failed to rebuild replay body with the current square_source_id for row %s (%v) — leaving pending", table, r.ID, mErr)
return staleReconcileLeavePending, ""
}
snapshot = overridden
}
// The replay repeats the stored request snapshot — with the live source_id
// override above — so Square's idempotency dedup returns the original
// payment for a retained key. The
// sweep resolves its Square environment and location through the same
// helpers the charge-time HTTP client uses (square.SquareEnvironment /
// square.SquareLocationID), so the replay and the charge can never read
@@ -746,6 +909,48 @@ func squareHasCode(err error, codes ...string) bool {
return false
}
// sweepSquarelessManualRefundsAtAttemptCap reconciles Square-less MANUAL refund
// rows that have already hit the 3-attempt cap. The Square-less pre-pass inside
// sweepManualPendingSquareRefunds (refunds.go) filters refund_attempts < 3, so
// a legacy manual refund on a payment with no square_payment_id that reached
// the cap (attempts incremented by pre-guard Square attempts) is never
// reconciled by it and would stay 'pending' forever, permanently blocking the
// over-refund guard (F10). Such rows can never be refunded via Square, so they
// are marked 'failed' and surfaced in the admin notification centre for
// in-person arrangement — the same terminal treatment the pre-pass gives rows
// under the cap. Returns the number of rows marked failed.
func sweepSquarelessManualRefundsAtAttemptCap(ctx context.Context) (int, error) {
rows, err := db.Conn.Query(ctx, `
UPDATE refunds r SET status = 'failed'
FROM payments p
WHERE p.id = r.payment_id
AND r.status = 'pending' AND r.refund_attempts >= 3
AND p.payment_method IN ('online_square', 'in_person_card')
AND p.square_payment_id IS NULL
AND r.origin = 'manual'
RETURNING r.id
`)
if err != nil {
return 0, err
}
defer rows.Close()
var failedIDs []string
for rows.Next() {
var id string
if err := rows.Scan(&id); err == nil {
failedIDs = append(failedIDs, id)
}
}
if err := rows.Err(); err != nil {
return 0, err
}
if len(failedIDs) > 0 {
log.Printf("Marked %d Square-less manual refund(s) at the 3-attempt cap failed (in-person arrangement needed)", len(failedIDs))
}
insertRefundFailedNotifications(ctx, failedIDs)
return len(failedIDs), nil
}
// staleTerminalCheckoutAge is how old a still-pending terminal checkout must
// be before the sweep cancels it. Terminal checkouts normally complete within
// minutes; an hour is far past any legitimate card-reader interaction while
@@ -1049,17 +1254,13 @@ func markTerminalCheckoutRowFailed(ctx context.Context, r staleTerminalCheckoutR
// recordUntrackedTerminalPayment records the payments row(s) for a terminal
// checkout that COMPLETED at Square but was never polled/recorded, then marks
// the terminal_checkouts row COMPLETED. The stale sweep otherwise leaves a real
// charge with NO payments row — invisible to refunds and TotalPaid (H4). It
// reuses the exact insert convention of GetCheckoutStatus: the same advisory
// lock key (serializes against a concurrent poll), the same dedup by
// booking_id + square_payment_id, the same PaymentRecord shape and derived
// idempotency key, and the same deposit/balance/tip split for a charge above
// the remaining booking value. It also mirrors GetCheckoutStatus's booking
// re-check: a booking that moved out of a payable state (cancelled / lapsed /
// no-show) after the charge completed refuses to record the payment, marks the
// checkout failed, and inserts a critical-payment admin notification so the
// owner is told a charge landed on a cancelled booking and a manual refund is
// required. Returns true when the checkout row was resolved (payment recorded,
// charge with NO payments row — invisible to refunds and TotalPaid (H4). The
// money-recording work lives in the SHARED recordTerminalPaymentTx core (also
// used by the GetCheckoutStatus poll handler), so both writers run the same
// dedup / FOR UPDATE recheck / insert / M4 split / VAT / guarded checkout
// update; this wrapper supplies the sweep-specific pieces: the advisory lock,
// the cancelled-booking critical admin notification and the CRITICAL completion
// log. Returns true when the checkout row was resolved (payment recorded,
// already recorded, or refused on a cancelled booking); false when recording
// failed (the row is left pending so the next sweep re-runs the whole
// reconcile).
@@ -1068,7 +1269,6 @@ func recordUntrackedTerminalPayment(ctx context.Context, checkoutID, bookingID s
log.Printf("CRITICAL: terminal checkout %s is COMPLETED at Square but carries no Square payment ID — cannot record the payment — MANUAL RECONCILIATION REQUIRED", checkoutID)
return false
}
service := NewPaymentService()
// Serialize with the poll handler (GetCheckoutStatus): both record the same
// Square payment, so the advisory lock + dedup SELECT prevent a double
@@ -1088,11 +1288,7 @@ func recordUntrackedTerminalPayment(ctx context.Context, checkoutID, bookingID s
log.Printf("Terminal-completion serialization lock for %s not acquired within bound — a poll is already recording this checkout", pr.SquarePayID)
return false
}
defer func() {
if _, err := pinConn.Exec(context.Background(), `SELECT pg_advisory_unlock(hashtext('crussell:terminal:' || $1))`, pr.SquarePayID); err != nil {
log.Printf("Failed to release terminal-completion serialization lock for %s: %v", pr.SquarePayID, err)
}
}()
defer releasePaymentLock(pinConn, "crussell:terminal:"+pr.SquarePayID)
tx, err := db.Conn.Begin(ctx)
if err != nil {
@@ -1105,8 +1301,56 @@ func recordUntrackedTerminalPayment(ctx context.Context, checkoutID, bookingID s
}
}()
paymentID, recErr := recordTerminalPaymentTx(ctx, tx, checkoutID, bookingID, pr)
if recErr != nil {
if errors.Is(recErr, errTerminalBookingNotPayable) {
// Sweep-only behaviour: the poll surfaces a 409 to a live caller,
// but a sweep has no one to tell, so alert ops in the admin
// notification centre that money was taken at Square and MUST be
// refunded manually (the core already committed the checkout's
// 'failed' mark).
insertCriticalPaymentNotification(ctx, &bookingID, nil)
return true
}
// The core logged the failure; the row is left pending so the next
// sweep run re-runs the whole reconcile.
return false
}
log.Printf("CRITICAL: recorded untracked terminal charge %s (booking %s) from stale checkout %s — payment row %s created (never polled by the frontend)", pr.SquarePayID, bookingID, checkoutID, paymentID)
return true
}
// errTerminalBookingNotPayable is returned by recordTerminalPaymentTx when the
// booking moved out of a payable state (cancelled / lapsed / no-show) between
// the terminal charge completing at Square and the record attempt. The core has
// ALREADY committed the terminal_checkouts 'failed' mark (and a non-payable
// commit failure is wrapped with this sentinel too), so the caller must not
// roll back: the poll surfaces a 409 conflict, the sweep inserts the critical
// admin notification and resolves the row.
var errTerminalBookingNotPayable = errors.New("booking is no longer payable for a terminal payment")
// recordTerminalPaymentTx records a COMPLETED terminal checkout as payments
// rows, applying the M4 tip split, per-record VAT and booking completion. Both
// the GetCheckoutStatus poll handler and the stale-terminal sweep call it so
// the money-recording logic exists exactly once.
//
// CALL-SITE CONTRACT: both call sites must keep passing the SAME inputs — the
// caller's transaction, the terminal_checkouts checkout_id, the booking id and
// the *square.PaymentResult returned by GetCheckout — and each must acquire the
// advisory lock on "crussell:terminal:<SquarePayID>" on its own pinned pool
// connection BEFORE calling (the core runs inside the caller's transaction).
// The core commits the transaction and completes a now-fully-paid booking; the
// caller owns only the error mapping (errTerminalBookingNotPayable vs generic),
// the HTTP response / sweep return value and any post-commit behaviour (e.g.
// the sweep's critical notification).
func recordTerminalPaymentTx(ctx context.Context, tx pgx.Tx, checkoutID, bookingID string, pr *square.PaymentResult) (string, error) {
service := NewPaymentService()
// Dedup by Square payment ID: a concurrent poll (or a prior sweep run)
// already recorded this charge — just release the in-flight guard.
// already recorded this charge — release the in-flight guard and return the
// existing row so the caller reports the same payment id. The checkout is
// marked COMPLETED under the same guarded status predicate used everywhere
// else, so a row already resolved to 'failed' is never resurrected.
var existingID string
if err := tx.QueryRow(ctx, `
SELECT id FROM payments
@@ -1116,52 +1360,50 @@ func recordUntrackedTerminalPayment(ctx context.Context, checkoutID, bookingID s
UPDATE terminal_checkouts SET status = 'COMPLETED', updated_at = NOW()
WHERE checkout_id = $1 AND status IN ('PENDING', 'IN_PROGRESS')
`, checkoutID); upErr != nil {
log.Printf("Failed to mark terminal checkout %s completed: %v", checkoutID, upErr)
return false
slog.Error("Failed to mark terminal checkout completed on dedup", "checkout_id", checkoutID, "err", upErr)
return "", fmt.Errorf("mark terminal checkout %s completed: %w", checkoutID, upErr)
}
if cErr := tx.Commit(ctx); cErr != nil {
log.Printf("Failed to commit terminal-completion transaction: %v", cErr)
return false
slog.Error("Failed to commit terminal-completion transaction", "checkout_id", checkoutID, "err", cErr)
return "", cErr
}
return true
return existingID, nil
} else if !errors.Is(err, pgx.ErrNoRows) {
log.Printf("Failed to check for existing terminal payment: %v", err)
return false
slog.Error("Failed to check for existing terminal payment", "checkout_id", checkoutID, "booking_id", bookingID, "square_payment_id", pr.SquarePayID, "err", err)
return "", err
}
// Re-check the booking status under the advisory lock (mirrors
// GetCheckoutStatus): a cancellation/eviction that committed between the
// terminal charge completing at Square and this sweep recording it must
// not produce a completed payment on a cancelled/lapsed/no-show booking —
// the cancellation refund path computes refunds from completed payments
// and would silently exclude this charge. Mark the checkout failed and
// alert ops: money was taken at Square and MUST be refunded manually.
// terminal charge completing at Square and this record attempt must not
// produce a completed payment on a cancelled/lapsed/no-show booking — the
// cancellation refund path computes refunds from completed payments and
// would silently exclude this charge. Mark the checkout failed and alert
// ops: money was taken at Square and MUST be refunded manually.
var recheckStatus string
// FOR UPDATE (C5): serializes against the cancellation path's lock on the
// same row so a concurrent cancellation cannot commit between this recheck
// and the transaction commit below.
if err := tx.QueryRow(ctx, `SELECT status FROM bookings WHERE id = $1 FOR UPDATE`, bookingID).Scan(&recheckStatus); err != nil {
log.Printf("CRITICAL: Square payment %s for checkout %s was processed but re-reading booking %s status failed: %v — manual reconciliation required",
pr.SquarePayID, checkoutID, bookingID, err)
return false
slog.Error("CRITICAL: Square payment was processed but re-reading booking status failed — manual reconciliation required", "square_payment_id", pr.SquarePayID, "checkout_id", checkoutID, "booking_id", bookingID, "err", err)
return "", err
}
if !bookingStatusAllowsCompletedPayment(recheckStatus) {
log.Printf("CRITICAL: Square payment %s for checkout %s was processed but booking %s is now %q — marking checkout failed; money taken at Square MUST be refunded manually",
pr.SquarePayID, checkoutID, bookingID, recheckStatus)
slog.Error("CRITICAL: Square payment was processed but booking is no longer payable — marking checkout failed; money taken at Square MUST be refunded manually", "square_payment_id", pr.SquarePayID, "checkout_id", checkoutID, "booking_id", bookingID, "status", recheckStatus)
if _, upErr := tx.Exec(ctx, `
UPDATE terminal_checkouts SET status = 'failed', updated_at = NOW()
WHERE checkout_id = $1 AND status IN ('PENDING', 'IN_PROGRESS')
`, checkoutID); upErr != nil {
log.Printf("CRITICAL: Square payment %s landed on %q booking %s but marking checkout %s failed errored: %v — manual reconciliation required",
pr.SquarePayID, recheckStatus, bookingID, checkoutID, upErr)
slog.Error("CRITICAL: Square payment landed on a non-payable booking but marking checkout failed errored — manual reconciliation required", "square_payment_id", pr.SquarePayID, "status", recheckStatus, "booking_id", bookingID, "checkout_id", checkoutID, "err", upErr)
}
if cErr := tx.Commit(ctx); cErr != nil {
log.Printf("CRITICAL: Square payment %s landed on %q booking %s and committing the checkout-failed mark errored: %v — manual reconciliation required",
pr.SquarePayID, recheckStatus, bookingID, cErr)
return false
slog.Error("CRITICAL: Square payment landed on a non-payable booking and committing the checkout-failed mark errored — manual reconciliation required", "square_payment_id", pr.SquarePayID, "status", recheckStatus, "booking_id", bookingID, "checkout_id", checkoutID, "err", cErr)
// Keep the sentinel wrapped so BOTH callers still surface the
// conflict path (poll 409 / sweep notification) even when the
// failed-mark commit itself failed.
return "", fmt.Errorf("%w: %v", errTerminalBookingNotPayable, cErr)
}
insertCriticalPaymentNotification(ctx, &bookingID, nil)
return true
return "", errTerminalBookingNotPayable
}
// The payment type the admin charged is recorded on the checkout row by
@@ -1171,7 +1413,7 @@ func recordUntrackedTerminalPayment(ctx context.Context, checkoutID, bookingID s
SELECT payment_type FROM terminal_checkouts WHERE checkout_id = $1
`, checkoutID).Scan(&checkoutPaymentType); err != nil {
if !errors.Is(err, pgx.ErrNoRows) {
log.Printf("Failed to read payment type for checkout %s: %v", checkoutID, err)
slog.Error("Failed to read payment type for checkout", "checkout_id", checkoutID, "err", err)
}
checkoutPaymentType = "full"
}
@@ -1211,15 +1453,15 @@ func recordUntrackedTerminalPayment(ctx context.Context, checkoutID, bookingID s
primary := records[0]
paymentID, err := service.CreatePaymentRecordTx(ctx, tx, primary, nil)
if err != nil {
log.Printf("Failed to create payment record for untracked terminal charge %s: %v", pr.SquarePayID, err)
return false
slog.Error("Failed to create payment record for terminal charge", "square_payment_id", pr.SquarePayID, "err", err)
return "", err
}
ApplyVATToBookingPayment(ctx, tx, paymentID)
for _, rec := range records[1:] {
pid, cErr := service.CreatePaymentRecordTx(ctx, tx, rec, nil)
if cErr != nil {
log.Printf("Failed to create terminal tip split record: %v", cErr)
return false
slog.Error("Failed to create terminal tip split record", "square_payment_id", pr.SquarePayID, "err", cErr)
return "", cErr
}
ApplyVATToBookingPayment(ctx, tx, pid)
}
@@ -1229,18 +1471,17 @@ func recordUntrackedTerminalPayment(ctx context.Context, checkoutID, bookingID s
UPDATE terminal_checkouts SET status = 'COMPLETED', updated_at = NOW()
WHERE checkout_id = $1 AND status IN ('PENDING', 'IN_PROGRESS')
`, checkoutID); err != nil {
log.Printf("Failed to mark terminal checkout %s completed: %v", checkoutID, err)
return false
slog.Error("Failed to mark terminal checkout completed", "checkout_id", checkoutID, "err", err)
return "", err
}
if err := tx.Commit(ctx); err != nil {
log.Printf("Failed to commit terminal-completion transaction: %v", err)
return false
slog.Error("Failed to commit terminal-completion transaction", "checkout_id", checkoutID, "err", err)
return "", err
}
// The booking may now be fully paid — complete it like the poll handler does.
completeFullyPaidBooking(ctx, bookingID)
log.Printf("CRITICAL: recorded untracked terminal charge %s (booking %s) from stale checkout %s — payment row %s created (never polled by the frontend)", pr.SquarePayID, bookingID, checkoutID, paymentID)
return true
return paymentID, nil
}
// recordUntrackedTillSalePayment records a stale card-machine till sale whose
+302 -57
View File
@@ -2,8 +2,9 @@ package payments
import (
"context"
"crypto/rand"
"crypto/sha256"
"database/sql"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
@@ -11,6 +12,7 @@ import (
"log/slog"
"math"
"net/http"
"strconv"
"strings"
"crussell/db"
@@ -30,10 +32,14 @@ type TillSaleRequest struct {
PaymentMethod string `json:"payment_method" validate:"required"`
UserSavedCardID *string `json:"user_saved_card_id,omitempty"`
UserID *string `json:"user_id,omitempty"`
// IdempotencyKey is optional; an empty key is replaced with a fresh
// uniqueChargeKey below. Limit 64: Square's terminal-checkout cap — this
// key feeds CreateCheckout AND the CreatePayment paths.
IdempotencyKey string `json:"idempotency_key,omitempty" validate:"omitempty,max=64"`
// IdempotencyKey is optional; an empty key is replaced with a DETERMINISTIC
// fallback derived from the canonical request fields
// (deriveTillIdempotencyKey) so a lost-response retry re-derives the SAME
// key and reuses the pending till_sale row instead of minting a second
// Square charge. Limit 45: this key feeds CreatePayment (Square's /v2/
// payments cap) as well as CreateCheckout, which allows 64 — the stricter
// 45 applies because the same key is replayed to /v2/payments.
IdempotencyKey string `json:"idempotency_key,omitempty" validate:"omitempty,max=45"`
CardToken string `json:"card_token,omitempty"`
RedeemToUserID *string `json:"redeem_to_user_id,omitempty"`
VerificationToken *string `json:"verification_token,omitempty"`
@@ -49,9 +55,12 @@ type TillSaleResponse struct {
CheckoutID *string `json:"checkout_id,omitempty"`
}
// uniqueChargeKey is defined in handlers.go (the till fallback key was
// byte-identical to the tip fallback except the prefix — see the consolidated
// helper).
// uniqueChargeKey (handlers.go) remains the random fallback for the TIP flow,
// where two identical no-key tips are distinct operations that must diverge.
// The till no-key fallback deliberately does NOT use it: a random key would
// mint a second Square charge (and second gift-card funding) when a lost-
// response create is retried. deriveTillIdempotencyKey (below) derives a
// request-stable key instead.
// definitivePaymentDeclineCodes are Square payment error codes meaning the
// card charge can never succeed (declined / expired / not supported). They are
@@ -114,6 +123,134 @@ func declineCodeListContains(s string) bool {
return false
}
// deriveTillIdempotencyKey returns the deterministic no-client-key fallback
// BASE idempotency key for a till sale: "till-" + sha256 over the canonical
// request fields (action, created_by admin, amount in pence, and the gift card
// / user / saved card / redeem targets when present), truncated to 16 bytes so
// the key stays within Square's 45-char /v2/payments limit.
//
// A lost-response retry re-derives the SAME base key, so the idempotency lookup
// in CreateTillSale reuses the pending till_sale row and Square dedups on the
// key — ONE charge and ONE gift-card funding instead of the old uniqueChargeKey
// fallback's second charge on retry. The card nonce (req.CardToken) is
// deliberately EXCLUDED — it changes between retries — and no random value
// enters the base key, so identical logical sales always collide on the SAME
// base key; the handler resolves that collision through the slot scan
// (scanTillIdempotencyKeySlot) run under the advisory lock, which diverges
// genuinely DISTINCT identical keyless sales onto distinct final keys (F5)
// while a lost-response retry of a pending sale keeps re-deriving the same
// candidate and reuses the pending row.
func deriveTillIdempotencyKey(req TillSaleRequest, adminID string) string {
var sb strings.Builder
sb.WriteString("till:")
sb.WriteString(req.Action)
sb.WriteString(":")
sb.WriteString(adminID)
sb.WriteString(":")
sb.WriteString(strconv.FormatInt(int64(math.Round(req.Amount*100)), 10))
if req.GiftCardID != nil && *req.GiftCardID != "" {
sb.WriteString(":gc:")
sb.WriteString(validators.NormalizeGiftCardCode(*req.GiftCardID))
}
if req.UserID != nil && *req.UserID != "" {
sb.WriteString(":user:")
sb.WriteString(*req.UserID)
}
if req.UserSavedCardID != nil && *req.UserSavedCardID != "" {
sb.WriteString(":card:")
sb.WriteString(*req.UserSavedCardID)
}
if req.RedeemToUserID != nil && *req.RedeemToUserID != "" {
sb.WriteString(":redeem:")
sb.WriteString(*req.RedeemToUserID)
}
sum := sha256.Sum256([]byte(sb.String()))
return "till-" + hex.EncodeToString(sum[:16])
}
// tillIdempotencyKeyCandidate appends the slot-sequence suffix to a derived
// base key, hashing back under Square's 45-char /v2/payments limit when the
// verbatim form would overflow (the hash stays deterministic).
func tillIdempotencyKeyCandidate(baseKey string, seq int) string {
if seq == 0 {
return baseKey
}
candidate := fmt.Sprintf("%s-%d", baseKey, seq)
if len(candidate) > 45 {
sum := sha256.Sum256([]byte(candidate))
return "till-" + hex.EncodeToString(sum[:16])
}
return candidate
}
// scanTillIdempotencyKeySlot resolves the FINAL deterministic idempotency key
// for a keyless Square-method till sale, mirroring the gift-card slot pattern
// (deriveGiftCardIdempotencyKey). Must be called under the crussell:till
// advisory lock so the scan-and-insert races no concurrent identical create.
//
// A COMPLETED (or swept/declined FAILED) sale occupies its candidate slot and
// forces the NEXT candidate — two genuine identical keyless sales (e.g. two
// £20 walk-in card creations with no user) must diverge onto distinct keys, or
// the second is silently swallowed by the first's dedup and its customer gets
// NO gift card (F5). A PENDING sale never occupies a slot: a lost-response
// retry re-derives the same candidate, the idempotency lookup below reuses the
// pending row and adopts its STORED key, and Square dedups the charge onto the
// original — ONE charge, ONE gift-card funding.
func scanTillIdempotencyKeySlot(ctx context.Context, baseKey string) (string, error) {
for seq := 0; ; seq++ {
candidate := tillIdempotencyKeyCandidate(baseKey, seq)
var status string
err := db.Conn.QueryRow(ctx, `SELECT status FROM till_sales WHERE idempotency_key = $1`, candidate).Scan(&status)
if errors.Is(err, pgx.ErrNoRows) {
return candidate, nil
}
if err != nil {
return "", fmt.Errorf("failed to scan till idempotency-key slot for %s: %w", baseKey, err)
}
if status == "completed" || status == "failed" {
continue // occupied slot — a distinct sale must diverge onto a fresh key.
}
// Pending (or any other state): the same logical sale is in flight — a
// lost-response retry must reuse it, so keep this candidate.
return candidate, nil
}
}
// refreshTillSnapshotSource rewrites the source_id field inside the stored
// square_request_snapshot JSON on a reused pending till sale so the snapshot
// stays consistent with the square_source_id column refreshed for the new
// charge attempt. The stale-pending sweep replays the snapshot VERBATIM as the
// charge body (sweep.go reconcileStalePaymentByKey); a snapshot whose source
// differs from the row's source would make Square return IDEMPOTENCY_KEY_REUSED
// for the retained key and strand the row pending forever. Runs inside the same
// transaction as the square_source_id refresh so the two columns never diverge.
// A row without a stored snapshot (or an unparseable one) is left untouched —
// the charge attempt below re-marshals a fresh full body before calling Square.
func refreshTillSnapshotSource(ctx context.Context, tx pgx.Tx, tillSaleID, newSource string) {
var snap sql.NullString
if err := tx.QueryRow(ctx, `SELECT square_request_snapshot FROM till_sales WHERE id = $1`, tillSaleID).Scan(&snap); err != nil {
log.Printf("Failed to read square_request_snapshot for reused till sale %s: %v", tillSaleID, err)
return
}
if !snap.Valid || snap.String == "" {
return
}
var req square.CreatePaymentReq
if err := json.Unmarshal([]byte(snap.String), &req); err != nil {
log.Printf("Failed to parse square_request_snapshot for reused till sale %s: %v", tillSaleID, err)
return
}
req.SourceID = newSource
updated, err := json.Marshal(req)
if err != nil {
log.Printf("Failed to re-marshal square_request_snapshot for reused till sale %s: %v", tillSaleID, err)
return
}
if _, err := tx.Exec(ctx, `UPDATE till_sales SET square_request_snapshot = $1 WHERE id = $2`, string(updated), tillSaleID); err != nil {
log.Printf("Failed to refresh square_request_snapshot for reused till sale %s: %v", tillSaleID, err)
}
}
// errTillSaleNotPending: the claim-first gating UPDATE matched zero rows, so
// the sale is no longer 'pending' and the gift card must be left untouched.
var errTillSaleNotPending = errors.New("till sale is not pending")
@@ -126,6 +263,11 @@ func revertGiftCardFunding(ctx context.Context, action, giftCardID string, amoun
return RevertGiftCardFunding(ctx, action, giftCardID, amount, redeemToUserID, tillSaleID)
}
// maxTillGiftCardAmountPence caps till gift-card creates/topups at £250
// (owner decision; tighter than the £10,000 general till cap). Local constant
// — do not import from giftcard_limits.go (may not exist yet).
const maxTillGiftCardAmountPence = 25_000
func CreateTillSale(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
// Defense-in-depth admin check (S-1) — a till sale moves money (charges a
@@ -160,6 +302,24 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Amount must be greater than zero", http.StatusBadRequest)
return
}
// C2: apply the shared £10,000 cap used by every other money entry point
// (validators.ValidateAmount). The till amount is pounds float64; validate
// the effective pence figure exactly as the Square charge amount below is
// derived (math.Round(req.Amount * 100)).
amountPence := int64(math.Round(req.Amount * 100))
// Gift-card creates/topups are additionally capped at £250 per transaction
// (owner decision). Both the create and topup branches fund the card from
// req.Amount and flow through this single validation point, so one guard
// covers both.
if req.ItemType == "gift_card" && amountPence > maxTillGiftCardAmountPence {
http.Error(w, "Gift card amount exceeds maximum (£250)", http.StatusBadRequest)
return
}
if err := ValidateAmount(amountPence); err != nil {
log.Printf("Failed to process request: %v", err)
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
if req.PaymentMethod != "cash" && req.PaymentMethod != "card_machine" && req.PaymentMethod != "saved_card" && req.PaymentMethod != "online_square" && req.PaymentMethod != "on_the_house" {
http.Error(w, "Payment method must be 'cash', 'card_machine', 'saved_card', 'online_square', or 'on_the_house'", http.StatusBadRequest)
return
@@ -191,19 +351,49 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
return
}
// C1: when the client supplies no idempotency key, compute a DETERMINISTIC
// base key from the canonical request fields instead of a fresh
// uniqueChargeKey. A lost-response retry re-derives the SAME base key (the
// card nonce is deliberately excluded — it changes between retries), so the
// idempotency lookup below reuses the pending till_sale row and Square
// dedups on the key: ONE charge, ONE gift-card funding instead of the old
// random fallback's second charge + second funding. The base key is also the
// advisory-lock key, so concurrent same-sale retries serialize on it; the
// FINAL key is resolved by a slot scan under the lock (F5) so two genuinely
// DISTINCT keyless sales (same admin, same amount, no user / gift card)
// diverge onto different keys instead of the second being swallowed by the
// first's dedup.
var derivedBaseKey string
if req.IdempotencyKey == "" {
switch req.PaymentMethod {
case "saved_card", "online_square", "card_machine":
// Square-charging methods: deterministic base so a lost-response retry
// re-derives the SAME base and reuses the pending row — ONE charge,
// ONE gift-card funding instead of a second charge. The slot scan
// under the lock finalizes the candidate (see
// scanTillIdempotencyKeySlot).
derivedBaseKey = deriveTillIdempotencyKey(req, adminID)
req.IdempotencyKey = derivedBaseKey
default:
// cash / on_the_house: no Square charge, so a unique key per request is
// required — two identical keyless cash gift-card creations are
// distinct sales and must both succeed (see
// TestCreateTillSale_TwoIdenticalCreateSales_BothSucceed).
req.IdempotencyKey = uniqueChargeKey("till-")
}
}
// Serialize till-sale attempts on the idempotency key to prevent concurrent
// same-key requests from both passing the idempotency check, both funding
// the gift card, and one dying on the till_sales idempotency_key UNIQUE
// constraint after the funding already committed. Mirrors the gift-card
// advisory-lock pattern (giftcards.go). Lock is keyed on the idempotency
// key so distinct sales are unaffected; falls back to a per-request key
// when absent (client-supplied key is always used in practice). Bounded
// try-lock (R6) so a contended lock never blocks the pool across the Square
// key so distinct sales are unaffected; a missing client key falls back to
// the DETERMINISTIC derived key above, so a retry of the same logical sale
// serializes on the SAME lock as the original attempt. Bounded try-lock
// (R6) so a contended lock never blocks the pool across the Square
// round-trip.
lockKey := req.IdempotencyKey
if lockKey == "" {
lockKey = "till-" + rand.Text()
}
pinConn, err := db.Conn.Acquire(ctx)
if err != nil {
log.Printf("Failed to acquire connection for till-sale lock: %v", err)
@@ -222,13 +412,24 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Sale in progress, try again", http.StatusConflict)
return
}
defer func() {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext('crussell:till:' || $1))
`, lockKey); err != nil {
log.Printf("Failed to release till-sale serialization lock for %s: %v", lockKey, err)
defer releasePaymentLock(pinConn, "crussell:till:"+lockKey)
// F5: under the advisory lock, resolve the final deterministic key for a
// keyless Square-method sale. The slot scan advances past COMPLETED/FAILED
// sales occupying a candidate key, so two identical keyless walk-in creates
// never collapse onto one key; a PENDING sale never occupies a slot, so a
// lost-response retry re-derives the same candidate and the idempotency
// lookup below reuses the pending row. Running under the lock makes the
// scan-and-insert atomic for concurrent identical creates.
if derivedBaseKey != "" {
candidate, scanErr := scanTillIdempotencyKeySlot(ctx, derivedBaseKey)
if scanErr != nil {
log.Printf("Failed to resolve till-sale idempotency key for %s: %v", derivedBaseKey, scanErr)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
}()
req.IdempotencyKey = candidate
}
// Idempotency handling. A 'completed' sale is a dedup (return it). A
// 'pending' sale means the previous Square charge failed — the gift card
@@ -237,11 +438,12 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
// Mirrors the tip/gift-card pending-reuse pattern.
var existingPendingID string
var existingPendingGiftCard string
var existingPendingMethod string
if req.IdempotencyKey != "" {
var existingID, existingStatus, existingItemID string
var existingID, existingStatus, existingItemID, existingStoredKey string
var existingTotal float64
var existingItemType, existingPaymentMethod string
err := db.Conn.QueryRow(ctx, `SELECT id, status, item_id, total_amount, item_type, payment_method FROM till_sales WHERE idempotency_key = $1`, req.IdempotencyKey).Scan(&existingID, &existingStatus, &existingItemID, &existingTotal, &existingItemType, &existingPaymentMethod)
err := db.Conn.QueryRow(ctx, `SELECT id, status, item_id, total_amount, item_type, payment_method, idempotency_key FROM till_sales WHERE idempotency_key = $1`, req.IdempotencyKey).Scan(&existingID, &existingStatus, &existingItemID, &existingTotal, &existingItemType, &existingPaymentMethod, &existingStoredKey)
if err == nil {
if existingStatus == "completed" {
// C4-class: a same-key retry of a COMPLETED sale must report the
@@ -278,6 +480,13 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
}
existingPendingID = existingID
existingPendingGiftCard = existingItemID
existingPendingMethod = existingPaymentMethod
// F5: reuse the row's STORED idempotency key rather than a
// re-derived one — the stored key is what Square charged (and
// dedups on), so a retry must replay it verbatim.
if existingStoredKey != "" {
req.IdempotencyKey = existingStoredKey
}
}
if existingStatus == "failed" {
// Swept as stale (>24h) or definitively rejected — a retry would
@@ -522,7 +731,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
// become an untracked charge.
var existingPendingCheckoutID string
if existingPendingID != "" {
err = tx.QueryRow(ctx, `SELECT COALESCE(square_checkout_id, '') FROM till_sales WHERE id = $1`, existingPendingID).Scan(&existingPendingCheckoutID)
err = tx.QueryRow(ctx, `SELECT COALESCE(square_checkout_id, ''), payment_method FROM till_sales WHERE id = $1`, existingPendingID).Scan(&existingPendingCheckoutID, &existingPendingMethod)
if err != nil {
log.Printf("Failed to query existing pending sale checkout: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
@@ -530,21 +739,42 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
}
}
// Method-switch guard on pending-retry: if the original attempt was
// card_machine and created a live terminal checkout, the retry MUST stay
// card_machine and reuse that checkout. Switching to cash/on_the_house/
// saved_card/online_square would report the sale completed while the
// original checkout is still live the customer could be charged at the
// terminal AND by the new method (double charge). The terminal checkout
// cannot be cancelled via this API, so reject the switch outright.
// Method-switch guard on pending-retry (F1): switching the payment method of
// a pending sale whose Square charge outcome is unknown must never mint a
// NEW checkout or charge that cannot dedup against the original. A pending
// card_machine checkout (whether its id is still stored, or was lost with
// the lost response) may still be live at the terminal, and a pending
// online_square/saved_card charge may have landed — retrying either as a
// different method risks TWO charges for one card. Reject the switch
// outright: the sale must be completed or refunded first. online_square ↔
// saved_card retries are the one allowed cross-method case: both charge via
// Square CreatePayment under the SAME idempotency key, so Square dedups the
// retry onto the original charge (no second charge).
// NOTE: a future provisional "tmp-" till_sales row (pre-Square, as the
// booking path uses) must be treated as "no real checkout" by BOTH this
// reuse and the method-switch guard — a "tmp-" id is provably not live at
// Square.
if existingPendingID != "" && existingPendingCheckoutID != "" && req.PaymentMethod != "card_machine" {
log.Printf("Till-sale retry rejected: pending sale %s has a live card-machine checkout, cannot switch method from card_machine to %s", existingPendingID, req.PaymentMethod)
http.Error(w, "This pending sale is tied to a live card-machine checkout — retry with card machine payment", http.StatusConflict)
return
if existingPendingID != "" {
cardMachineInProgress := existingPendingCheckoutID != "" || existingPendingMethod == "in_person_card"
if cardMachineInProgress && req.PaymentMethod != "card_machine" {
// Pending card-machine sale retried by any other method: the original
// terminal checkout may still complete — switching would let the
// customer be charged at the terminal AND by the new method. The
// terminal checkout cannot be cancelled via this API.
log.Printf("Till-sale retry rejected: pending sale %s is tied to a card-machine checkout, cannot switch method to %s", existingPendingID, req.PaymentMethod)
http.Error(w, "This pending sale is tied to a card-machine checkout — complete or refund it first, then retry with card machine payment", http.StatusConflict)
return
}
if req.PaymentMethod == "card_machine" && !cardMachineInProgress {
// F1: a pending online_square/saved_card charge (outcome unknown)
// retried as card_machine would mint a NEW terminal checkout at
// Square — CreateCheckout cannot dedup against the original
// CreatePayment charge, so the original may land on top of the new
// terminal charge (double charge for one card).
log.Printf("Till-sale retry rejected: pending sale %s has payment method %s, cannot retry as card_machine (original charge outcome unknown)", existingPendingID, existingPendingMethod)
http.Error(w, "This pending sale is already in progress on a different payment method — complete or refund it first", http.StatusConflict)
return
}
}
// Reconcile-or-reject on a cash/on_the_house retry of a pending card sale.
@@ -595,9 +825,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
case "cash":
saleStatus = "completed"
dbPaymentMethod = "cash"
if req.IdempotencyKey == "" {
req.IdempotencyKey = uniqueChargeKey("till-")
}
case "saved_card":
dbPaymentMethod = "online_square"
if req.UserID != nil && *req.UserID != "" {
@@ -658,18 +885,11 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
return
}
if req.IdempotencyKey == "" {
req.IdempotencyKey = uniqueChargeKey("till-")
}
tillSquareSourceID = savedCardSqCardID
saleStatus = "pending"
needsSquarePayment = true
case "card_machine":
dbPaymentMethod = "in_person_card"
if req.IdempotencyKey == "" {
req.IdempotencyKey = uniqueChargeKey("till-")
}
if existingPendingCheckoutID != "" {
// Pending retry — reuse the checkout already created for this sale
@@ -679,6 +899,36 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
squareCheckoutID = &existingPendingCheckoutID
saleStatus = "pending"
} else {
// F1: never mint a NEW terminal checkout while a pending charge for
// the same logical sale exists. existingPendingID (resolved by the
// idempotency-key lookup or the gift-card fallback) IS such a
// pending charge — a pending card_machine sale with no stored
// checkout id means the prior response was lost and the original
// checkout may still be live at the terminal; a fresh checkout
// would orphan it into a second, untracked charge.
if existingPendingID != "" {
log.Printf("Till-sale retry rejected: pending sale %s has no stored checkout id; refusing to create a second terminal checkout (original may still be live)", existingPendingID)
http.Error(w, "This pending sale has a card-machine payment in progress with an unknown checkout — complete or refund it first", http.StatusConflict)
return
}
// Defense-in-depth: a card_machine top-up on a card that already
// carries a pending till_sale (reached only when the earlier
// idempotency / gift-card resolution treated this as a NEW sale,
// e.g. a different amount) must not mint a terminal checkout while
// that pending sale's charge is still unresolved.
if req.Action == "topup" && req.GiftCardID != nil && *req.GiftCardID != "" {
var pendingOnCard string
if err := tx.QueryRow(ctx, `
SELECT id FROM till_sales
WHERE item_id = $1 AND status = 'pending'
ORDER BY created_at DESC
LIMIT 1
`, validators.NormalizeGiftCardCode(*req.GiftCardID)).Scan(&pendingOnCard); err == nil && pendingOnCard != "" {
log.Printf("Till-sale card_machine create rejected: gift card %s already has a pending till sale %s", *req.GiftCardID, pendingOnCard)
http.Error(w, "This gift card already has a pending sale — complete or refund it first", http.StatusConflict)
return
}
}
checkoutReq := square.CreateCheckoutReq{
Amount: penceAmount,
Currency: "GBP",
@@ -700,9 +950,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
}
case "online_square":
dbPaymentMethod = "online_square"
if req.IdempotencyKey == "" {
req.IdempotencyKey = uniqueChargeKey("till-")
}
tillSquareSourceID = req.CardToken
saleStatus = "pending"
@@ -710,9 +957,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
case "on_the_house":
saleStatus = "completed"
dbPaymentMethod = "on_the_house"
if req.IdempotencyKey == "" {
req.IdempotencyKey = uniqueChargeKey("till-")
}
}
desc := fmt.Sprintf("Gift Card %s (£%.2f)", req.Action, req.Amount)
@@ -729,6 +973,13 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
if _, srcErr := tx.Exec(ctx, `UPDATE till_sales SET square_source_id = $1 WHERE id = $2`, tillSquareSourceID, tillSaleID); srcErr != nil {
log.Printf("Failed to update square_source_id on reused till sale %s: %v", tillSaleID, srcErr)
}
// B6: the sweep replays the stored square_request_snapshot VERBATIM,
// so the snapshot's source_id must stay in lock-step with the
// refreshed square_source_id — a snapshot whose source differs from
// the row's source returns IDEMPOTENCY_KEY_REUSED at Square and
// strands the row pending forever. Refresh both columns in this same
// transaction so they never diverge.
refreshTillSnapshotSource(ctx, tx, tillSaleID, tillSquareSourceID)
}
} else {
err = tx.QueryRow(ctx, `
@@ -1050,13 +1301,7 @@ func GetTillCheckoutStatus(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Payment in progress, try again", http.StatusConflict)
return
}
defer func() {
if _, err := pinConn.Exec(context.Background(), `
SELECT pg_advisory_unlock(hashtext('crussell:tillcomplete:' || $1))
`, tillSaleID); err != nil {
log.Printf("Failed to release till-completion serialization lock for %s: %v", tillSaleID, err)
}
}()
defer releasePaymentLock(pinConn, "crussell:tillcomplete:"+tillSaleID)
tx, err := db.Conn.Begin(r.Context())
if err != nil {
+1 -1
View File
@@ -850,7 +850,7 @@ func TestCreateTillSale_OnlineSquareWithToken(t *testing.T) {
reqBody := TillSaleRequest{
ItemType: "gift_card",
Action: "create",
Amount: 1000,
Amount: 50,
PaymentMethod: "online_square",
CardToken: "cnon:visa",
}