From 57bdeb92329443a55653472ab1c060f02f99633e Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Mon, 3 Aug 2026 18:10:33 +0100 Subject: [PATCH] Adopt shared charge helpers in till and gift-card flows; redact card tokens Till sales and gift-card purchases now use uniqueChargeKey and resolveChargeSource/chargeFailureStatus instead of duplicated inline logic. The saved-card delete path logs the square_card_id through square.TokenPrefix so full ccof references never reach logs. --- backend/handlers/payments/giftcards.go | 100 +++---------------------- backend/handlers/payments/service.go | 4 +- backend/handlers/payments/till.go | 22 ++---- 3 files changed, 21 insertions(+), 105 deletions(-) diff --git a/backend/handlers/payments/giftcards.go b/backend/handlers/payments/giftcards.go index ea9f17d..a447590 100644 --- a/backend/handlers/payments/giftcards.go +++ b/backend/handlers/payments/giftcards.go @@ -1,7 +1,6 @@ package payments import ( - "context" "database/sql" "encoding/json" "errors" @@ -896,7 +895,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { return } if req.IdempotencyKey == "" { - req.IdempotencyKey = uniqueTillKey() + req.IdempotencyKey = uniqueChargeKey("till-") } allowedAmounts := map[int64]bool{1000: true, 2000: true, 5000: true} @@ -935,31 +934,11 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { if lockKey == "" { lockKey = userID } - pinConn, err := db.Conn.Acquire(ctx) - if err != nil { - log.Printf("Failed to acquire connection for gift-card lock: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - defer pinConn.Release() - lockOK, err := acquireAdvisoryLock(ctx, pinConn, "crussell:giftcard:"+lockKey) - if err != nil { - log.Printf("Failed to acquire gift-card serialization lock for %s: %v", lockKey, err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } + pinConn, lockOK := acquireBookingPaymentLock(ctx, w, "crussell:giftcard:"+lockKey, "Purchase in progress, try again") if !lockOK { - log.Printf("Gift-card serialization lock for %s not acquired within bound — a purchase is already in progress", lockKey) - http.Error(w, "Purchase in progress, try again", http.StatusConflict) return } - defer func() { - if _, err := pinConn.Exec(context.Background(), ` - SELECT pg_advisory_unlock(hashtext('crussell:giftcard:' || $1)) - `, lockKey); err != nil { - log.Printf("Failed to release gift-card serialization lock for %s: %v", lockKey, err) - } - }() + defer releaseBookingPaymentLock(pinConn, "crussell:giftcard:"+lockKey) // Idempotency: only short-circuit when the existing record is 'completed'. // A 'pending' record means the previous Square call failed — returning it @@ -1003,71 +982,12 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { var sourceID string var savedCardID *string var savedCardCustomerID string - - if req.NewCardToken != nil && *req.NewCardToken != "" { - // A cnon: nonce charge needs NO card-on-file and NO customer (R6). The - // old code tokenized every new card via CreateCardOnFile even for - // one-off non-save purchases, creating an orphan card at Square for a - // payment that only ever uses the nonce once. - if req.SaveCard { - // Save path: provision (or reuse) the user's Square customer BEFORE - // tokenizing so the new card is created against that customer, and - // forward the customer id on the charge. A ccof: source MUST carry - // its customer (R6). - squareCustomerID, custErr := paymentService.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 - } - cardOnFile, err := SquareClient.CreateCardOnFile(ctx, userID, *req.NewCardToken, squareCustomerID) - if err != nil { - log.Printf("Failed to create card on file: %v", err) - http.Error(w, "Failed to process card", http.StatusInternalServerError) - return - } - sourceID = cardOnFile.CardID - savedCardCustomerID = squareCustomerID - - cardID, err := paymentService.SaveCardForUser(ctx, userID, squareCustomerID, cardOnFile.CardID, cardOnFile.Brand, cardOnFile.Last4, cardOnFile.ExpMonth, cardOnFile.ExpYear, cardOnFile.Fingerprint) - if err != nil { - log.Printf("Failed to save card: %v", err) - } else { - savedCardID = &cardID - } - } else { - // One-off new-card purchase: use the cnon: nonce DIRECTLY as the - // source. No card-on-file is created (nothing to orphan, no - // customer needed). - sourceID = *req.NewCardToken - } - } else if req.CardID != nil { - card, err := paymentService.GetCardByID(ctx, *req.CardID, userID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - http.Error(w, "Card not found", http.StatusNotFound) - return - } - log.Printf("Failed to get card: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - // A ccof: source can NEVER be charged without a CustomerID — Square - // rejects the payment. A saved-card row created before P14 has an empty - // square_customer_id; lazily provision the user's Square customer and - // persist it on the row BEFORE charging (R6). - if card.SquareCustomerID == "" { - provisioned, provErr := paymentService.EnsureSquareCustomerForSavedCard(ctx, *req.CardID, userID) - if provErr != nil { - log.Printf("Failed to provision Square customer for saved card %s (user %s): %v", *req.CardID, userID, provErr) - http.Error(w, "Failed to process card", http.StatusInternalServerError) - return - } - card.SquareCustomerID = provisioned - } - sourceID = card.SquareCardID - savedCardID = req.CardID - savedCardCustomerID = card.SquareCustomerID + // Resolve the new-card-vs-saved-card Square source — shared with + // CreateBookingPayment/CreateTipPayment (see resolveChargeSource for the + // R6 rationale). + sourceID, savedCardID, savedCardCustomerID, sourceOK := resolveChargeSource(ctx, w, paymentService, userID, req.NewCardToken, req.CardID, req.SaveCard, "Card not found") + if !sourceOK { + return } amountPounds := float64(req.Amount) / 100.0 @@ -1177,7 +1097,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { if err != nil { log.Printf("Failed to process gift card purchase payment: %v", err) // Payment record intentionally left as 'pending' for manual retry. - http.Error(w, "Payment failed", http.StatusPaymentRequired) + http.Error(w, "Payment failed", chargeFailureStatus(err)) return } diff --git a/backend/handlers/payments/service.go b/backend/handlers/payments/service.go index 6d8613d..c46c1bf 100644 --- a/backend/handlers/payments/service.go +++ b/backend/handlers/payments/service.go @@ -548,7 +548,9 @@ func (s *PaymentService) DeletePaymentMethod(ctx context.Context, cardID, userID if square.ErrorCode(err) == "NOT_FOUND" || strings.Contains(msg, "NOT_FOUND") || strings.Contains(msg, "NOT FOUND") { // Square already removed/disabled the card — nothing to do. } else { - slog.Warn("failed to disable Square card on local delete — card may remain enabled at Square", "square_card_id", sqCardID.String, "err", err) + // TokenPrefix redacts the ccof: card token — the full ID must + // never reach logs. + slog.Warn("failed to disable Square card on local delete — card may remain enabled at Square", "square_card_id", square.TokenPrefix(sqCardID.String), "err", err) } } } diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index cde04a7..6905a68 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -46,15 +46,9 @@ type TillSaleResponse struct { CheckoutID *string `json:"checkout_id,omitempty"` } -// uniqueTillKey generates a unique idempotency key for till sales where the -// client did not supply one. Dedup of retries is handled by the client-supplied -// key (the frontend sends a UUID); this fallback only needs to be unique so it -// never collides with the till_sales idempotency_key UNIQUE constraint. -// Deliberately NOT derived from request fields — two legitimate identical -// sales (e.g. two £50 cash gift-card creations) would hash to the same key. -func uniqueTillKey() string { - return "till-" + rand.Text() -} +// uniqueChargeKey is defined in handlers.go (the till fallback key was +// byte-identical to the tip fallback except the prefix — see the consolidated +// helper). // definitivePaymentDeclineCodes are Square payment error codes meaning the // card charge can never succeed (declined / expired / not supported). They are @@ -520,7 +514,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { saleStatus = "completed" dbPaymentMethod = "cash" if req.IdempotencyKey == "" { - req.IdempotencyKey = uniqueTillKey() + req.IdempotencyKey = uniqueChargeKey("till-") } case "saved_card": dbPaymentMethod = "online_square" @@ -577,7 +571,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { } if req.IdempotencyKey == "" { - req.IdempotencyKey = uniqueTillKey() + req.IdempotencyKey = uniqueChargeKey("till-") } saleStatus = "pending" @@ -585,7 +579,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { case "card_machine": dbPaymentMethod = "in_person_card" if req.IdempotencyKey == "" { - req.IdempotencyKey = uniqueTillKey() + req.IdempotencyKey = uniqueChargeKey("till-") } if existingPendingCheckoutID != "" { @@ -617,7 +611,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { case "online_square": dbPaymentMethod = "online_square" if req.IdempotencyKey == "" { - req.IdempotencyKey = uniqueTillKey() + req.IdempotencyKey = uniqueChargeKey("till-") } saleStatus = "pending" @@ -626,7 +620,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { saleStatus = "completed" dbPaymentMethod = "on_the_house" if req.IdempotencyKey == "" { - req.IdempotencyKey = uniqueTillKey() + req.IdempotencyKey = uniqueChargeKey("till-") } }