popertots
36887167c6
Loop A fresh money/security/dup-mod review of the whole payments overhaul. 28 consolidated findings fixed:
MONEY:
- HIGH-1: B12 overflow guard now uses the discounted obligation — a pre-start deposit can never mint an unintended tip; the discount is never truncated to £0 when the customer pays the discounted deposit
- HIGH-2: discounted-deposit pending-reuse retry compares pendingStoredAmountPence vs chargeAmount (the actual Square amount), not req.Amount — no more permanent amount_mismatch 400 on lost-response retries
- MEDIUM-3: sweep rescue now carves overflow as a tip record + runs completion side-effects (was booking overflow as service revenue, skipping completion)
- MEDIUM-4 (shared w/ security): admin_audit_log.admin_id made nullable + anonymize_user/delete_guest_user NULL it + scrub details.card_last4 — 2fa_fallback_charge PII no longer survives account deletion
- MEDIUM-5: till gift-card payment now passes the £5,000/day admin cap (giftcard_limits)
- LOW-6: expired gift-card balance surfaced as expired/zero in GetUserGiftCardBalance
SECURITY:
- 2FA single-use consume made atomic at verify time for all 5 saved-card gates (fresh charges consume; pending-reuse retries don't); deferred consumption removed
- reissueTwoFACodeAfterFailedCharge routed through the fail-closed issuance gate (pepper check, cooldown) + fresh-only semantics (only when a code was actually consumed)
- family-alive cache invalidated on the stale-family cleanup DELETE (no 30s warm window after expiry)
- frontend 503-retry no longer reuses a consumed 2FA code — aligns with backend re-issue
DUP/MOD:
- reissue helper single-sourced (5 call sites), squareRefundStatusToLocal (10 inline switches), writeChargeSnapshot (7 sites, immutability guard on gift-card/till), postChargeRecheck (3+1 sites), scanIdempotencySlot (2), applyVATToChargeRecord (3 patterns), user_saved_cards upsert (2), BuyGiftCard pending INSERT via service
- till completed-dedup now re-validates paymentHasLiveRefund (aligns with booking/tip/gift-card)
- frontend 402 idempotency-key regeneration added to PaymentModal (aligns with other CIT surfaces)
- PAYMENT_METHOD_SAVED_CARD constant standardised ('saved_card' everywhere)
- admin audit coverage added for AdminRefundBooking + gift-card buy/top-up
- audit-helper cross-package dedup (user/twofa.go now calls payments' exported insert)
Verified: 26/26 dev + 24/24 prod packages, both vet tags, frontend tests + build, gitleaks clean.
268 lines
12 KiB
Go
268 lines
12 KiB
Go
//go:build test && dev
|
|
|
|
package payments
|
|
|
|
// Tests pinning the Loop B 2FA single-use consume-at-gate fix on the two
|
|
// saved-card charge gates that were still consuming AFTER the charge landed:
|
|
// the till saved-card charge (till.go) and the gift-card purchase saved-card
|
|
// gate (giftcards.go).
|
|
//
|
|
// Semantics (mirroring CreateBookingPayment, handlers.go):
|
|
// - A FRESH charge verifies the 2FA code WITH consumption at the gate
|
|
// (consume = !reusePendingRecord). The code is single-use, closing the
|
|
// TOCTOU where a verified-but-unconsumed code could authorize a second
|
|
// concurrent charge. If Square then fails, a fresh code is re-issued
|
|
// (reissueTwoFACodeAfterFailedCharge) so the same-key retry has a live
|
|
// code to verify.
|
|
// - A PENDING-REUSE retry verifies WITHOUT consuming: the code was re-issued
|
|
// for exactly this retry, and the post-charge success path consumes it on
|
|
// terminal success, so a retry that fails again keeps its code for one
|
|
// more attempt.
|
|
//
|
|
// These tests flip REQUIRE_2FA/SQUARE_ENVIRONMENT via t.Setenv and therefore
|
|
// must stay sequential (no t.Parallel) — see the note at the top of
|
|
// twofa_test.go. They use SQUARE_ENVIRONMENT=staging (NOT production) for
|
|
// enforcement: twoFactorEnforced() is fail-closed, so any non-mock/dev value
|
|
// enforces the gate, while square.NewDevClient() — which the injected fault
|
|
// client and structuredSquareErrorWithCode construct at call time — returns
|
|
// the in-memory mock for every env except production/sandbox. The shared
|
|
// helperEnvEnforce2FA sets production, which would panic NewDevClient in a
|
|
// dev build.
|
|
|
|
import (
|
|
"context"
|
|
"database/sql"
|
|
"net/http"
|
|
"testing"
|
|
"time"
|
|
|
|
"crussell/db"
|
|
"crussell/internal/square"
|
|
"crussell/internal/twofa"
|
|
"crussell/testutils"
|
|
"crussell/testutils/fixtures"
|
|
"crussell/testutils/jwt"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
)
|
|
|
|
// TestTwoFactorEnforced_CreateTillSale_SavedCard_FreshCharge_Failure pins the
|
|
// till saved-card gate on a FRESH charge: the 2FA code is consumed AT THE GATE
|
|
// (single-use), and a failed Square charge re-issues a fresh code so the
|
|
// same-key retry can verify again. The stored hash must differ from the seeded
|
|
// one — if the gate still used consume=false the seeded hash would survive
|
|
// unchanged and there would be no re-issue.
|
|
func TestTwoFactorEnforced_CreateTillSale_SavedCard_FreshCharge_Failure_ConsumesAtGate_Reissues(t *testing.T) {
|
|
t.Setenv("REQUIRE_2FA", "true")
|
|
t.Setenv("SQUARE_ENVIRONMENT", "staging")
|
|
ctx, tx := testutils.SetupTestTx(t)
|
|
|
|
adminID, err := fixtures.CreateTestAdminUser(tx)
|
|
require.NoError(t, err)
|
|
userID, err := fixtures.CreateTestUser(tx)
|
|
require.NoError(t, err)
|
|
adminToken := jwt.GenerateTestToken(adminID, "admin")
|
|
seedTwoFAPendingCode(t, tx, userID, "556677")
|
|
|
|
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:sq_test_card_id", "VISA", "1234")
|
|
require.NoError(t, err)
|
|
|
|
origClient := SquareClient
|
|
SquareClient = &definitiveChargeClient{SquareClient: square.NewDevClient(), createErr: structuredSquareErrorWithCode(t, http.StatusPaymentRequired, "CARD_DECLINED")}
|
|
defer func() { SquareClient = origClient }()
|
|
|
|
req := TillSaleRequest{
|
|
ItemType: "gift_card",
|
|
Action: "create",
|
|
Amount: 50.00,
|
|
PaymentMethod: "saved_card",
|
|
UserSavedCardID: &cardID,
|
|
UserID: &userID,
|
|
IdempotencyKey: "2fa-till-fresh-decline",
|
|
VerificationCode: "556677",
|
|
}
|
|
|
|
w := makePaymentRequest(CreateTillSale, "POST", "/api/admin/till/sale", req, adminToken, ctx)
|
|
require.Equal(t, http.StatusPaymentRequired, w.Code, w.Body.String())
|
|
|
|
var hash sql.NullString
|
|
require.NoError(t, tx.QueryRow(ctx, "SELECT two_factor_pending_code_hash FROM users WHERE id = $1", userID).Scan(&hash))
|
|
require.True(t, hash.Valid, "a failed fresh saved-card charge must re-issue a live 2FA code for the same-key retry")
|
|
require.NotEqual(t, twofa.Hash("556677"), hash.String, "the gate must have consumed the seeded code at verification time (single-use)")
|
|
}
|
|
|
|
// TestTwoFactorEnforced_CreateTillSale_SavedCard_PendingReuse_Failure pins the
|
|
// till saved-card gate on a PENDING-REUSE retry: the code is verified WITHOUT
|
|
// consumption, so a retry that fails again keeps its seeded code unchanged and
|
|
// no re-issue runs (the fresh-charge-only guard must not fire).
|
|
func TestTwoFactorEnforced_CreateTillSale_SavedCard_PendingReuse_Failure_KeepsCode(t *testing.T) {
|
|
t.Setenv("REQUIRE_2FA", "true")
|
|
t.Setenv("SQUARE_ENVIRONMENT", "staging")
|
|
ctx, tx := testutils.SetupTestTx(t)
|
|
|
|
adminID, err := fixtures.CreateTestAdminUser(tx)
|
|
require.NoError(t, err)
|
|
userID, err := fixtures.CreateTestUser(tx)
|
|
require.NoError(t, err)
|
|
adminToken := jwt.GenerateTestToken(adminID, "admin")
|
|
seedTwoFAPendingCode(t, tx, userID, "667788")
|
|
|
|
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:sq_test_card_id", "VISA", "1234")
|
|
require.NoError(t, err)
|
|
|
|
// Seed a PENDING till_sale with the same key — a prior attempt whose
|
|
// Square charge failed after the DB transaction committed (card funded).
|
|
key := "2fa-till-pending-reuse"
|
|
var giftCardID string
|
|
require.NoError(t, tx.QueryRow(ctx, `
|
|
INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, voucher_type_at_purchase)
|
|
VALUES (50.00, 50.00, $1, FALSE, 'SPV') RETURNING id
|
|
`, adminID).Scan(&giftCardID))
|
|
_, err = tx.Exec(ctx, `
|
|
INSERT INTO till_sales (item_type, item_id, description, quantity, unit_price, total_amount,
|
|
payment_method, status, user_id, user_saved_card_id, idempotency_key, created_by, created_at, updated_at)
|
|
VALUES ('gift_card', $1, 'Gift Card create', 1, 50.00, 50.00, 'online_square', 'pending',
|
|
$2, $3, $4, $5, NOW(), NOW())
|
|
`, giftCardID, userID, cardID, key, adminID)
|
|
require.NoError(t, err)
|
|
|
|
origClient := SquareClient
|
|
SquareClient = &definitiveChargeClient{SquareClient: square.NewDevClient(), createErr: structuredSquareErrorWithCode(t, http.StatusPaymentRequired, "CARD_DECLINED")}
|
|
defer func() { SquareClient = origClient }()
|
|
|
|
req := TillSaleRequest{
|
|
ItemType: "gift_card",
|
|
Action: "create",
|
|
Amount: 50.00,
|
|
PaymentMethod: "saved_card",
|
|
UserSavedCardID: &cardID,
|
|
UserID: &userID,
|
|
IdempotencyKey: key,
|
|
VerificationCode: "667788",
|
|
}
|
|
|
|
w := makePaymentRequest(CreateTillSale, "POST", "/api/admin/till/sale", req, adminToken, ctx)
|
|
require.Equal(t, http.StatusPaymentRequired, w.Code, w.Body.String())
|
|
|
|
var hash sql.NullString
|
|
require.NoError(t, tx.QueryRow(ctx, "SELECT two_factor_pending_code_hash FROM users WHERE id = $1", userID).Scan(&hash))
|
|
require.True(t, hash.Valid, "a pending-reuse retry must not consume the code at the gate")
|
|
require.Equal(t, twofa.Hash("667788"), hash.String, "a failed pending-reuse retry must keep its seeded code unchanged (no re-issue)")
|
|
}
|
|
|
|
// TestTwoFactorEnforced_BuyGiftCard_SavedCard_Fresh_Failure pins the gift-card
|
|
// purchase saved-card gate on a FRESH charge: consume-at-gate + re-issue on
|
|
// failure, exactly as the till gate above.
|
|
func TestTwoFactorEnforced_BuyGiftCard_SavedCard_Fresh_Failure_ConsumesAtGate_Reissues(t *testing.T) {
|
|
t.Setenv("REQUIRE_2FA", "true")
|
|
t.Setenv("SQUARE_ENVIRONMENT", "staging")
|
|
ctx, tx := testutils.SetupTestTx(t)
|
|
|
|
userID, err := fixtures.CreateTestUser(tx)
|
|
require.NoError(t, err)
|
|
token := jwt.GenerateTestToken(userID, "verified_email")
|
|
seedTwoFAPendingCode(t, tx, userID, "112233")
|
|
|
|
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:sq_test_card_id", "VISA", "1234")
|
|
require.NoError(t, err)
|
|
|
|
origClient := SquareClient
|
|
SquareClient = &definitiveChargeClient{SquareClient: square.NewDevClient(), createErr: structuredSquareErrorWithCode(t, http.StatusPaymentRequired, "CARD_DECLINED")}
|
|
defer func() { SquareClient = origClient }()
|
|
|
|
req := BuyGiftCardRequest{
|
|
Amount: 2000,
|
|
RecipientType: "self",
|
|
CardID: &cardID,
|
|
IdempotencyKey: "2fa-buy-gc-fresh-decline",
|
|
VerificationCode: "112233",
|
|
}
|
|
|
|
w := makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", req, token, ctx)
|
|
require.Equal(t, http.StatusPaymentRequired, w.Code, w.Body.String())
|
|
|
|
var hash sql.NullString
|
|
require.NoError(t, tx.QueryRow(ctx, "SELECT two_factor_pending_code_hash FROM users WHERE id = $1", userID).Scan(&hash))
|
|
require.True(t, hash.Valid, "a failed fresh saved-card purchase must re-issue a live 2FA code for the same-key retry")
|
|
require.NotEqual(t, twofa.Hash("112233"), hash.String, "the gate must have consumed the seeded code at verification time (single-use)")
|
|
}
|
|
|
|
// TestTwoFactorEnforced_BuyGiftCard_SavedCard_PendingReuse_Failure pins the
|
|
// gift-card purchase gate on a PENDING-REUSE retry: verify-without-consume and
|
|
// no re-issue, so the seeded code survives a second failed retry unchanged.
|
|
func TestTwoFactorEnforced_BuyGiftCard_SavedCard_PendingReuse_Failure_KeepsCode(t *testing.T) {
|
|
t.Setenv("REQUIRE_2FA", "true")
|
|
t.Setenv("SQUARE_ENVIRONMENT", "staging")
|
|
ctx, tx := testutils.SetupTestTx(t)
|
|
|
|
userID, err := fixtures.CreateTestUser(tx)
|
|
require.NoError(t, err)
|
|
token := jwt.GenerateTestToken(userID, "verified_email")
|
|
seedTwoFAPendingCode(t, tx, userID, "334455")
|
|
|
|
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:sq_test_card_id", "VISA", "1234")
|
|
require.NoError(t, err)
|
|
|
|
// Seed a PENDING payment record with the same key — a prior attempt whose
|
|
// Square charge failed (mirrors TestBuyGiftCard_RetryPending_ReattemptsCharge).
|
|
key := "2fa-buy-gc-pending-reuse"
|
|
_, err = tx.Exec(context.Background(), `
|
|
INSERT INTO payments (payment_type, payment_method, status, amount, idempotency_key, created_at, updated_at, created_by)
|
|
VALUES ('full', 'online_square', 'pending', 20.00, $1, NOW(), NOW(), $2)
|
|
`, key, userID)
|
|
require.NoError(t, err)
|
|
|
|
origClient := SquareClient
|
|
SquareClient = &definitiveChargeClient{SquareClient: square.NewDevClient(), createErr: structuredSquareErrorWithCode(t, http.StatusPaymentRequired, "CARD_DECLINED")}
|
|
defer func() { SquareClient = origClient }()
|
|
|
|
req := BuyGiftCardRequest{
|
|
Amount: 2000,
|
|
RecipientType: "self",
|
|
CardID: &cardID,
|
|
IdempotencyKey: key,
|
|
VerificationCode: "334455",
|
|
}
|
|
|
|
w := makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", req, token, ctx)
|
|
require.Equal(t, http.StatusPaymentRequired, w.Code, w.Body.String())
|
|
|
|
var hash sql.NullString
|
|
require.NoError(t, tx.QueryRow(ctx, "SELECT two_factor_pending_code_hash FROM users WHERE id = $1", userID).Scan(&hash))
|
|
require.True(t, hash.Valid, "a pending-reuse retry must not consume the code at the gate")
|
|
require.Equal(t, twofa.Hash("334455"), hash.String, "a failed pending-reuse retry must keep its seeded code unchanged (no re-issue)")
|
|
}
|
|
|
|
// TestReissueTwoFACodeAfterFailedCharge_RespectsMintCooldown pins the
|
|
// LOW-MEDIUM finding 2 contract on the re-issue path: the re-issue mints a
|
|
// live code after a FRESH charge consumed one at the gate, respects the same
|
|
// per-user mint cooldown as the interactive mint endpoints (a second re-issue
|
|
// inside the window is a no-op), and runs again once the cooldown elapses
|
|
// (simulated by clearing the shared stamp the way a successful verify does).
|
|
func TestReissueTwoFACodeAfterFailedCharge_RespectsMintCooldown(t *testing.T) {
|
|
t.Setenv("REQUIRE_2FA", "true")
|
|
t.Setenv("SQUARE_ENVIRONMENT", "staging")
|
|
ctx, tx := testutils.SetupTestTx(t)
|
|
|
|
userID, err := fixtures.CreateTestUser(tx)
|
|
require.NoError(t, err)
|
|
|
|
reissueTwoFACodeAfterFailedCharge(ctx, db.Conn, userID, true, true, nil)
|
|
var hash sql.NullString
|
|
require.NoError(t, tx.QueryRow(ctx, "SELECT two_factor_pending_code_hash FROM users WHERE id = $1", userID).Scan(&hash))
|
|
require.True(t, hash.Valid, "a failed fresh saved-card charge must re-issue a live code for the same-key retry")
|
|
firstHash := hash.String
|
|
|
|
reissueTwoFACodeAfterFailedCharge(ctx, db.Conn, userID, true, true, nil)
|
|
require.NoError(t, tx.QueryRow(ctx, "SELECT two_factor_pending_code_hash FROM users WHERE id = $1", userID).Scan(&hash))
|
|
require.Equal(t, firstHash, hash.String, "a re-issue inside the mint cooldown must be a no-op (the stored code is untouched)")
|
|
|
|
st := twofa.StateFor(userID)
|
|
st.Mu.Lock()
|
|
st.LastMintAt = time.Time{}
|
|
st.Mu.Unlock()
|
|
reissueTwoFACodeAfterFailedCharge(ctx, db.Conn, userID, true, true, nil)
|
|
require.NoError(t, tx.QueryRow(ctx, "SELECT two_factor_pending_code_hash FROM users WHERE id = $1", userID).Scan(&hash))
|
|
require.NotEqual(t, firstHash, hash.String, "an out-of-window re-issue must mint a fresh code")
|
|
}
|