Files
Crussell/backend/handlers/payments/handlers_round9_fixes_test.go
T
popertotsandSisyphus 1d6d3e2f8d test: round-9/10 adversarial suites — sync-path vs webhook completion, gift-card double-refund, sweep VAT, till lock, account lockout + erasure
- handlers_round9/round10: status-guarded flips, split-key hashing, cross-booking key 409, existingCount refund exclusion, SCA save-card exemption, routeNonCompletedPayment, no phantom split rows
- giftcards_round10: saved-card SCA buy, cancel resume reconcile (pending blocks, diff-only re-issue, no over-refund)
- sweep/till_round10: split-accurate VAT, all-tip VAT-free, status/key-changed skip, final-key lock held across charge
- webhooks_round8/9: booking gate + M2, payable side-effects, unknown-event 503, refund-before-row 503, no double-complete after sync
- account_round9: password lockout budgets, S3 erasure outbox, DAV in-tx deletion

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
2026-08-22 00:34:51 +01:00

408 lines
20 KiB
Go
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
//go:build test && dev
package payments
// Round-9 money-fix tests: tip-record VAT exclusion (R11), hashed split
// idempotency keys, cross-booking idempotency-key collisions, refund-then-repay
// on the same payment type, the SCA token-like save-gate exemption, and the
// non-COMPLETED Square payment classification. These tests exercise the shared
// package mock and the handler integration paths, so they follow the existing
// sequential (non-parallel) conventions of the money-path tests they guard.
import (
"database/sql"
"net/http"
"testing"
"crussell/internal/square"
"crussell/testutils"
"crussell/testutils/jwt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestCreateBookingPayment_VATApplied_OverflowTipSkipped locks the R11 fix: an
// online full payment whose overflow carves a payment_type='tip' split record
// must NOT have VAT applied to the tip portion. vat.go's single-source policy
// excludes tip payments entirely (ApplyVATToBookingPayment), and every other
// tip path (CreateTipPayment, the terminal sweep rescue) applies VAT to the
// booking portion only — the post-charge split loop must match. A £55 charge on
// a £50 past-start booking splits into [£50 booking portion, £5 tip]; the tip
// record keeps vat_amount NULL while the booking portion gets VAT, and the two
// records still partition the charged amount exactly.
func TestCreateBookingPayment_VATApplied_OverflowTipSkipped(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
// Enable VAT for this test's transaction (the handler routes through the
// same per-test tx via the context savepoint, so the change is visible).
_, err := tx.Exec(ctx, `UPDATE business_settings SET is_vat_registered = TRUE, default_vat_rate = 20.00`)
require.NoError(t, err)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
cardToken := "cnon:round9-vat-tip-overflow"
req := CreateBookingPaymentRequest{
Amount: 5500, // £55 on a £50 booking — £5 overflows into a tip record
PaymentType: "full",
NewCardToken: &cardToken,
IdempotencyKey: "round9-vat-tip-overflow-" + bookingID,
ConfirmOverflowTip: true, // B12: a post-start overflow always requires explicit confirmation
}
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusOK, w.Code, "overflow payment must succeed, body: %s", w.Body.String())
rows, err := tx.Query(ctx, `
SELECT payment_type, amount, is_vat_applicable, vat_amount, net_amount
FROM payments
WHERE booking_id = $1 AND status = 'completed'
ORDER BY payment_type
`, bookingID)
require.NoError(t, err)
defer rows.Close()
var bookingVAT, tipVAT sql.NullFloat64
var bookingNet, tipNet sql.NullFloat64
var bookingVATFlag, tipVATFlag bool
var bookingAmount, tipAmount float64
var gotBooking, gotTip bool
for rows.Next() {
var pt string
var amt float64
var applicable bool
var vat, net sql.NullFloat64
require.NoError(t, rows.Scan(&pt, &amt, &applicable, &vat, &net))
switch pt {
case "tip":
gotTip = true
tipAmount = amt
tipVAT = vat
tipVATFlag = applicable
tipNet = net
case "full":
gotBooking = true
bookingAmount = amt
bookingVAT = vat
bookingVATFlag = applicable
bookingNet = net
}
}
require.NoError(t, rows.Err())
require.True(t, gotBooking, "the booking portion record must exist")
require.True(t, gotTip, "the tip split record must exist")
// The tip portion is gratuity, never VAT-applicable.
require.False(t, tipVATFlag, "tip record must never be VAT-applicable")
require.False(t, tipVAT.Valid, "tip record must have vat_amount NULL — the fix skips it in the apply_vat_to_payment loop")
require.False(t, tipNet.Valid, "tip record must have net_amount NULL")
// The booking portion carries VAT at the 20% rate (net = 50/1.2 = 41.67,
// vat = 50 41.67 = 8.33).
require.True(t, bookingVATFlag, "the booking portion must be VAT-applicable")
require.True(t, bookingVAT.Valid, "booking portion vat_amount must be set")
require.InDelta(t, 8.33, bookingVAT.Float64, 0.01, "£50 at 20% → £8.33 VAT")
require.True(t, bookingNet.Valid, "booking portion net_amount must be set")
// The split records still partition the charged amount exactly.
require.InDelta(t, 5500, int64((bookingAmount+tipAmount)*100), 0.5, "booking portion + tip must equal the £55 charge")
require.InDelta(t, 50.0, bookingAmount, 0.001, "booking portion = the £50 booking value")
require.InDelta(t, 5.0, tipAmount, 0.001, "tip portion = the £5 overflow")
}
// TestSplitIdempotencyKey_DistinctBases_DistinctKeys locks the R12 fix for
// splitIdempotencyKey: two distinct terminal base keys that share a long common
// prefix (e.g. Square payment IDs differing only in their tail) must derive
// DISTINCT keys. The old raw `base[:maxBase]` prefix cut collapsed such bases
// onto the same key, 500-ing the second insert on the UNIQUE(idempotency_key)
// index. The keys must also stay within the payments.idempotency_key VARCHAR(64)
// column (and, via truncateIdempotencyKey, within Square's 45-char cap).
func TestSplitIdempotencyKey_DistinctBases_DistinctKeys(t *testing.T) {
// Both bases are long enough that a raw prefix cut would keep only the
// identical leading bytes — the OLD code returned the same key for both.
base1 := "pay-booking-000000000001-deposit-2500-pay_mock_12345678901234567890"
base2 := "pay-booking-000000000001-deposit-2500-pay_mock_12345678901234567899"
k1 := splitIdempotencyKey(base1, "-split-tip")
k2 := splitIdempotencyKey(base2, "-split-tip")
assert.NotEqual(t, k1, k2,
"two distinct base keys differing only in their tail must derive distinct split keys — the raw prefix cut collapses them")
// The split keys are DB-only (the Square charge already used the primary
// key) — the binding constraint is the payments.idempotency_key VARCHAR(64)
// column, and the hashed base keeps the whole key well under it.
assert.LessOrEqual(t, len(k1), 64, "split key must fit the payments.idempotency_key VARCHAR(64) column")
assert.LessOrEqual(t, len(k2), 64, "split key must fit the payments.idempotency_key VARCHAR(64) column")
// The derivation stays deterministic — a retry of the same base must
// reproduce the same key (same-key dedup relies on it).
assert.Equal(t, k1, splitIdempotencyKey(base1, "-split-tip"), "split key derivation must be deterministic")
// A short candidate is preserved verbatim (≤45 chars), so the existing
// terminal-split tests that assert the "-split-tip" suffix still pass.
short := splitIdempotencyKey("test-key", "-split-tip")
assert.Equal(t, "test-key-split-tip", short, "a short candidate is returned verbatim")
// Distinct suffixes on the same base must stay distinct (the suffix is
// part of the hashed candidate).
tipped := splitIdempotencyKey(base1, "-split-tip")
splitted := splitIdempotencyKey(base1, "-split-2")
assert.NotEqual(t, tipped, splitted, "distinct split suffixes must derive distinct keys")
}
// TestCreateBookingPayment_CrossBookingIdempotencyKey_CollisionRecovers locks
// the R12 fix: a client-supplied idempotency_key that already belongs to a
// payment on a DIFFERENT booking must NOT die on the global
// UNIQUE(idempotency_key) index (500 → frontend same-key retry → forever 500).
// Mirroring the A6 cross-user pattern (giftcards.go), the handler derives a
// fresh deterministic key for the colliding booking and the charge succeeds.
func TestCreateBookingPayment_CrossBookingIdempotencyKey_CollisionRecovers(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
// Booking A pays first and burns the key.
userA, bookingA, _ := setupTestData(t, ctx, tx)
tokenA := jwt.GenerateUserToken(userA)
const sharedKey = "client-key-reused-across-bookings"
reqA := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
NewCardToken: strPtr("cnon:round9-cross-booking-a"),
IdempotencyKey: sharedKey,
}
wA := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingA+"/payment", reqA, tokenA, ctx)
require.Equal(t, http.StatusOK, wA.Code, "booking A must charge, body: %s", wA.Body.String())
// Booking B (different user) reuses the SAME key. Before the fix this
// 500'd on the UNIQUE constraint; after the fix it derives a fresh key and
// charges cleanly.
userB, bookingB, _ := setupTestData(t, ctx, tx)
tokenB := jwt.GenerateUserToken(userB)
reqB := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
NewCardToken: strPtr("cnon:round9-cross-booking-b"),
IdempotencyKey: sharedKey,
}
wB := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingB+"/payment", reqB, tokenB, ctx)
require.Equal(t, http.StatusOK, wB.Code, "the cross-booking key reuse must recover with a fresh key, body: %s", wB.Body.String())
// Booking B got its own payment under a key distinct from the shared one.
var bID string
var bKey string
require.NoError(t, tx.QueryRow(ctx, `
SELECT id, idempotency_key FROM payments WHERE booking_id = $1 AND status = 'completed'
`, bookingB).Scan(&bID, &bKey))
require.NotEqual(t, sharedKey, bKey, "booking B must have been charged under a freshly derived key, not the collided one")
assert.Contains(t, bKey, bookingB, "the fresh key must be derived from booking B's identity")
// Booking A's original payment is untouched and still keyed by the shared key.
var aCount int
require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND idempotency_key = $2 AND status = 'completed'`, bookingA, sharedKey).Scan(&aCount))
require.Equal(t, 1, aCount, "booking A's payment must remain intact under the shared key")
}
// TestCreateBookingPayment_RefundedDeposit_RepaySucceeds locks the R11 fix for
// the existingCount check: a completed payment of the same non-partial type
// whose money is no longer live (it has a completed/pending refund) must NOT
// block a fresh equal-type charge — refund-then-repay is a legitimate flow
// (deriveBookingPaymentIdempotencyKey already rotates past refunded rows).
func TestCreateBookingPayment_RefundedDeposit_RepaySucceeds(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupPaymentStatusTest(t, ctx, tx, "confirmed")
userToken := jwt.GenerateUserToken(userID)
cardToken := "cnon:round9-refund-repay"
req := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
NewCardToken: &cardToken,
IdempotencyKey: "round9-refund-repay-first",
}
w1 := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusOK, w1.Code, "the initial deposit must succeed, body: %s", w1.Body.String())
var paymentID string
require.NoError(t, tx.QueryRow(ctx, `SELECT id FROM payments WHERE booking_id = $1 AND idempotency_key = $2 AND status = 'completed'`, bookingID, req.IdempotencyKey).Scan(&paymentID))
// The deposit is refunded in full — a live (completed) refund row means its
// money is no longer collectable, so the type slot must re-open.
_, err := tx.Exec(ctx, `
INSERT INTO refunds (payment_id, booking_id, amount, status, reason, created_at)
VALUES ($1, $2, 25.00, 'completed', 'admin refund', NOW())
`, paymentID, bookingID)
require.NoError(t, err)
// Same type, fresh key: before the fix the existingCount check counted the
// refunded completed deposit and 409'd; after the fix the refunded row is
// excluded and the repay succeeds.
req2 := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
NewCardToken: &cardToken,
IdempotencyKey: "round9-refund-repay-second",
}
w2 := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req2, userToken, ctx)
require.Equal(t, http.StatusOK, w2.Code, "a refunded deposit must re-open the type slot, body: %s", w2.Body.String())
var secondKeyCount int
require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND idempotency_key = $2 AND status = 'completed'`, bookingID, req2.IdempotencyKey).Scan(&secondKeyCount))
require.Equal(t, 1, secondKeyCount, "the refund-then-repay deposit must be recorded as completed")
// An UN-refunded completed deposit still blocks a second charge — the
// double-charge protection must survive the change.
req3 := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
NewCardToken: &cardToken,
IdempotencyKey: "round9-refund-repay-third",
}
w3 := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req3, userToken, ctx)
require.Equal(t, http.StatusConflict, w3.Code, "an un-refunded completed deposit must still block a second equal-type charge")
}
// TestCreateBookingPayment_SaveCard_SCATokenizeResult_Enforced2FA_PersistsCard
// locks the R13 fix: in an enforced-2FA deployment, a genuine SCA tokenize-result
// charge with save_card=true must skip the SAVE gate (the token was minted by a
// tokenization flow that ran the STORE-intent SCA — the same token-like
// exemption CreatePaymentMethod applies) and persist the card. Before the fix
// the tokenForwardedToSquare=false gate refused 402, making save_card
// unreachable in production.
func TestCreateBookingPayment_SaveCard_SCATokenizeResult_Enforced2FA_PersistsCard(t *testing.T) {
// Any non-mock SQUARE_ENVIRONMENT enforces 2FA (fail-closed).
t.Setenv("REQUIRE_2FA", "true")
t.Setenv("SQUARE_ENVIRONMENT", "staging")
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
// A genuine tokenize-result token (cnon:sca-...) for a NEW card — no saved
// card reference, so scaTokenizedSavedCard is false and the SAVE gate runs.
cardToken := "cnon:sca-round9-save-new-card"
req := CreateBookingPaymentRequest{
Amount: 5000,
PaymentType: "full",
NewCardToken: &cardToken,
SaveCard: true,
IdempotencyKey: "round9-sca-save-enforced-" + bookingID,
}
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusOK, w.Code, "a token-like SCA save must skip the 2FA gate in an enforced deployment, body: %s", w.Body.String())
var cardCount int
require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, userID).Scan(&cardCount))
require.Equal(t, 1, cardCount, "the SCA tokenize-result save_card=true charge must persist the card in an enforced env")
}
// TestCreateBookingPayment_NonCompletedSquareStatus_StaysPending locks the R11
// fix: a CreatePayment that returns nil error but a non-COMPLETED status must
// NOT be recorded as completed. APPROVED/PENDING are non-terminal (mirroring
// the sweep's staleReconcileLeavePending — the row stays pending for a later
// sweep run to re-poll); the handler returns 5xx and no split/completed
// side-effects run.
func TestCreateBookingPayment_NonCompletedSquareStatus_StaysPending(t *testing.T) {
for _, status := range []string{"PENDING", "APPROVED"} {
t.Run(status, func(t *testing.T) {
origClient := SquareClient
mock := square.NewDevClient().(*square.MockClient)
mock.ForcePaymentStatus = status
SquareClient = mock
defer func() { SquareClient = origClient }()
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
req := CreateBookingPaymentRequest{
Amount: 5000,
PaymentType: "full",
NewCardToken: strPtr("cnon:round9-nonterminal-" + status),
IdempotencyKey: "round9-nonterminal-" + status + "-" + bookingID,
}
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusInternalServerError, w.Code, "%q must NOT be reported as success, body: %s", status, w.Body.String())
var rowStatus string
var rowCount int
require.NoError(t, tx.QueryRow(ctx, `SELECT status FROM payments WHERE booking_id = $1 AND idempotency_key = $2`, bookingID, req.IdempotencyKey).Scan(&rowStatus))
require.Equal(t, "pending", rowStatus, "a non-terminal %q payment must stay pending (staleReconcileLeavePending semantics)", status)
require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1`, bookingID).Scan(&rowCount))
require.Equal(t, 1, rowCount, "no split records may be created for a non-completed payment")
// The booking is untouched — no promotion/completion side-effects.
var bookingStatus string
require.NoError(t, tx.QueryRow(ctx, `SELECT status FROM bookings WHERE id = $1`, bookingID).Scan(&bookingStatus))
require.Equal(t, "in_progress", bookingStatus, "a non-terminal payment must not promote or complete the booking")
})
}
}
// TestCreateBookingPayment_TerminalNonSuccessSquareStatus_MarksFailed locks the
// R11 fix for terminal non-success statuses: a CANCELED/FAILED payment never
// landed at Square, so the row is marked failed (a same-key retry can never
// issue a second charge under the key) and the handler surfaces the failure.
func TestCreateBookingPayment_TerminalNonSuccessSquareStatus_MarksFailed(t *testing.T) {
for _, status := range []string{"CANCELED", "FAILED"} {
t.Run(status, func(t *testing.T) {
origClient := SquareClient
mock := square.NewDevClient().(*square.MockClient)
mock.ForcePaymentStatus = status
SquareClient = mock
defer func() { SquareClient = origClient }()
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
req := CreateBookingPaymentRequest{
Amount: 5000,
PaymentType: "full",
NewCardToken: strPtr("cnon:round9-terminal-" + status),
IdempotencyKey: "round9-terminal-" + status + "-" + bookingID,
}
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusPaymentRequired, w.Code, "%q must surface as a charge failure, body: %s", status, w.Body.String())
var rowStatus string
require.NoError(t, tx.QueryRow(ctx, `SELECT status FROM payments WHERE booking_id = $1 AND idempotency_key = $2`, bookingID, req.IdempotencyKey).Scan(&rowStatus))
require.Equal(t, "failed", rowStatus, "a terminal non-success %q payment must be marked failed", status)
})
}
}
// TestCreateTipPayment_NonCompletedSquareStatus_StaysPending locks the R11 fix
// on the tip path too: a tip charge that returns non-COMPLETED must stay pending
// (never flipped to completed), so the sweep can reconcile it.
func TestCreateTipPayment_NonCompletedSquareStatus_StaysPending(t *testing.T) {
origClient := SquareClient
mock := square.NewDevClient().(*square.MockClient)
mock.ForcePaymentStatus = "PENDING"
SquareClient = mock
defer func() { SquareClient = origClient }()
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
// The tip flow requires an existing completed payment on the booking.
_, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, status, amount, idempotency_key, created_by, created_at, updated_at)
VALUES ($1, 'full', 'online_square', 'completed', 50.00, $2, $3, NOW(), NOW())
`, bookingID, "round9-tip-primary-"+bookingID, userID)
require.NoError(t, err)
req := CreateTipPaymentRequest{
Amount: 500,
NewCardToken: strPtr("cnon:round9-tip-nonterminal"),
IdempotencyKey: "round9-tip-nonterminal-" + bookingID,
}
w := makePaymentRequest(CreateTipPayment, "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, ctx)
require.Equal(t, http.StatusInternalServerError, w.Code, "a non-COMPLETED tip must not be reported as success, body: %s", w.Body.String())
var rowStatus string
require.NoError(t, tx.QueryRow(ctx, `SELECT status FROM payments WHERE booking_id = $1 AND idempotency_key = $2`, bookingID, req.IdempotencyKey).Scan(&rowStatus))
require.Equal(t, "pending", rowStatus, "a non-terminal tip payment must stay pending for the sweep to reconcile")
}