Files
Crussell/backend/handlers/payments/charge_helpers.go
T
popertots 7f1c649f1e Apply second-round review fixes: idempotency-key length caps, stable-sentinel card keys, test-isolation, naming
Money-safety idempotency hardening (I1, wide):
- validate:"max=45" on CreateTerminalPayment/BookingPayment/Refund/Tip/
  BuyGiftCard idempotency keys (all feed Square's 45-char /v2/payments,
  /v2/refunds, /v2/cards caps); BuyGiftCard corrected from a wrongly-loose
  max=64. Till keeps max=64 (its key also feeds the 64-char terminal-checkout
  endpoint).
- Explicit 45-char guard in RefundPayment: the one handler that decodes
  RefundRequest without running the struct validator, so the tag alone was
  inert; a longer key would 400 at Square and be misclassified as a
  definitive refund decline.
- New TestIdempotencyKey_OverLength_RejectedAcrossPaymentHandlers covers all
  six endpoints (terminal saved-card, booking, tip, gift-card, till, refund).

Stable-sentinel card identity in idempotency keys (C1, wide):
- BookingFlow deposit key now uses the 'new-card' sentinel instead of
  embedding the cnon: nonce (matches UserPaymentModal/account). A re-tokenize
  after a spent nonce no longer regenerates the key, closing a lost-response
  double-charge window.
- TipPayment + UserBookingModal tip keys now include card identity
  (selectedCardId || 'new-card'); previously keyed on amount only, so a
  same-amount tip on a DIFFERENT card reused the key and deduped a distinct
  charge. Resets cleared in every success/close path.

Test isolation (R1): TestRefund_PendingResume_NewKeyAfterModalReopen no
longer t.Parallel — it swaps the package-level SquareClient mid-test and a
concurrent parallel test could observe the swapped instance.

Naming/quality (M1/M2/M4): resolveChargeSource local renamed savedRowID (was
shadowing the cardID *string parameter); BuyGiftCard fallback prefix
"till-" -> "gc-"; saved-card terminal response key "checkout_id" -> "payment_id"
(it holds a DB payment row, not a Square checkout) with matching frontend
fallback. README maintenance-job count corrected 24 -> 25.

Full suite 25/25 + race clean via run-tests.sh lockfile; svelte-check 0
errors/warnings; production build succeeds.
2026-08-22 00:34:49 +01:00

168 lines
7.5 KiB
Go

package payments
import (
"context"
"errors"
"log"
"net/http"
"crussell/db"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgxpool"
)
// resolveChargeSource resolves the Square payment source for a card charge,
// shared by CreateBookingPayment, CreateTipPayment, BuyGiftCard, and the
// CreateTerminalPayment saved-card branch (which passes the booking user's id
// and a nil new-card token).
//
// New-card path (cnon: nonce): the nonce is used DIRECTLY for one-off charges
// (no card-on-file is created — the old tokenize-then-charge flow left orphan
// cards at Square). When saveCard is true the user's Square customer is
// provisioned FIRST and the card is tokenized against it (a ccof: source MUST
// carry its customer — R6), then saved via SaveCardForUser.
//
// Saved-card path (ccof:): a saved-card row predating P14 has an empty
// square_customer_id; the user's Square customer is lazily provisioned and
// persisted on the row BEFORE charging (a ccof: source can never be charged
// without a CustomerID).
//
// On any error the helper writes the HTTP response and returns ok=false — the
// caller must return immediately.
func resolveChargeSource(ctx context.Context, w http.ResponseWriter, svc *PaymentService, userID string, newCardToken, cardID *string, saveCard bool, notFoundMsg string) (sourceID string, savedCardID *string, squareCustomerID string, ok bool) {
if newCardToken != nil && *newCardToken != "" {
if saveCard {
sqCustomerID, custErr := svc.EnsureSquareCustomer(ctx, userID)
if custErr != nil {
log.Printf("Failed to provision Square customer for user %s: %v", userID, custErr)
http.Error(w, "Failed to process card", http.StatusInternalServerError)
return "", nil, "", false
}
cardOnFile, err := SquareClient.CreateCardOnFile(ctx, userID, *newCardToken, sqCustomerID)
if err != nil {
log.Printf("Failed to create card on file: %v", err)
http.Error(w, "Failed to process card", http.StatusInternalServerError)
return "", nil, "", false
}
sourceID = cardOnFile.CardID
squareCustomerID = sqCustomerID
// CreateCardOnFile runs before the charge. If the subsequent payment
// fails, this card-on-file is intentionally NOT deleted: the pending
// record's retry re-creates it via the deterministic sha256
// idempotency key (the SAVE path), and Square returns the same card —
// deleting it would break that retry.
savedRowID, saveErr := svc.SaveCardForUser(ctx, userID, sqCustomerID, cardOnFile.CardID, cardOnFile.Brand, cardOnFile.Last4, cardOnFile.ExpMonth, cardOnFile.ExpYear, cardOnFile.Fingerprint)
if saveErr != nil {
log.Printf("Failed to save card: %v", saveErr)
} else {
savedCardID = &savedRowID
}
} else {
// One-off new-card charge: use the cnon: nonce DIRECTLY as the
// source. No card-on-file is created (nothing to orphan, no
// customer needed).
sourceID = *newCardToken
}
if savedCardID == nil && saveCard {
log.Printf("Card was not saved despite save_card=true for user %s", userID)
}
return sourceID, savedCardID, squareCustomerID, true
}
if cardID != nil {
card, err := svc.GetCardByID(ctx, *cardID, userID)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
http.Error(w, notFoundMsg, http.StatusNotFound)
return "", nil, "", false
}
log.Printf("Failed to get card: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return "", nil, "", false
}
if card.SquareCustomerID == "" {
if userID == "" {
// Defensive parity with the original saved-card block: a card
// with no bookable owner cannot be provisioned. Unreachable in
// practice — GetCardByID above filters on user_id and would
// have 404'd for an empty owner.
http.Error(w, "Saved card has no owner and cannot be charged", http.StatusBadRequest)
return "", nil, "", false
}
provisioned, provErr := svc.EnsureSquareCustomerForSavedCard(ctx, *cardID, userID)
if provErr != nil {
log.Printf("Failed to provision Square customer for saved card %s (user %s): %v", *cardID, userID, provErr)
http.Error(w, "Failed to process card", http.StatusInternalServerError)
return "", nil, "", false
}
card.SquareCustomerID = provisioned
}
return card.SquareCardID, cardID, card.SquareCustomerID, true
}
// Neither a new-card token nor a saved card — validation upstream
// (ValidateCardInfo) guarantees one of them is present.
return "", nil, "", false
}
// acquireBookingPaymentLock acquires a pinned pool connection and a bounded
// try-lock (R6) on lockKey, serializing payment attempts per booking (the core
// defence against the two-tab double-payment race). A blocking pg_advisory_lock
// would hold the pinned pool connection for the full Square round-trip of
// whichever request holds the lock; the bounded try-lock loop gives up after
// ~3s and surfaces a 409 instead of exhausting the pool. On any failure the
// helper writes the HTTP response and returns ok=false — the caller must
// return. On success the caller MUST defer releaseBookingPaymentLock(pinConn,
// lockKey): the lock and connection stay held for the whole handler so
// pg_advisory_unlock runs on the SAME session that acquired the lock.
func acquireBookingPaymentLock(ctx context.Context, w http.ResponseWriter, lockKey, conflictMsg string) (*pgxpool.Conn, bool) {
pinConn, err := db.Conn.Acquire(ctx)
if err != nil {
log.Printf("Failed to acquire connection for payment lock (%s): %v", lockKey, err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return nil, false
}
lockOK, err := acquireAdvisoryLock(ctx, pinConn, lockKey)
if err != nil {
pinConn.Release()
log.Printf("Failed to acquire payment serialization lock %s: %v", lockKey, err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return nil, false
}
if !lockOK {
pinConn.Release()
log.Printf("Payment serialization lock %s not acquired within bound — a payment is already in progress", lockKey)
http.Error(w, conflictMsg, http.StatusConflict)
return nil, false
}
return pinConn, true
}
// releaseBookingPaymentLock releases the advisory lock acquired by
// acquireBookingPaymentLock and returns the pinned connection to the pool.
// Both run on the same session that holds the lock.
func releaseBookingPaymentLock(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)
}
pinConn.Release()
}
// recheckBookingPayable re-reads the booking status after a Square charge
// succeeded (R9): a concurrent cancellation/eviction can move the booking out
// of a payable state between the pre-charge status check and the charge
// completing. 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 it. Returns the
// re-read status and whether a completed payment is still allowed; the caller
// owns the CRITICAL logging, the mark-failed write (whose target and
// transaction semantics differ per path), and the 409 conflict response.
func recheckBookingPayable(ctx context.Context, q db.Querier, bookingID string) (string, bool, error) {
var status string
if err := q.QueryRow(ctx, `SELECT status FROM bookings WHERE id = $1`, bookingID).Scan(&status); err != nil {
return "", false, err
}
return status, bookingStatusAllowsCompletedPayment(status), nil
}