popertots/Crussell
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: payments hardening — SCA wire contract (saved-card ref + tokenize-result), terminal/till token routing, tip-cap overflow carve, completion campaign atomicity, orphan B1-evidence gate, gift-card gates/locks, admin backstops

Browse Source 
- ValidateCardInfo accepts saved-card ref + new_card_token coexistence (matches resolveChargeSource); new_card_token added to terminal/till request structs so SCA tokens are never dropped
- maxOnlineTipPence (£250) enforced on the overflow-tip carve AND buildSplitRecords (both carve paths) — closes the £10k bypass
- completion-path campaign increments made atomic reserve-first (conditional UPDATE ... RETURNING) + schema backstops (chk_times_redeemed, partial unique index on milestone redemptions)
- webhook orphan detection gated on B1 evidence (b1_attempts / sweep-duplicate refund row) so a delayed legit completion is never marked failed
- gift-card: per-user £500/day cap lock held across read-modify-write, expired-card top-up gate, NaN/Inf float bounds, refund_failed ack filter, on_the_house excluded from balance, postChargeRecheck notification
- admin apply-redemption route + admin-or-owner, in-handler isAdminRequest on 4 gift-card handlers, tip lock key aligned
- 2FA fallback machinery removed (insertTwoFAFallbackAudit/reissue/consent), dead fields stripped from charge structs
- tests: prod-tag suite, mock SCA parity, tip-cap overflow, completion races, cards pagination, ValidateCardInfo tables
This commit is contained in:
popertots
2026-08-22 00:34:50 +01:00
parent 1d9c87d6d6
commit 1429eddd34
43 changed files with 2211 additions and 1275 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/handlers/payments/charge_helpers.go
+12
View File
@@ -302,6 +302,18 @@ func postChargeRecheck(ctx context.Context, w http.ResponseWriter, tx pgx.Tx, bo
log.Printf("CRITICAL: Square payment %s (ID=%s) landed on %q booking %s and committing the failed mark errored: %v — manual reconciliation required",
sqStatus, sqPayID, recheckStatus, bookingID, cErr)
}
// The CRITICAL log line alone was the only operator signal for a
// stranded charge — no admin-visible trace. Raise the flood-capped
// critical-payment admin notification (sweep.go's
// insertCriticalPaymentNotification, which applies the shared
// unacknowledged-queue cap atomically and dedups per
// booking/user). It runs AFTER the commit: this transaction held the
// booking row FOR UPDATE (recheckBookingPayable), and the notification
// INSERT's FK check on bookings(id) takes FOR KEY SHARE — inserting
// before the commit would self-deadlock against this tx's own booking
// lock (the sweep's gateStalePaymentRescueOnBooking commits first for
// the same reason).
insertCriticalPaymentNotification(ctx, &bookingID, nil)
http.Error(w, conflictMsg, http.StatusConflict)
return false, nil
}
backend/handlers/payments/completion.go
+129 -65
View File
@@ -157,24 +157,42 @@ func ApplyBookingCompletionSideEffects(ctx context.Context, tx pgx.Tx, bookingID
if capped, ok := capDiscountToRemainingObligation(ctx, tx, bookingID, discountAmount); ok {
discountAmount = capped
if _, err := tx.Exec(ctx, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'time_based', NULL, $4, $5, $6)
`, bookingID, userID, campaignID, campaignPercent, bookingTotal, discountAmount); err != nil {
log.Printf("ALERT: failed to insert booking discount: %v", err)
}
// B13 atomic reservation FIRST — mirror the apply-at-payment
// path (discounts.go ApplyEligibleDiscount). The eligibility
// SELECT above is a plain read; a concurrent completion on
// another booking can exhaust the campaign between that read
// and here. The conditional UPDATE only increments while the
// campaign still has headroom (PostgreSQL re-evaluates the
// WHERE against the post-lock row), so exactly one concurrent
// completion wins the redemption. On pgx.ErrNoRows the
// discount is SKIPPED — the completion still succeeds, we just
// log and move on rather than minting a discount row for a
// redemption that never happened.
var reservedID string
if err := tx.QueryRow(ctx, `
UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1
WHERE id = $1 AND (max_redemptions IS NULL OR times_redeemed < max_redemptions)
RETURNING id
`, campaignID).Scan(&reservedID); err != nil {
if errors.Is(err, pgx.ErrNoRows) {
log.Printf("Skipping time_based campaign %s at completion for booking %s — campaign fully redeemed by a concurrent redemption", campaignID, bookingID)
} else {
log.Printf("ALERT: failed to reserve redemption for campaign %s at completion for booking %s: %v", campaignID, bookingID, err)
}
} else {
if _, err := tx.Exec(ctx, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'time_based', NULL, $4, $5, $6)
`, bookingID, userID, campaignID, campaignPercent, bookingTotal, discountAmount); err != nil {
log.Printf("ALERT: failed to insert booking discount: %v", err)
}
if _, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
}
if _, err := tx.Exec(ctx, `
UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1 WHERE id = $1
`, campaignID); err != nil {
log.Printf("ALERT: failed to update discount campaign usage: %v", err)
if _, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
}
}
} else {
log.Printf("Skipping time_based campaign %s at completion for booking %s — obligation already covered by real money (would over-credit)", campaignID, bookingID)
@@ -204,22 +222,43 @@ func ApplyBookingCompletionSideEffects(ctx context.Context, tx pgx.Tx, bookingID
// F1 over-credit guard — see the time-based block above.
if capped, ok := capDiscountToRemainingObligation(ctx, tx, bookingID, discountAmount); ok {
discountAmount = capped
if _, err := tx.Exec(ctx, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'milestone', 'per_user_booking_count', $4, $5, $6)
`, bookingID, userID, milestoneCampaignID, milestonePercent, bookingTotal, discountAmount); err != nil {
log.Printf("ALERT: failed to insert booking discount: %v", err)
}
if _, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
}
if _, err := tx.Exec(ctx, `
UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1 WHERE id = $1
`, milestoneCampaignID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
// B13 atomic reservation FIRST — see the time-based block above.
var reservedID string
if err := tx.QueryRow(ctx, `
UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1
WHERE id = $1 AND (max_redemptions IS NULL OR times_redeemed < max_redemptions)
RETURNING id
`, milestoneCampaignID).Scan(&reservedID); err != nil {
if errors.Is(err, pgx.ErrNoRows) {
log.Printf("Skipping per-user milestone campaign %s at completion for booking %s — campaign fully redeemed by a concurrent redemption", milestoneCampaignID, bookingID)
} else {
log.Printf("ALERT: failed to reserve redemption for campaign %s at completion for booking %s: %v", milestoneCampaignID, bookingID, err)
}
} else {
// Once-per-user backstop: the eligibility NOT EXISTS above
// is a plain read, so two concurrent completions of
// DIFFERENT bookings of this user can both pass it. The
// partial unique index uq_booking_discounts_user_milestone_campaign
// on (user_id, source_id) for milestone campaigns is the
// schema backstop — the second INSERT is suppressed by
// ON CONFLICT DO NOTHING and the discount is skipped.
tag, err := tx.Exec(ctx, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'milestone', 'per_user_booking_count', $4, $5, $6)
ON CONFLICT (user_id, source_id) WHERE discount_source = 'campaign' AND milestone_type IN ('per_user_booking_count', 'anniversary') DO NOTHING
`, bookingID, userID, milestoneCampaignID, milestonePercent, bookingTotal, discountAmount)
if err != nil {
log.Printf("ALERT: failed to insert booking discount: %v", err)
} else if tag.RowsAffected() == 0 {
log.Printf("Per-user milestone campaign %s already applied for user %s — skipping duplicate at completion for booking %s", milestoneCampaignID, userID, bookingID)
} else {
if _, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
}
}
}
} else {
log.Printf("Skipping per-user milestone campaign %s at completion for booking %s — obligation already covered by real money (would over-credit)", milestoneCampaignID, bookingID)
@@ -260,22 +299,31 @@ func ApplyBookingCompletionSideEffects(ctx context.Context, tx pgx.Tx, bookingID
// F1 over-credit guard — see the time-based block above.
if capped, ok := capDiscountToRemainingObligation(ctx, tx, bookingID, discountAmount); ok {
discountAmount = capped
if _, err := tx.Exec(ctx, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'milestone', 'global_booking_count', $4, $5, $6)
`, bookingID, userID, globalCampaignID, globalPercent, bookingTotal, discountAmount); err != nil {
log.Printf("ALERT: failed to insert booking discount: %v", err)
}
if _, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
}
if _, err := tx.Exec(ctx, `
UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1 WHERE id = $1
`, globalCampaignID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
// B13 atomic reservation FIRST — see the time-based block above.
var reservedID string
if err := tx.QueryRow(ctx, `
UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1
WHERE id = $1 AND (max_redemptions IS NULL OR times_redeemed < max_redemptions)
RETURNING id
`, globalCampaignID).Scan(&reservedID); err != nil {
if errors.Is(err, pgx.ErrNoRows) {
log.Printf("Skipping global milestone campaign %s at completion for booking %s — campaign fully redeemed by a concurrent redemption", globalCampaignID, bookingID)
} else {
log.Printf("ALERT: failed to reserve redemption for campaign %s at completion for booking %s: %v", globalCampaignID, bookingID, err)
}
} else {
if _, err := tx.Exec(ctx, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'milestone', 'global_booking_count', $4, $5, $6)
`, bookingID, userID, globalCampaignID, globalPercent, bookingTotal, discountAmount); err != nil {
log.Printf("ALERT: failed to insert booking discount: %v", err)
}
if _, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
}
}
} else {
log.Printf("Skipping global milestone campaign %s at completion for booking %s — obligation already covered by real money (would over-credit)", globalCampaignID, bookingID)
@@ -331,22 +379,38 @@ func ApplyBookingCompletionSideEffects(ctx context.Context, tx pgx.Tx, bookingID
// F1 over-credit guard — see the time-based block above.
if capped, ok := capDiscountToRemainingObligation(ctx, tx, bookingID, discountAmount); ok {
discountAmount = capped
if _, err := tx.Exec(ctx, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'milestone', 'anniversary', $4, $5, $6)
`, bookingID, userID, c.id, c.pct, bookingTotal, discountAmount); err != nil {
log.Printf("ALERT: failed to insert booking discount: %v", err)
}
if _, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
}
if _, err := tx.Exec(ctx, `
UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1 WHERE id = $1
`, c.id); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
// B13 atomic reservation FIRST — see the time-based block above.
var reservedID string
if err := tx.QueryRow(ctx, `
UPDATE discount_campaigns SET times_redeemed = times_redeemed + 1
WHERE id = $1 AND (max_redemptions IS NULL OR times_redeemed < max_redemptions)
RETURNING id
`, c.id).Scan(&reservedID); err != nil {
if errors.Is(err, pgx.ErrNoRows) {
log.Printf("Skipping anniversary campaign %s at completion for booking %s — campaign fully redeemed by a concurrent redemption", c.id, bookingID)
} else {
log.Printf("ALERT: failed to reserve redemption for campaign %s at completion for booking %s: %v", c.id, bookingID, err)
}
} else {
// Once-per-user backstop — see the per-user
// milestone block above.
tag, err := tx.Exec(ctx, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'milestone', 'anniversary', $4, $5, $6)
ON CONFLICT (user_id, source_id) WHERE discount_source = 'campaign' AND milestone_type IN ('per_user_booking_count', 'anniversary') DO NOTHING
`, bookingID, userID, c.id, c.pct, bookingTotal, discountAmount)
if err != nil {
log.Printf("ALERT: failed to insert booking discount: %v", err)
} else if tag.RowsAffected() == 0 {
log.Printf("Anniversary campaign %s already applied for user %s — skipping duplicate at completion for booking %s", c.id, userID, bookingID)
} else {
if _, err := tx.Exec(ctx, `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
log.Printf("ALERT: failed to insert payment record: %v", err)
}
}
}
} else {
log.Printf("Skipping anniversary campaign %s at completion for booking %s — obligation already covered by real money (would over-credit)", c.id, bookingID)
backend/handlers/payments/completion_test.go
+280
View File
@@ -4,11 +4,16 @@ package payments
import (
"context"
"sync"
"testing"
"time"
"crussell/db"
"crussell/testutils"
"crussell/testutils/fixtures"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestCompleteActiveBookingFromPayment_RefusesCancelledBooking locks the M2
@@ -83,3 +88,278 @@ func TestCompleteFullyPaidBooking_CompletesPayableBooking(t *testing.T) {
t.Errorf("expected the fully-paid payable booking completed, got %q", status)
}
}
// TestCompletion_ConcurrentCompletions_CampaignMaxRedemptions proves the B13
// atomic reservation in the completion path (completion.go): two concurrent
// completions of two DIFFERENT bookings of the same user both pass the
// eligibility SELECT while a max_redemptions=1 campaign still has headroom, but
// only ONE may win the conditional reservation UPDATE. The loser matches zero
// rows (pgx.ErrNoRows) and SKIPS the discount — the booking still completes.
// times_redeemed must end at exactly 1, never 2.
func TestCompletion_ConcurrentCompletions_CampaignMaxRedemptions(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)
bookingIDs := make([]string, 2)
for i := range bookingIDs {
bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID,
time.Date(2099, 12, 31, 10, i+1, 0, 0, time.UTC))
require.NoError(t, err)
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)
}
bookingIDs[i] = bookingID
}
pool := context.Background()
t.Cleanup(func() {
for _, bid := range bookingIDs {
_, _ = db.Conn.Exec(pool, `DELETE FROM booking_discounts WHERE booking_id = $1`, bid)
_, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE booking_id = $1`, bid)
_, _ = db.Conn.Exec(pool, `DELETE FROM booking_services WHERE booking_id = $1`, bid)
_, _ = db.Conn.Exec(pool, `DELETE FROM bookings WHERE id = $1`, bid)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM discount_campaigns WHERE name = 'Completion Race Campaign'`)
_, _ = db.Conn.Exec(pool, `DELETE FROM services WHERE id = $1`, serviceID)
_, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, userID)
})
// Commit the setup so both goroutines complete at pool level on independent
// connections — a shared per-test tx would serialize them.
pgxTx := db.TxFromContext(ctx)
require.NotNil(t, pgxTx, "no transaction in context")
require.NoError(t, pgxTx.Commit(ctx), "failed to commit setup tx")
var campaignID string
require.NoError(t, db.Conn.QueryRow(pool, `
INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, max_redemptions, times_redeemed)
VALUES ('Completion Race Campaign', 'time_based', 10, 'active', NOW() - INTERVAL '1 day', NOW() + INTERVAL '1 day', 1, 0)
RETURNING id
`).Scan(&campaignID))
var wg sync.WaitGroup
startBoth := make(chan struct{})
commitErrs := make([]error, 2)
for i, bid := range bookingIDs {
wg.Add(1)
go func(idx int, booking string) {
defer wg.Done()
<-startBoth
cctx := context.Background()
gtx, err := db.Conn.Begin(cctx)
if err != nil {
commitErrs[idx] = err
return
}
defer func() { _ = gtx.Rollback(cctx) }()
completeActiveBookingFromPayment(cctx, gtx, booking)
commitErrs[idx] = gtx.Commit(cctx)
}(i, bid)
}
close(startBoth)
wg.Wait()
for i, err := range commitErrs {
require.NoError(t, err, "completion tx %d must commit", i)
}
// Both bookings must complete regardless of who won the discount race.
for _, bid := range bookingIDs {
var status string
require.NoError(t, db.Conn.QueryRow(pool, "SELECT status FROM bookings WHERE id = $1", bid).Scan(&status))
assert.Equal(t, "completed", status, "a discount race must never fail a booking completion")
}
// Exactly ONE booking got the campaign discount.
var discountCount int
require.NoError(t, db.Conn.QueryRow(pool, `
SELECT COUNT(*) FROM booking_discounts
WHERE source_id = $1 AND discount_source = 'campaign' AND campaign_type = 'time_based'
`, campaignID).Scan(&discountCount))
assert.Equal(t, 1, discountCount, "exactly one completion must win the max_redemptions=1 campaign")
// The redemption counter never over-increments past max_redemptions.
var redeemed int
require.NoError(t, db.Conn.QueryRow(pool, "SELECT times_redeemed FROM discount_campaigns WHERE id = $1", campaignID).Scan(&redeemed))
assert.Equal(t, 1, redeemed, "times_redeemed must end at 1, never 2")
}
// TestCompletion_CampaignExhaustedAtReservation_SkipsDiscount locks the
// pgx.ErrNoRows skip branch of the completion's conditional reservation
// deterministically. A concurrent redemption exhausts the campaign
// (times_redeemed = max_redemptions) in an uncommitted transaction AFTER the
// completion's eligibility SELECT reads headroom but BEFORE its conditional
// UPDATE. Under READ COMMITTED the blocked UPDATE re-evaluates its WHERE
// against the post-increment row, matches zero rows, and the completion must
// STILL complete the booking while SKIPPING the discount — no booking_discounts
// row, no discount payment, counter unchanged.
func TestCompletion_CampaignExhaustedAtReservation_SkipsDiscount(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, serviceID := setupTestData(t, ctx, tx)
var campaignID string
require.NoError(t, tx.QueryRow(ctx, `
INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, max_redemptions, times_redeemed)
VALUES ('Exhausted At Completion', 'time_based', 10, 'active', NOW() - INTERVAL '1 day', NOW() + INTERVAL '1 day', 1, 0)
RETURNING id
`).Scan(&campaignID))
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 booking_discounts WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM booking_services WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM bookings WHERE id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM discount_campaigns WHERE id = $1`, campaignID)
_, _ = db.Conn.Exec(pool, `DELETE FROM services WHERE id = $1`, serviceID)
_, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, userID)
})
// A separate transaction exhausts the campaign (times_redeemed = 1) WITHOUT
// committing — its row lock makes the completion's conditional UPDATE block
// until it is released, while READ COMMITTED keeps the completion's earlier
// eligibility SELECT reading the old committed value (0). That is exactly
// the read-then-write window the race exploits.
htx, err := db.Conn.Begin(pool)
require.NoError(t, err, "failed to begin holder tx")
defer func() { _ = htx.Rollback(pool) }()
_, err = htx.Exec(pool, `UPDATE discount_campaigns SET times_redeemed = 1 WHERE id = $1`, campaignID)
require.NoError(t, err, "failed to exhaust campaign on holder tx")
// Launch the completion on another connection — it passes the eligibility
// SELECT and blocks on the conditional UPDATE. Releasing the holder lets the
// blocked UPDATE re-evaluate and miss.
done := make(chan struct{})
var completeErr error
go func() {
defer close(done)
cctx := context.Background()
gtx, err := db.Conn.Begin(cctx)
if err != nil {
completeErr = err
return
}
defer func() { _ = gtx.Rollback(cctx) }()
completeActiveBookingFromPayment(cctx, gtx, bookingID)
completeErr = gtx.Commit(cctx)
}()
// Give the completion time to reach the blocked conditional UPDATE.
time.Sleep(300 * time.Millisecond)
require.NoError(t, htx.Commit(pool), "failed to release holder tx")
select {
case <-done:
case <-time.After(30 * time.Second):
t.Fatal("completion did not finish after holder release (deadlock?)")
}
require.NoError(t, completeErr, "completion must succeed despite the exhausted campaign")
var status string
require.NoError(t, db.Conn.QueryRow(pool, "SELECT status FROM bookings WHERE id = $1", bookingID).Scan(&status))
assert.Equal(t, "completed", status, "a discount race must never fail the booking completion")
var discountCount int
require.NoError(t, db.Conn.QueryRow(pool, `SELECT COUNT(*) FROM booking_discounts WHERE booking_id = $1`, bookingID).Scan(&discountCount))
assert.Zero(t, discountCount, "the exhausted campaign must NOT be applied at completion")
var redeemed int
require.NoError(t, db.Conn.QueryRow(pool, "SELECT times_redeemed FROM discount_campaigns WHERE id = $1", campaignID).Scan(&redeemed))
assert.Equal(t, 1, redeemed, "times_redeemed stays at the concurrent redemption's 1")
}
// TestCompletion_PerUserMilestone_DuplicateSuppressed locks the once-per-user
// backstop for the per-user milestone (completion.go): the partial unique index
// uq_booking_discounts_user_milestone_campaign + ON CONFLICT DO NOTHING. A
// concurrent completion of ANOTHER of this user's bookings inserts the milestone
// discount row (uncommitted — invisible to this completion's NOT EXISTS
// eligibility read under READ COMMITTED). This completion's own INSERT then
// blocks on the conflicting unique index entry and is suppressed after the
// concurrent winner commits: exactly ONE milestone row exists for the user, no
// discount payment is minted, and the booking still completes.
func TestCompletion_PerUserMilestone_DuplicateSuppressed(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, serviceID := setupTestData(t, ctx, tx)
var campaignID string
require.NoError(t, tx.QueryRow(ctx, `
INSERT INTO discount_campaigns (name, campaign_type, discount_percent, status, start_date, end_date, milestone_type, milestone_value, max_redemptions)
VALUES ('Milestone At Completion', 'milestone', 15, 'active', NOW() - INTERVAL '1 day', NOW() + INTERVAL '1 day', 'per_user_booking_count', 1, 10)
RETURNING id
`).Scan(&campaignID))
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 booking_discounts WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM booking_services WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM bookings WHERE id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM discount_campaigns WHERE id = $1`, campaignID)
_, _ = db.Conn.Exec(pool, `DELETE FROM services WHERE id = $1`, serviceID)
_, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, userID)
})
// The concurrent winner already inserted the milestone row for this user
// (uncommitted — the completion's NOT EXISTS eligibility read passes, its
// INSERT then blocks on the unique index entry).
htx, err := db.Conn.Begin(pool)
require.NoError(t, err, "failed to begin holder tx")
defer func() { _ = htx.Rollback(pool) }()
_, err = htx.Exec(pool, `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'campaign', $3, 'milestone', 'per_user_booking_count', 15, 50.00, 7.50)
`, bookingID, userID, campaignID)
require.NoError(t, err, "failed to insert concurrent milestone discount on holder tx")
done := make(chan struct{})
var completeErr error
go func() {
defer close(done)
cctx := context.Background()
gtx, err := db.Conn.Begin(cctx)
if err != nil {
completeErr = err
return
}
defer func() { _ = gtx.Rollback(cctx) }()
completeActiveBookingFromPayment(cctx, gtx, bookingID)
completeErr = gtx.Commit(cctx)
}()
// Give the completion time to reach the blocked booking_discounts INSERT.
time.Sleep(300 * time.Millisecond)
require.NoError(t, htx.Commit(pool), "failed to release holder tx")
select {
case <-done:
case <-time.After(30 * time.Second):
t.Fatal("completion did not finish after holder release (deadlock?)")
}
require.NoError(t, completeErr, "completion must succeed despite the suppressed duplicate")
var status string
require.NoError(t, db.Conn.QueryRow(pool, "SELECT status FROM bookings WHERE id = $1", bookingID).Scan(&status))
assert.Equal(t, "completed", status, "a duplicate milestone must never fail the booking completion")
var milestoneCount int
require.NoError(t, db.Conn.QueryRow(pool, `
SELECT COUNT(*) FROM booking_discounts
WHERE user_id = $1 AND source_id = $2 AND discount_source = 'campaign'
`, userID, campaignID).Scan(&milestoneCount))
assert.Equal(t, 1, milestoneCount, "the once-per-user milestone must apply at most once per user")
var payCount int
require.NoError(t, db.Conn.QueryRow(pool, `
SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_method = 'discount'
`, bookingID).Scan(&payCount))
assert.Zero(t, payCount, "the suppressed duplicate must not mint a discount payment")
}
backend/handlers/payments/dst_timezone_test.go
+8 -2
View File
@@ -90,7 +90,10 @@ func TestDepositProtectionWindow_UTCInstant(t *testing.T) {
// protection window -> a single unsplit record.
record := makeTestRecord("b-boundary-past", "full", 50)
info := &BookingPaymentInfo{StartTime: boundaryUTC, TotalAmount: 50, TotalPaid: 0}
records := buildSplitRecords(record, "full", info, 50)
records, err := buildSplitRecords(record, "full", info, 50)
if err != nil {
t.Fatalf("buildSplitRecords: %v", err)
}
if len(records) != 1 {
t.Fatalf("past BST-midnight booking: expected 1 record (no split), got %d", len(records))
}
@@ -109,7 +112,10 @@ func TestDepositProtectionWindow_UTCInstant(t *testing.T) {
record2 := makeTestRecord("b-boundary-future", "full", 50)
info2 := &BookingPaymentInfo{StartTime: futureBoundary, TotalAmount: 50, TotalPaid: 0}
records2 := buildSplitRecords(record2, "full", info2, 50)
records2, err2 := buildSplitRecords(record2, "full", info2, 50)
if err2 != nil {
t.Fatalf("buildSplitRecords: %v", err2)
}
if len(records2) != 2 {
t.Fatalf("future BST-midnight booking: expected 2 records (deposit split), got %d", len(records2))
}
backend/handlers/payments/errors_test.go
+7 -5
View File
@@ -730,9 +730,11 @@ func TestVerificationRequiredSurfacing_AllChargeSites(t *testing.T) {
}
// the dedicated add-card endpoint: with REQUIRE_2FA enforced, persisting a card
// is refused 402 verification_required (SCA-only — the 2FA fallback was
// removed) and no card row is created — the save-card endpoint is not an
// un-gated side door.
// from a NON-token-like source (a raw PAN — the only shape the gate still
// refuses) is rejected 402 verification_required (SCA-only — the 2FA fallback
// was removed) and no card row is created — the save-card endpoint is not an
// un-gated side door. A genuine Square token-like card token (cnon:/ccof:) is
// SCA-proven and skips the gate (see TestTwoFactorEnforced_CreatePaymentMethod_SCATokenizeResult_Save_Succeeds).
func TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_402(t *testing.T) {
t.Setenv("REQUIRE_2FA", "true")
t.Setenv("SQUARE_ENVIRONMENT", "production")
@@ -745,7 +747,7 @@ func TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_402(t *testing.T) {
token := jwt.GenerateUserToken(userID)
handler := CreatePaymentMethod
w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "cnon:2fa-blocked"}, token, ctx)
w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "4111111111111111"}, token, ctx)
if w.Code != http.StatusPaymentRequired {
t.Fatalf("expected 402 verification_required when 2FA is enforced (SCA-only), got %d: %s", w.Code, w.Body.String())
}
@@ -782,7 +784,7 @@ func TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Blocked(t *testing.T) {
token := jwt.GenerateUserToken(userID)
handler := CreatePaymentMethod
w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "cnon:2fa-ok", VerificationCode: "778899"}, token, ctx)
w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "4111111111111111"}, token, ctx)
if w.Code != http.StatusPaymentRequired {
t.Fatalf("expected 402 verification_required even with a valid 2FA code (SCA-only), got %d: %s", w.Code, w.Body.String())
}
backend/handlers/payments/giftcards.go
+153 -61
View File
@@ -117,21 +117,31 @@ type BuyGiftCardRequest struct {
// distinct purchases get different keys. Max=45 matches Square's limit.
IdempotencyKey string `json:"idempotency_key,omitempty" validate:"omitempty,max=45"`
VerificationToken *string `json:"verification_token,omitempty"`
// VerificationCode is the customer's current 2FA one-time code (B10): an
// enforced environment charges/persists a saved card only when this matches
// the customer's pending code.
VerificationCode string `json:"verification_code,omitempty"`
// ConsentVersion / ConsentAccepted: the customer's explicit versioned
// consent to the SCA-unavailable → 2FA fallback (C6), enforced server-side
// (403 consent_required) and recorded on the 2fa_fallback_charge audit row.
ConsentVersion *string `json:"consent_version,omitempty"`
ConsentAccepted bool `json:"consent_accepted"`
}
type RedeemGiftCardRequest struct {
Code string `json:"code"`
}
// giftCardAmountPence validates an admin gift-card transaction amount (pounds
// float64) and converts it to pence. Non-finite values (NaN/Inf) and amounts
// above the £250 per-transaction bound (maxAdminGiftCardTransactionPence) are
// rejected BEFORE the float→int64 conversion: a huge float would otherwise wrap
// int64(math.Round(...)) to INT64_MIN and silently bypass the cap (the check
// `int64(...) > maxAdminGiftCardTransactionPence` would be false for a negative
// wrap). Returns ok=false with the HTTP error already written on rejection.
func giftCardAmountPence(w http.ResponseWriter, amount float64) (int64, bool) {
if math.IsNaN(amount) || math.IsInf(amount, 0) {
http.Error(w, "Amount must be a finite number", http.StatusBadRequest)
return 0, false
}
if amount > maxAdminGiftCardTransactionPence/100.0 {
http.Error(w, "Amount exceeds the maximum of £250", http.StatusBadRequest)
return 0, false
}
return int64(math.Round(amount * 100)), true
}
// --- Admin Handlers ---
// giftCardDailyCapLockKey is the session advisory-lock key that serializes one
@@ -140,6 +150,48 @@ type RedeemGiftCardRequest struct {
// new value are atomic (M7). Keyed per admin: distinct admins never contend.
const giftCardDailyCapLockKey = "crussell:giftcard-daily-cap:"
// giftCardUserCapLockKey is the session advisory-lock key that serializes one
// USER's online gift-card purchases (BuyGiftCard) so the £500/day cap check
// (userGiftCardSpentToday) and the purchase that records the new spend are
// atomic. The idempotency-key lock (crussell:giftcard:<key>) only serializes
// SAME-key retries; two concurrent DISTINCT purchases would otherwise both
// read spentToday=0 before either commits and both pass the cap. Keyed per
// user: distinct users never contend.
const giftCardUserCapLockKey = "crussell:giftcard-user-cap:"
// acquireUserGiftCardCapLock acquires the per-user gift-card daily-cap lock
// (a bounded try-lock on a pinned pool connection, mirroring
// acquireGiftCardDailyCapLock). The cap read (userGiftCardSpentToday) and the
// purchase that records the new spend must run under the same lock, or two
// concurrent distinct purchases could both read the day's spend before either
// commits and both pass the cap — overshooting the £500/day ceiling. Returns
// the pinned connection once the lock is held (the caller must defer
// capConn.Release() and releasePaymentLock(capConn, key)) or nil after writing
// the error response.
func acquireUserGiftCardCapLock(ctx context.Context, w http.ResponseWriter, userID string) (*pgxpool.Conn, bool) {
lockKey := giftCardUserCapLockKey + userID
pinConn, err := db.Conn.Acquire(ctx)
if err != nil {
log.Printf("Failed to acquire connection for gift-card user-cap 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 gift-card user-cap lock %s: %v", lockKey, err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return nil, false
}
if !lockOK {
pinConn.Release()
log.Printf("Gift-card user-cap lock %s not acquired within bound — another gift-card purchase for this user is in progress", lockKey)
http.Error(w, "Another gift-card purchase is already in progress — please try again in a moment", http.StatusConflict)
return nil, false
}
return pinConn, true
}
// acquireGiftCardDailyCapLock acquires the per-admin gift-card daily-cap lock
// (a bounded try-lock on a pinned pool connection, mirroring the payment
// handlers' serialization pattern). The daily cap check
@@ -474,6 +526,13 @@ func GetGiftCards(w http.ResponseWriter, r *http.Request) {
func CreateGiftCard(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
// Defense-in-depth admin check (S-1) — creating a funded gift card moves
// money, so it must stay admin-only even if the route is ever re-registered
// on a router without mw.RequireAdmin.
if !isAdminRequest(r) {
http.Error(w, "Admin access required", http.StatusForbidden)
return
}
adminID, _ := ctx.Value(mw.UserIDKey).(string)
var req CreateGiftCardRequest
@@ -494,9 +553,11 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) {
// C2: cap the admin-funded amount at £250 (25,000 pence) per transaction
// (owner decision — tighter than the £10,000 ceiling ValidateAmount
// enforces on other payment entry points, and matching the till's cap).
// An inventory card may still be created at £0.
if int64(math.Round(req.Amount*100)) > maxAdminGiftCardTransactionPence {
http.Error(w, "Amount exceeds the maximum of £250", http.StatusBadRequest)
// An inventory card may still be created at £0. The pence conversion is
// bounded against non-finite/oversized floats BEFORE the int64 conversion
// (a huge float would wrap to INT64_MIN and bypass the cap).
amountPence, amountOK := giftCardAmountPence(w, req.Amount)
if !amountOK {
return
}
@@ -517,7 +578,7 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) {
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if int64(math.Round(adminValueToday*100))+int64(math.Round(req.Amount*100)) > maxAdminGiftCardDailyPence {
if int64(math.Round(adminValueToday*100))+amountPence > maxAdminGiftCardDailyPence {
http.Error(w, "You have reached your £5,000 daily gift-card value limit", http.StatusBadRequest)
return
}
@@ -620,6 +681,13 @@ func CreateGiftCard(w http.ResponseWriter, r *http.Request) {
func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
// Defense-in-depth admin check (S-1) — topping up a gift card moves money,
// so it must stay admin-only even if the route is ever re-registered on a
// router without mw.RequireAdmin.
if !isAdminRequest(r) {
http.Error(w, "Admin access required", http.StatusForbidden)
return
}
adminID, _ := ctx.Value(mw.UserIDKey).(string)
cardID := chi.URLParam(r, "id")
if cardID == "" || !validators.IsValidID(cardID) {
@@ -640,9 +708,11 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
// C2: cap the top-up at £250 (25,000 pence) per transaction (owner
// decision — tighter than the £10,000 ceiling ValidateAmount enforces on
// other payment entry points, and matching the till's cap).
if int64(math.Round(req.Amount*100)) > maxAdminGiftCardTransactionPence {
http.Error(w, "Amount exceeds the maximum of £250", http.StatusBadRequest)
// other payment entry points, and matching the till's cap). The pence
// conversion is bounded against non-finite/oversized floats BEFORE the
// int64 conversion (a huge float would wrap to INT64_MIN and bypass the cap).
amountPence, amountOK := giftCardAmountPence(w, req.Amount)
if !amountOK {
return
}
@@ -663,7 +733,7 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if int64(math.Round(adminValueToday*100))+int64(math.Round(req.Amount*100)) > maxAdminGiftCardDailyPence {
if int64(math.Round(adminValueToday*100))+amountPence > maxAdminGiftCardDailyPence {
http.Error(w, "You have reached your £5,000 daily gift-card value limit", http.StatusBadRequest)
return
}
@@ -695,7 +765,8 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
var redeemedBy sql.NullString
var isInventory bool
var currentTotalFunds float64
err = tx.QueryRow(ctx, "SELECT redeemed_by, is_inventory, total_funds_added FROM gift_cards WHERE id = $1 FOR UPDATE", cardID).Scan(&redeemedBy, &isInventory, &currentTotalFunds)
var expiryDate sql.NullTime
err = tx.QueryRow(ctx, "SELECT redeemed_by, is_inventory, total_funds_added, expiry_date FROM gift_cards WHERE id = $1 FOR UPDATE", cardID).Scan(&redeemedBy, &isInventory, &currentTotalFunds, &expiryDate)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
http.Error(w, "Gift card not found", http.StatusNotFound)
@@ -711,6 +782,23 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
return
}
// money-F4: an expired gift card must never be topped up — the UPDATE
// below resets expiry_date to NOW()+months and would resurrect a card
// whose remaining value the nightly cleanup job already forfeited.
// Mirrors the till path's expiry gate (till.go:812-825) and
// RedeemGiftCard's (same DB-clock comparison, same 400); a NULL
// expiry_date (legacy) is treated as unexpired.
expired, err := giftCardExpired(ctx, tx, expiryDate)
if err != nil {
log.Printf("Failed to check gift card expiry: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if expired {
http.Error(w, "Gift card has expired", http.StatusBadRequest)
return
}
expiryMonths, err := GetGiftCardExpiryMonths(ctx, tx)
if err != nil {
log.Printf("Failed to query gift card expiry months (using default %d): %v", defaultGiftCardExpiryMonths, err)
@@ -782,6 +870,13 @@ func TopUpGiftCard(w http.ResponseWriter, r *http.Request) {
func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
// Defense-in-depth admin check (S-1) — transferring gift-card value moves
// money, so it must stay admin-only even if the route is ever re-registered
// on a router without mw.RequireAdmin.
if !isAdminRequest(r) {
http.Error(w, "Admin access required", http.StatusForbidden)
return
}
adminID, _ := ctx.Value(mw.UserIDKey).(string)
fromCardID := chi.URLParam(r, "from")
if fromCardID == "" || !validators.IsValidID(fromCardID) {
@@ -813,9 +908,11 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
// C2: cap the transfer at £250 (25,000 pence) per transaction (owner
// decision — tighter than the £10,000 ceiling ValidateAmount enforces on
// other payment entry points, and matching the till's cap).
if int64(math.Round(req.Amount*100)) > maxAdminGiftCardTransactionPence {
http.Error(w, "Amount exceeds the maximum of £250", http.StatusBadRequest)
// other payment entry points, and matching the till's cap). The pence
// conversion is bounded against non-finite/oversized floats BEFORE the
// int64 conversion (a huge float would wrap to INT64_MIN and bypass the cap).
amountPence, amountOK := giftCardAmountPence(w, req.Amount)
if !amountOK {
return
}
@@ -836,7 +933,7 @@ func TransferGiftCard(w http.ResponseWriter, r *http.Request) {
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if int64(math.Round(adminValueToday*100))+int64(math.Round(req.Amount*100)) > maxAdminGiftCardDailyPence {
if int64(math.Round(adminValueToday*100))+amountPence > maxAdminGiftCardDailyPence {
http.Error(w, "You have reached your £5,000 daily gift-card value limit", http.StatusBadRequest)
return
}
@@ -1427,7 +1524,7 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
return
}
if err := ValidateCardInfo(req.CardID, req.NewCardToken); err != nil {
if err := ValidateCardInfo(req.CardID, nil, req.NewCardToken); err != nil {
log.Printf("Failed to process request: %v", err)
http.Error(w, "Invalid request", http.StatusBadRequest)
return
@@ -1565,10 +1662,30 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
}
}
// Daily limit (owner decision): a user may buy at most £500 of online
// daily limit (owner decision): a user may buy at most £500 of online
// gift cards per UTC day. Sums only COMPLETED online purchases (the
// gift_card_transactions rows BuyGiftCard writes) so a pending-retry of a
// failed Square attempt is never blocked by its own un-issued value.
//
// The cap read and the purchase that records the new spend are serialized
// per user (acquireUserGiftCardCapLock — a bounded try-lock on a pinned
// pool connection, mirroring the admin per-admin cap lock): the
// idempotency-key lock above only serializes SAME-key retries, so two
// concurrent DISTINCT purchases could otherwise both read spentToday=0
// before either commits and both pass the cap. The lock is held (via the
// defers) through the purchase's issue transaction, so the read-modify-write
// cycle is atomic per user. A pending-reuse retry still passes through here
// (only a COMPLETED purchase short-circuits above): its first attempt's
// spend was never recorded (the gift_card_transactions row is written in
// the issue transaction only after Square succeeds), so the re-read is
// correct.
userCapPinConn, userCapLockOK := acquireUserGiftCardCapLock(ctx, w, userID)
if !userCapLockOK {
return
}
defer userCapPinConn.Release()
defer releasePaymentLock(userCapPinConn, giftCardUserCapLockKey+userID)
spentToday, err := userGiftCardSpentToday(ctx, db.Conn, userID)
if err != nil {
log.Printf("Failed to query user gift-card spend today for %s: %v", userID, err)
@@ -1584,33 +1701,16 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
// feature is enforced — both paying with an existing saved card (CardID)
// and SAVING a new card during this purchase (SaveCard), mirroring
// CreateBookingPayment/CreateTipPayment. A one-off new-card (nonce) charge
// that is not saved is not gated.
// consume=!reuse (LOW 6a): a FRESH purchase verifies WITH consumption —
// the code is single-use at the gate, closing the TOCTOU where a
// verified-but-unconsumed code could authorize a second charge within its
// lifetime — and a failed Square charge re-issues a fresh code
// (reissueTwoFACodeAfterFailedCharge below). A pending-reuse retry
// (reusePendingID != "") verifies WITHOUT consuming: the code was re-issued
// for exactly this retry and the completed-charge transaction below
// (ConsumePendingCode) burns it on terminal success, so a retry that fails
// again keeps its code for one more attempt. A charge carrying a Square
// verification_token (SCA performed) skips the gate; a token-less charge
// falls back to 2FA and twoFAFallbackUsed is set for the charge-success
// audit.
// that is not saved is not gated. A charge carrying a Square
// verification_token (SCA performed) skips the gate; a token-less charge is
// refused 402 verification_required (SCA-only — the homegrown 2FA fallback
// was removed).
giftCardVerificationToken := ""
if req.VerificationToken != nil {
giftCardVerificationToken = *req.VerificationToken
}
twoFAFallbackUsed := false
if (req.CardID != nil && *req.CardID != "") || req.SaveCard {
var gateOK bool
gateOK, twoFAFallbackUsed = requireTwoFactorForCardAccess(w, r, paymentService, userID, req.VerificationCode, giftCardVerificationToken, reusePendingID == "")
if !gateOK {
return
}
// C6: a fallback-authorized purchase must carry the customer's accepted
// consent (403 consent_required otherwise).
if !enforceSCAFallbackConsent(w, req.ConsentVersion, req.ConsentAccepted, twoFAFallbackUsed) {
if gateOK, _ := requireTwoFactorForCardAccess(w, r, paymentService, userID, giftCardVerificationToken, reusePendingID == ""); !gateOK {
return
}
}
@@ -1804,13 +1904,6 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
paymentResult, err := SquareClient.CreatePayment(ctx, paymentReq)
if err != nil {
log.Printf("Failed to process gift card purchase payment: %v", err)
// The gate consumed the 2FA code for a FRESH saved-card purchase —
// re-issue so the same-key retry has a live code to verify. A
// pending-reuse retry verified without consuming at the gate, so its
// code survives for one more attempt.
if (req.CardID != nil && *req.CardID != "") || req.SaveCard {
reissueTwoFACodeAfterFailedCharge(ctx, db.Conn, userID, true, twoFAFallbackUsed && reusePendingID == "", r)
}
// Payment record intentionally left as 'pending' for manual retry.
// SCA-required failures must surface the structured verification_required
// body so the frontend triggers the 3DS challenge, not a plain decline.
@@ -1981,14 +2074,6 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
return
}
// The 2FA BACKUP authorized this token-less saved-card gift-card purchase
// (SCA was unavailable) — record the strict fallback audit row AFTER the
// money transaction commits (a failed audit write must never roll back a
// completed charge). The actor is the customer's own userID.
if twoFAFallbackUsed {
insertTwoFAFallbackAudit(ctx, userID, userID, paymentResult.CardLast4, buyPaymentID, "gift-card purchase authorized via 2FA fallback (SCA unavailable)", consentVersionValue(req.ConsentVersion), req.ConsentAccepted)
}
w.WriteHeader(http.StatusCreated)
if err := json.NewEncoder(w).Encode(map[string]any{
"status": "success",
@@ -2155,6 +2240,13 @@ func ClaimExpiredBalance(w http.ResponseWriter, r *http.Request) {
http.Error(w, "Unauthorized", http.StatusUnauthorized)
return
}
// Defense-in-depth admin check (S-1) — claiming expired gift-card balance
// moves money, so it must stay admin-only even if the route is ever
// re-registered on a router without mw.RequireAdmin.
if !isAdminRequest(r) {
http.Error(w, "Admin access required", http.StatusForbidden)
return
}
var req ClaimExpiredBalanceRequest
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
backend/handlers/payments/giftcards_test.go
+112
View File
@@ -10,6 +10,7 @@ import (
"math"
"net/http"
"net/http/httptest"
"strings"
"sync"
"testing"
"time"
@@ -2812,3 +2813,114 @@ func TestRedeemGiftCard_ExpiryEdge_DBClock(t *testing.T) {
t.Errorf("expected 200 for a card expiring just after the DB clock, got %d. body: %s", w.Code, w.Body.String())
}
}
// Defense-in-depth (S-1): the money-moving gift-card admin handlers carry an
// in-handler isAdminRequest backstop, so a non-admin request must be refused
// with 403 even when the route is mounted on a router WITHOUT mw.RequireAdmin.
// Each router below applies ONLY mw.RequireAuth — the weaker-router scenario
// the backstop guards against.
func TestCreateGiftCard_NonAdminRefused(t *testing.T) {
t.Parallel()
_, tx := testutils.SetupTestTx(t)
token := jwt.GenerateVerifiedUserToken("nonadminuser001")
reqBody, _ := json.Marshal(map[string]interface{}{"amount": 50.00})
req := httptest.NewRequest("POST", "/api/admin/gift-cards", bytes.NewBuffer(reqBody))
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Post("/api/admin/gift-cards", CreateGiftCard)
r.ServeHTTP(w, req)
if w.Code != http.StatusForbidden {
t.Errorf("expected 403 for non-admin CreateGiftCard, got %d. body: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "Admin access required") {
t.Errorf("expected 'Admin access required' body, got: %s", w.Body.String())
}
}
func TestTopUpGiftCard_NonAdminRefused(t *testing.T) {
t.Parallel()
_, tx := testutils.SetupTestTx(t)
token := jwt.GenerateVerifiedUserToken("nonadminuser001")
reqBody, _ := json.Marshal(map[string]interface{}{
"amount": 25.00,
"payment_method": "on_the_house",
})
req := httptest.NewRequest("PUT", "/api/admin/gift-cards/aabbccddeeff/topup", bytes.NewBuffer(reqBody))
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Put("/api/admin/gift-cards/{id}/topup", TopUpGiftCard)
r.ServeHTTP(w, req)
if w.Code != http.StatusForbidden {
t.Errorf("expected 403 for non-admin TopUpGiftCard, got %d. body: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "Admin access required") {
t.Errorf("expected 'Admin access required' body, got: %s", w.Body.String())
}
}
func TestTransferGiftCard_NonAdminRefused(t *testing.T) {
t.Parallel()
_, tx := testutils.SetupTestTx(t)
token := jwt.GenerateVerifiedUserToken("nonadminuser001")
reqBody, _ := json.Marshal(map[string]interface{}{
"to_card_id": "112233445566",
"amount": 30.00,
})
req := httptest.NewRequest("POST", "/api/admin/gift-cards/aabbccddeeff/transfer", bytes.NewBuffer(reqBody))
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Post("/api/admin/gift-cards/{from}/transfer", TransferGiftCard)
r.ServeHTTP(w, req)
if w.Code != http.StatusForbidden {
t.Errorf("expected 403 for non-admin TransferGiftCard, got %d. body: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "Admin access required") {
t.Errorf("expected 'Admin access required' body, got: %s", w.Body.String())
}
}
func TestClaimExpiredBalance_NonAdminRefused(t *testing.T) {
t.Parallel()
_, tx := testutils.SetupTestTx(t)
token := jwt.GenerateVerifiedUserToken("nonadminuser001")
reqBody, _ := json.Marshal(map[string]interface{}{"balance_id": "aabbccddeeff"})
req := httptest.NewRequest("POST", "/api/admin/gift-cards/expired-balances/claim", bytes.NewBuffer(reqBody))
req.Header.Set("Authorization", "Bearer "+token)
req.Header.Set("Content-Type", "application/json")
req = req.WithContext(db.ContextWithTx(req.Context(), tx.(pgx.Tx)))
w := httptest.NewRecorder()
r := chi.NewRouter()
r.Use(mw.RequireAuth)
r.Post("/api/admin/gift-cards/expired-balances/claim", ClaimExpiredBalance)
r.ServeHTTP(w, req)
if w.Code != http.StatusForbidden {
t.Errorf("expected 403 for non-admin ClaimExpiredBalance, got %d. body: %s", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "Admin access required") {
t.Errorf("expected 'Admin access required' body, got: %s", w.Body.String())
}
}
backend/handlers/payments/handlers.go
+167 -266
View File
@@ -68,38 +68,6 @@ func InsertAdminAuditCharge(ctx context.Context, adminID, targetUserID, action s
}
}
// insertTwoFAFallbackAudit records that a saved-card charge was authorized by
// the homegrown 2FA BACKUP because SCA was unavailable (the charge carried no
// Square verification_token). It is the strict-audit half of the SCA-primary /
// 2FA-backup decision model: every 2FA-fallback authorization of a saved-card
// charge must land an admin_audit_log row (action_type '2fa_fallback_charge',
// details {sca_performed:false, fallback_reason:"verification_unavailable"}) so
// the operator can distinguish SCA-authorized charges from fallback-authorized
// ones. Since C6 the row also captures the customer's versioned consent to the
// fallback (consent_version, consent_accepted, consent_versioned_at) — the
// exact values the server validated before the charge — so an operator/GDPR
// export can reconstruct which dialog version was shown, that it was accepted,
// when, and on which charge. Mirrors InsertAdminAuditCharge's best-effort,
// own-transaction, non-fatal failure handling (a failed audit write can never
// abort a completed charge). For customer-initiated online charges the actor
// (adminID) is the customer's own userID; for admin surfaces it is the admin
// from request context — the caller passes accordingly. cardLast4 and
// referenceID are filled by the caller at charge success
// (paymentResult.CardLast4, booking/till/payment id), where they are actually
// known.
func insertTwoFAFallbackAudit(ctx context.Context, adminID, userID, cardLast4, referenceID, notes string, consentVersion string, consentAccepted bool) {
InsertAdminAuditCharge(ctx, adminID, userID, "2fa_fallback_charge", map[string]any{
"sca_performed": false,
"fallback_reason": "verification_unavailable",
"card_last4": cardLast4,
"reference_id": referenceID,
"notes": notes,
"consent_version": consentVersion,
"consent_accepted": consentAccepted,
"consent_versioned_at": clock.Now().Format(time.RFC3339),
})
}
// logVerificationTokenProvenance records the charge context a legacy SCA
// verification_token arrived with (saved-card reference + booking) so an
// operator can correlate a minted token with the exact charge it authorized.
@@ -134,10 +102,12 @@ type CreateTerminalPaymentRequest struct {
// directly, bypassing the terminal. The frontend sends this for the admin
// "Charge Saved Card" action.
UserSavedCardID *string `json:"saved_card_id,omitempty"`
// verification_code: the customer's current 2FA one-time code (B10). An
// enforced environment charges a saved card only when this matches the
// customer's pending code; the operator relays it from the [2FA] log/email.
VerificationCode string `json:"verification_code,omitempty"`
// new_card_token: the SCA tokenize-result token (card.tokenize(
// verificationDetails, cardId)) for a saved-card charge. When present it
// coexists with saved_card_id — the token is the one-time charge SOURCE and
// the saved-card row supplies the customer (resolveChargeSource). Without a
// token the stored ccof: card id is the source (legacy saved-card charge).
NewCardToken *string `json:"new_card_token,omitempty"`
// verification_token: Square 3DS/SCA verification token returned by the
// frontend's buyer-verification flow (tokenizeWithVerification). When a
// saved-card charge carries one, SCA has been performed by the issuer and
@@ -153,14 +123,6 @@ type CreateTerminalPaymentRequest struct {
// amount+card key for no-client-key retry safety. Cap ≤45 (Square's
// idempotency-key limit for /v2/payments — this key feeds CreatePayment).
IdempotencyKey string `json:"idempotency_key,omitempty" validate:"omitempty,max=45"`
// ConsentVersion / ConsentAccepted: the customer's explicit versioned
// consent to the SCA-unavailable → 2FA fallback (C6). The frontend sends
// consent_version:"v1" + consent_accepted:true (scaFallbackConsentFields)
// when the ScaFallbackConsentDialog was accepted; the server enforces it
// (403 consent_required) before a fallback charge can reach Square and
// records it on the 2fa_fallback_charge audit row.
ConsentVersion *string `json:"consent_version,omitempty"`
ConsentAccepted bool `json:"consent_accepted"`
}
type CreateBookingPaymentRequest struct {
@@ -178,15 +140,6 @@ type CreateBookingPaymentRequest struct {
SaveCard bool `json:"save_card"`
IdempotencyKey string `json:"idempotency_key,omitempty" validate:"omitempty,max=45"`
VerificationToken *string `json:"verification_token,omitempty"`
// VerificationCode is the customer's current 2FA one-time code (B10): an
// enforced environment charges a saved card only when this matches the
// customer's pending code.
VerificationCode string `json:"verification_code,omitempty"`
// ConsentVersion / ConsentAccepted: the customer's explicit versioned
// consent to the SCA-unavailable → 2FA fallback (C6), enforced server-side
// (403 consent_required) and recorded on the 2fa_fallback_charge audit row.
ConsentVersion *string `json:"consent_version,omitempty"`
ConsentAccepted bool `json:"consent_accepted"`
// ConfirmOverflowTip acknowledges that an overpayment beyond the booking's
// remaining balance will be recorded as a tip (M7). Tips cannot be paid in
// advance, so a pre-start overpayment is rejected with 400
@@ -214,15 +167,6 @@ type CreateTipPaymentRequest struct {
SaveCard bool `json:"save_card"`
IdempotencyKey string `json:"idempotency_key,omitempty" validate:"omitempty,max=45"`
VerificationToken *string `json:"verification_token,omitempty"`
// VerificationCode is the customer's current 2FA one-time code (B10): an
// enforced environment charges a saved card only when this matches the
// customer's pending code.
VerificationCode string `json:"verification_code,omitempty"`
// ConsentVersion / ConsentAccepted: the customer's explicit versioned
// consent to the SCA-unavailable → 2FA fallback (C6), enforced server-side
// (403 consent_required) and recorded on the 2fa_fallback_charge audit row.
ConsentVersion *string `json:"consent_version,omitempty"`
ConsentAccepted bool `json:"consent_accepted"`
}
type CheckoutResponse struct {
@@ -418,6 +362,26 @@ func calculateDiscountPreview(ctx context.Context, bookingID string, userID stri
// remaining + this bound.
const maxTerminalTipPence = int64(5000) // £50
// maxOnlineTipPence caps a single ONLINE tip (CreateTipPayment, the dedicated
// POST /api/bookings/{id}/tip endpoint). The terminal checkout (B3) caps its
// embedded gratuity at maxTerminalTipPence (£50) because the admin keys the tip
// in alongside the booking portion; the online path is customer-initiated and
// is deliberately more generous. £250 is the bound because:
// - tips are gratuity on a percentage of the service — the frontend presets
// are 10/15/20% of the booking subtotal (frontend/src/lib/constants/policy.ts
// TIP_PRESET_PCTS), so £250 is ~5x the 20% preset on even the salon's most
// expensive service;
// - it matches the £250 per-transaction ceiling the business already uses for
// gift-card creates/topups (maxAdminGiftCardTransactionPence, giftcard_limits.go),
// an owner-signed figure this codebase already treats as the generous
// single-transaction bound;
// - it sits far above the £50 till cap while staying well below
// ValidateAmount's generic £10,000 ceiling — without it a customer could tip
// £9,999 online when the till is capped at £50 (money/UX inconsistency).
// - 25,000 pence is the effective online ceiling: ValidateAmount passes first,
// and this stricter bound makes the generic £10,000 cap unreachable here.
const maxOnlineTipPence = int64(25000) // £250
// clampTerminalChargeToRemainingBalance caps a requested terminal charge at the
// booking's remaining obligation (B3). The admin "Take Payment" PaymentModal
// sends subtotal - discounts - campaignDiscountPence, which ignores PRIOR
@@ -969,8 +933,12 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
// Resolve the saved-card Square source for the booking's user (the
// card's owner, not the admin) — shared new-card-vs-saved-card
// resolution, see resolveChargeSource for the R6 rationale.
sourceID, _, savedCardCustomerID, sourceOK := resolveChargeSource(r.Context(), w, service, bookingUserID.String, nil, req.UserSavedCardID, false, "Saved card not found")
// resolution, see resolveChargeSource for the R6 rationale. When the
// request carries an SCA tokenize-result token (new_card_token), it is
// used as the one-time charge source and the saved-card row supplies
// the customer — mirroring the booking path (handlers.go:2050-2051,
// 2626).
sourceID, _, savedCardCustomerID, sourceOK := resolveChargeSource(r.Context(), w, service, bookingUserID.String, req.NewCardToken, req.UserSavedCardID, false, "Saved card not found")
if !sourceOK {
return
}
@@ -1140,17 +1108,14 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
// retry verifies WITHOUT consuming, so a retry that fails again keeps
// its code for one more attempt (the completed-charge transaction
// consumes it on terminal success).
twoFAFallbackUsed := false
if bookingUserID.Valid {
var gateOK bool
gateOK, twoFAFallbackUsed = requireTwoFactorForCardAccess(w, r, service, bookingUserID.String, req.VerificationCode, terminalVerificationToken, !reusePendingRecord)
if !gateOK {
return
}
// C6: a fallback-authorized charge must carry the customer's
// accepted consent (403 consent_required otherwise) — the server-side
// guard that stops a 2FA-path charge reaching Square without it.
if !enforceSCAFallbackConsent(w, req.ConsentVersion, req.ConsentAccepted, twoFAFallbackUsed) {
// scaTokenizedSavedCard (an SCA tokenize-result token charging a saved
// card) skips the 2FA gate exactly like a present verification_token:
// the token only exists after the issuer completed buyer verification for
// this card + amount (SCA-primary), so no homegrown fallback
// authorization is needed.
scaTokenizedSavedCard := req.NewCardToken != nil && *req.NewCardToken != ""
if bookingUserID.Valid && !scaTokenizedSavedCard {
if gateOK, _ := requireTwoFactorForCardAccess(w, r, service, bookingUserID.String, terminalVerificationToken, !reusePendingRecord); !gateOK {
return
}
}
@@ -1261,16 +1226,6 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
paymentResult, err := SquareClient.CreatePayment(r.Context(), paymentReq)
if err != nil {
log.Printf("Failed to process saved-card payment: %v (error_code=%q)", err, square.ErrorCode(err))
// The gate consumed the 2FA code for a fresh saved-card charge —
// re-issue so the same-key retry has a live code to verify
// (mirrors CreateBookingPayment's post-failure re-issue, finding
// 4). Only runs for a FRESH charge whose gate consumed a code
// (!reusePendingRecord): a pending-reuse retry verified WITHOUT
// consuming, so its code is still live and re-issuing would
// silently invalidate the one the customer holds.
if bookingUserID.Valid {
reissueTwoFACodeAfterFailedCharge(r.Context(), db.Conn, bookingUserID.String, true, twoFAFallbackUsed && !reusePendingRecord, r)
}
// SCA-required failures (Square demands buyer verification) must
// surface the structured verification_required body so the frontend
// triggers the 3DS challenge instead of treating the payment as a
@@ -1392,11 +1347,6 @@ func CreateTerminalPayment(w http.ResponseWriter, r *http.Request) {
"card_last4": paymentResult.CardLast4,
"square_payment_id": paymentResult.SquarePayID,
})
// The 2FA BACKUP authorized this token-less saved-card charge
// (SCA was unavailable) — record the strict fallback audit row.
if twoFAFallbackUsed {
insertTwoFAFallbackAudit(r.Context(), adminID, bookingUserID.String, paymentResult.CardLast4, bookingID, "admin saved-card charge authorized via 2FA fallback (SCA unavailable)", consentVersionValue(req.ConsentVersion), req.ConsentAccepted)
}
}
// F6: a fully-paid saved-card charge completes the booking exactly like
@@ -1985,7 +1935,7 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
return
}
if err := ValidateCardInfo(req.CardID, req.NewCardToken); err != nil {
if err := ValidateCardInfo(req.CardID, req.UserSavedCardID, req.NewCardToken); err != nil {
log.Printf("Failed to process request: %v", err)
http.Error(w, "Invalid request", http.StatusBadRequest)
return
@@ -2041,8 +1991,9 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
// same user_saved_cards.id; card_id wins when both are sent). When a
// NEW-card token arrives alongside it (SCA tokenize-result wire contract),
// the token is the one-time charge source and this row supplies the
// customer. ValidateCardInfo above already rejected card_id + new_card_token
// together, so a coexistence can only be new_card_token + saved_card_id.
// customer. ValidateCardInfo above already validated the three legal card
// source shapes (saved-card ref alone, token alone, or ref + token as the
// SCA tokenize-result source).
savedCardRef := req.CardID
if savedCardRef == nil || *savedCardRef == "" {
savedCardRef = req.UserSavedCardID
@@ -2280,21 +2231,17 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
// 2FA gating (C5): persisting a card requires 2FA when the feature is
// enforced. This runs AFTER the idempotency dedup's completed
// short-circuit (Loop B MEDIUM): a same-key lost-response retry returns the
// already-completed payment above without re-entering the gate, so its
// single-use code (already consumed by the original attempt) is never
// re-rejected as "expired". Pending-reuse and fresh paths still gate — a
// new charge may move at Square. The gate also runs before
// resolveChargeSource below, so an un-2FA'd request never persists a card.
// already-completed payment above without re-entering the gate. Pending-reuse
// and fresh paths still gate — a new charge may move at Square. The gate
// also runs before resolveChargeSource below, so an un-2FA'd request never
// persists a card.
// SCA-primary (auth-F1): the SAVE surface never forwards the client's
// verification_token to Square (the card is persisted via CreateCardOnFile,
// which takes no token), so a non-empty token is client-asserted and must
// NOT skip the gate — a token-less save falls back to the customer's 2FA
// code, and twoFAFallbackUsed records that the 2FA BACKUP authorized the
// operation (the caller audits it). Only the call-site's
// scaTokenizedSavedCard tokenize-result flow skips the save gate (the
// combined path never persists a card and Square validates the token as the
// source_id).
twoFAFallbackUsed := false
// NOT skip the gate — a token-less save is refused 402 (SCA-only). Only the
// call-site's scaTokenizedSavedCard tokenize-result flow skips the save gate
// (the combined path never persists a card and Square validates the token as
// the source_id).
bookingVerificationToken := ""
if req.VerificationToken != nil {
bookingVerificationToken = *req.VerificationToken
@@ -2308,14 +2255,7 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
// a card (resolveChargeSource uses the token as a one-time source, no
// card-on-file is created).
if req.SaveCard && !scaTokenizedSavedCard {
var gateOK bool
gateOK, twoFAFallbackUsed = requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, req.VerificationCode, bookingVerificationToken, true, false)
if !gateOK {
return
}
// C6: a fallback-authorized save/charge must carry the customer's
// accepted consent (403 consent_required otherwise).
if !enforceSCAFallbackConsent(w, req.ConsentVersion, req.ConsentAccepted, twoFAFallbackUsed) {
if gateOK, _ := requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, bookingVerificationToken, true, false); !gateOK {
return
}
}
@@ -2488,6 +2428,25 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
})
return
}
// maxOnlineTipPence cap (the £250 bound the dedicated tip endpoint
// enforces): the portion buildSplitRecords will actually carve as a
// payment_type='tip' row is the excess beyond the booking's REAL
// remaining obligation (TotalAmount - TotalPaid, which excludes
// discount rows) — the post-start carve and the pre-start
// deposit/balance carve both compute tipPortion =
// paymentAmount - realRemaining. The deposit-with-discount
// overflowThreshold compared above is only the CONFIRMATION
// trigger (a raw deposit between the discounted obligation and the
// real remaining needs the flag but carves no tip); capping the
// real tip portion keeps every minted tip row within
// maxOnlineTipPence. Without this, a confirmed £10,000 payment on
// a booking with £50 remaining would mint a £9,950 tip row — far
// over the cap and unreachable by the tip-refund path.
if tipPortion := req.Amount - remainingPence; tipPortion > maxOnlineTipPence {
log.Printf("Overflow tip rejected: requested %d exceeds obligation %d for booking %s — the %d pence tip portion exceeds the £250 online tip cap (discount credit %d pence)", req.Amount, overflowThreshold, bookingID, tipPortion, eligibleDiscountPence)
http.Error(w, "Tip exceeds the maximum allowed amount (£250)", http.StatusBadRequest)
return
}
chargeAmount = req.Amount
log.Printf("Overflow accepted as tip: requested %d exceeds obligation %d for booking %s (discount credit %d pence, confirmed=%v)", req.Amount, overflowThreshold, bookingID, eligibleDiscountPence, req.ConfirmOverflowTip)
}
@@ -2593,26 +2552,12 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
var savedCardID *string
var savedCardCustomerID string
// 2FA gating (C5): charging a SAVED card requires 2FA when the feature is
// enforced. New-card (nonce) charges are not gated. consume=!reusePendingRecord
// (LOW 6a): a FRESH charge verifies WITH consumption — the code is single-use
// at the gate, closing the TOCTOU where a verified-but-unconsumed code could
// authorize a second charge within its lifetime — and a failed Square charge
// re-issues a fresh code (reissueTwoFACodeAfterFailedCharge). A pending-reuse
// retry verifies WITHOUT consuming: the code was re-issued for exactly this
// retry and the completed-charge transaction consumes it on terminal success,
// so a retry that fails again keeps its code for one more attempt. A charge
// carrying a Square verification_token (SCA performed) skips the gate; a
// token-less charge falls back to 2FA and twoFAFallbackUsed is set for the
// charge-success audit.
// enforced. New-card (nonce) charges are not gated. A charge carrying a
// Square verification_token (SCA performed) skips the gate; a token-less
// charge is refused 402 verification_required (SCA-only — the homegrown 2FA
// fallback was removed).
if savedCardRef != nil && *savedCardRef != "" && !scaTokenizedSavedCard {
var gateOK bool
gateOK, twoFAFallbackUsed = requireTwoFactorForCardAccess(w, r, service, userID, req.VerificationCode, bookingVerificationToken, !reusePendingRecord)
if !gateOK {
return
}
// C6: a fallback-authorized charge must carry the customer's accepted
// consent (403 consent_required otherwise).
if !enforceSCAFallbackConsent(w, req.ConsentVersion, req.ConsentAccepted, twoFAFallbackUsed) {
if gateOK, _ := requireTwoFactorForCardAccess(w, r, service, userID, bookingVerificationToken, !reusePendingRecord); !gateOK {
return
}
}
@@ -2733,21 +2678,6 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
paymentResult, err := SquareClient.CreatePayment(r.Context(), paymentReq)
if err != nil {
log.Printf("Failed to create payment: %v (error_code=%q)", err, square.ErrorCode(err))
// The gate consumed the 2FA code for a fresh saved-card charge —
// re-issue so the same-key retry has a live code to verify. A NEW-CARD
// (cnon) charge with save_card=false never gated and involves no code:
// re-issuing there would overwrite the customer's standing pending code
// with a fresh undelivered one, silently burning the code the operator
// relayed (finding 5). But a NEW-CARD charge with save_card=true DID
// gate — the SAVE gate above consumed the code before the charge — so
// the code must be re-issued there too or every retry hits "Verification
// code expired" forever (finding: save-gate burned code never re-issued).
// A pending-reuse retry verified WITHOUT consuming, so its code is still
// live and no re-issue runs (a re-issue would invalidate the code the
// customer already holds).
if (savedCardRef != nil && *savedCardRef != "") || req.SaveCard {
reissueTwoFACodeAfterFailedCharge(r.Context(), db.Conn, userID, true, twoFAFallbackUsed && !reusePendingRecord, r)
}
// SCA-required failures must surface the structured verification_required
// body so the frontend triggers the 3DS challenge, not a plain decline.
if isVerificationRequiredError(err) {
@@ -2843,7 +2773,13 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
var records []PaymentRecord
if bErr == nil && bookingInfo != nil {
records = buildSplitRecords(primaryRecord, req.PaymentType, bookingInfo, paymentAmount)
var splitErr error
records, splitErr = buildSplitRecords(primaryRecord, req.PaymentType, bookingInfo, paymentAmount)
if splitErr != nil {
log.Printf("CRITICAL: Square payment %s (ID=%s) was processed but buildSplitRecords rejected the split for booking %s: %v — the pending record %s stays for the stale-pending sweep; manual reconciliation required", paymentResult.Status, paymentResult.SquarePayID, bookingID, splitErr, paymentID)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
} else {
if bErr != nil {
log.Printf("Failed to get booking info for split: %v — using single record", bErr)
@@ -2967,15 +2903,6 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
return
}
// The 2FA BACKUP authorized this token-less saved-card charge (SCA was
// unavailable) — record the strict fallback audit row AFTER the money
// transaction commits (a failed audit write must never roll back a
// completed charge). The actor is the customer's own userID
// (customer-initiated online charge).
if twoFAFallbackUsed {
insertTwoFAFallbackAudit(r.Context(), userID, userID, paymentResult.CardLast4, bookingID, "saved-card charge authorized via 2FA fallback (SCA unavailable)", consentVersionValue(req.ConsentVersion), req.ConsentAccepted)
}
// B13: a campaign was exhausted between the preview and the apply-time
// re-check. The charge already succeeded at Square and the payment record
// is committed, so the customer's promised discount must not silently
@@ -3200,7 +3127,7 @@ func applyEligibleCampaignsAtPayment(ctx context.Context, q db.Querier, bookingI
// over-allocate toward balance and under-allocate toward tip (a bookkeeping
// simplification, not an overcharge) — the partition still equals the charged
// amount. See TestBuildSplitRecords_DiscountBooking_TipOverflow_SumNeverExceedsCharge.
func buildSplitRecords(primary PaymentRecord, reqPaymentType string, info *BookingPaymentInfo, paymentAmount float64) []PaymentRecord {
func buildSplitRecords(primary PaymentRecord, reqPaymentType string, info *BookingPaymentInfo, paymentAmount float64) ([]PaymentRecord, error) {
// After the booking starts there is no deposit protection window, but an
// overpayment beyond the remaining booking value is still gratuity and must
// be carved out as its own payment_type='tip' record (F3) — mirroring
@@ -3215,6 +3142,15 @@ func buildSplitRecords(primary PaymentRecord, reqPaymentType string, info *Booki
bookingPortion = math.Round(bookingPortion*100) / 100
tipPortion := math.Round((paymentAmount-bookingPortion)*100) / 100
if tipPortion > roundingEpsilon {
// Belt-and-braces: the online tip bound (maxOnlineTipPence) applies
// to EVERY tip row — including overflow tips carved from a booking
// charge. The B12 gate enforces it before the charge is taken; this
// check catches any future caller that skips the gate. Returning an
// error (never clamping) preserves the partition invariant: the
// records must always sum to the charged paymentAmount.
if tipPence := int64(math.Round(tipPortion * 100)); tipPence > maxOnlineTipPence {
return nil, fmt.Errorf("buildSplitRecords: post-start carve for booking %s would mint a tip of %d pence, exceeding the £250 online tip cap (charge %.2f)", primary.BookingID, tipPence, paymentAmount)
}
records := []PaymentRecord{primary}
records[0].Amount = bookingPortion
tip := primary
@@ -3225,9 +3161,9 @@ func buildSplitRecords(primary PaymentRecord, reqPaymentType string, info *Booki
k := *primary.IdempotencyKey + "-split-tip"
tip.IdempotencyKey = &k
}
return append(records, tip)
return append(records, tip), nil
}
return []PaymentRecord{primary}
return []PaymentRecord{primary}, nil
}
// 1. Deposit portion: up to 50% of total, minus what's already been paid.
@@ -3247,6 +3183,18 @@ func buildSplitRecords(primary PaymentRecord, reqPaymentType string, info *Booki
// 4. Tip: anything beyond the booking total.
tipPortion := math.Round((remainingAfterDeposit-balancePortion)*100) / 100
// Belt-and-braces (same bound as the post-start carve and the dedicated
// tip endpoint): the pre-start deposit/balance/tip carve can mint a tip
// row when the payment exceeds the booking's REAL remaining (e.g. a
// deposit-with-discount overflow). The B12 gate enforces the cap before
// the charge; this check catches any future caller that skips the gate.
// Never clamp — the records must partition the charged amount exactly.
if tipPortion > roundingEpsilon {
if tipPence := int64(math.Round(tipPortion * 100)); tipPence > maxOnlineTipPence {
return nil, fmt.Errorf("buildSplitRecords: pre-start carve for booking %s would mint a tip of %d pence, exceeding the £250 online tip cap (charge %.2f)", primary.BookingID, tipPence, paymentAmount)
}
}
var records []PaymentRecord
splitIdx := 0
@@ -3308,7 +3256,7 @@ func buildSplitRecords(primary PaymentRecord, reqPaymentType string, info *Booki
primary.Fees = 0
records = append(records, primary)
}
return records
return records, nil
}
// buildTerminalSplitRecords splits a completed terminal checkout charge that
@@ -3462,10 +3410,6 @@ func DeletePaymentMethod(w http.ResponseWriter, r *http.Request) {
type CreatePaymentMethodRequest struct {
CardToken string `json:"card_token" validate:"required"`
// VerificationCode is the customer's current 2FA one-time code (B10): an
// enforced environment persists a card only when this matches the
// customer's pending code.
VerificationCode string `json:"verification_code,omitempty"`
// VerificationToken is a Square 3DS/SCA verification token. On the add-card
// SAVE surface it is CLIENT-ASSERTED and never forwarded to Square
// (CreateCardOnFile takes no verification_token), so the 2FA gate IGNORES it
@@ -3473,8 +3417,9 @@ type CreatePaymentMethodRequest struct {
// carried on the wire for parity with the charge surfaces and passed through
// to the gate, whose save variant applies the token-less SCA-only refusal
// when a genuine SCA proof is absent. The genuine proof for a SAVE is the
// STORE-intent tokenize-result submitted as card_token (see
// isSCATokenizeResultCardToken).
// token itself — any Square token-like card token (cnon:/ccof:) was minted
// by a tokenization flow that ran the STORE-intent SCA, so it is treated as
// SCA-proven and the gate is skipped (see CreatePaymentMethod).
VerificationToken *string `json:"verification_token,omitempty"`
// ConsentVersion / ConsentAccepted mirror the charge structs (C6): the
// add-card endpoint never charges, so they are recorded on the fallback
@@ -3513,30 +3458,27 @@ func CreatePaymentMethod(w http.ResponseWriter, r *http.Request) {
// token itself. The frontend performs the STORE-intent SCA at tokenization
// (SquareCardInput.tokenizeForStore) — the tokenize-result IS the card
// token, and Square validates it as a card-on-file source when
// CreateCardOnFile persists it. A save that carries a genuine SCA
// tokenize-result as card_token is therefore SCA-compliant and skips the
// 2FA gate at the call site — exactly like the charge surfaces'
// scaTokenizedSavedCard skip: the STORE-intent SCA performed at tokenization
// IS the verification (PSR 2017 reg 100 compliant), so no homegrown fallback
// authorization is needed and no fallback audit is written.
// CreateCardOnFile persists it. Any genuine Square token-like card token
// (a cnon: nonce or ccof: card id — the only shapes the mock's
// CreateCardOnFile and real Square accept, plus the dev mock's
// verify_mock_ transition shape) was minted by a tokenization flow, so it is
// treated as SCA-proven and the homegrown gate is skipped; Square's own
// acceptance of the token is the authoritative check. The fictional
// cnon:sca- marker requirement is gone — real Square never produces it, so
// it made every save 402 in an enforced deployment.
//
// Every other save stays gated through the save-surface variant
// (tokenForwardedToSquare=false): a verification_token is client-asserted
// and NEVER forwarded to Square on a SAVE surface, so it must NOT skip the
// gate (auth-F1 — a forged value cannot authorise a save, and no 2FA code
// can either, SCA-only). A token-less legacy save (a raw card.tokenize()
// nonce, or a token the backend cannot recognize as a genuine SCA
// tokenize-result) is refused 402 verification_required in an enforced
// deployment.
// A non-token-like value (a raw PAN or any other shape) fails closed
// through the save-surface variant (tokenForwardedToSquare=false): a
// verification_token is client-asserted and NEVER forwarded to Square on a
// SAVE surface, so it must NOT skip the gate (auth-F1 — a forged value
// cannot authorise a save, and no 2FA code can either, SCA-only). It is
// refused 402 verification_required in an enforced deployment.
saveVerificationToken := ""
if req.VerificationToken != nil {
saveVerificationToken = *req.VerificationToken
}
twoFAFallbackUsed := false
if !isSCATokenizeResultCardToken(req.CardToken) {
var gateOK bool
gateOK, twoFAFallbackUsed = requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, req.VerificationCode, saveVerificationToken, true, false)
if !gateOK {
if !isTokenLikeSaveSource(req.CardToken) {
if gateOK, _ := requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, saveVerificationToken, true, false); !gateOK {
return
}
}
@@ -3553,31 +3495,19 @@ func CreatePaymentMethod(w http.ResponseWriter, r *http.Request) {
return
}
// The 2FA BACKUP authorized persisting this card (no SCA was performed on
// the add-card endpoint); the actor is the customer's own userID. Unreachable
// under SCA-only (the gate never sets fallbackUsed), retained for parity
// with the charge surfaces.
if twoFAFallbackUsed && card != nil {
insertTwoFAFallbackAudit(r.Context(), userID, userID, card.Last4, "", "card persisted via 2FA fallback (SCA unavailable)", consentVersionValue(req.ConsentVersion), req.ConsentAccepted)
}
if err := json.NewEncoder(w).Encode(card); err != nil {
log.Printf("Failed to encode JSON response: %v", err)
}
}
// isSCATokenizeResultCardToken reports whether a card token submitted to the
// add-card save surface is a GENUINE Square tokenizeWithVerification result —
// the STORE-intent tokenize-result the account page's tokenizeForStore
// produces (the SCA challenge runs at tokenization, so the returned token is
// the SCA-verified source the backend stores). Real Square returns opaque cnon:
// tokens, so the dev mock's contract marks a genuine tokenize-result with
// "sca-" immediately after the cnon: prefix (internal/square's
// isSCATokenizeResultSource) — the enforcement-parity stand-in the charge
// paths also rely on (money-F2). A raw card.tokenize() nonce or any other
// shape is NOT an SCA proof and never skips the gate.
func isSCATokenizeResultCardToken(cardToken string) bool {
return strings.HasPrefix(cardToken, "cnon:sca-")
// isTokenLikeSaveSource reports whether a card token submitted to the add-card
// save surface is a genuine Square token-like source — the shapes the mock's
// CreateCardOnFile accepts (cnon: nonces and ccof: card ids, plus the dev
// mock's verify_mock_ transition shape), which real Square only mints after a
// tokenization flow ran the STORE-intent SCA. Anything else (a raw PAN, an
// arbitrary string) is not token-like and fails closed through the 2FA gate.
func isTokenLikeSaveSource(cardToken string) bool {
return strings.HasPrefix(cardToken, "cnon:") || strings.HasPrefix(cardToken, "ccof:") || strings.HasPrefix(cardToken, "verify_mock_")
}
// isDefinitiveCardSaveFailure reports whether a CreatePaymentMethod error is a
@@ -4729,30 +4659,18 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
service := NewPaymentService()
// 2FA gating (C5): persisting a card requires 2FA when the feature is enforced.
// consume=true: saving a card is a terminal operation (the card row is
// created right here), so the verified code is single-use immediately —
// unlike the saved-card CHARGE gate below, which defers consumption to the
// charge's terminal success (MEDIUM-2). SCA-primary (auth-F1): the SAVE
// surface never forwards the client's verification_token to Square (the
// card is persisted via CreateCardOnFile, which takes no token), so a
// non-empty token is client-asserted and must NOT skip the gate — a
// token-less save falls back to 2FA and twoFAFallbackUsed is set for the
// charge-success audit.
twoFAFallbackUsed := false
// 2FA gating (C5): persisting a card requires 2FA when the feature is
// enforced. SCA-primary (auth-F1): the SAVE surface never forwards the
// client's verification_token to Square (the card is persisted via
// CreateCardOnFile, which takes no token), so a non-empty token is
// client-asserted and must NOT skip the gate — a token-less save is refused
// 402 (SCA-only).
tipVerificationToken := ""
if req.VerificationToken != nil {
tipVerificationToken = *req.VerificationToken
}
if req.SaveCard {
var gateOK bool
gateOK, twoFAFallbackUsed = requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, req.VerificationCode, tipVerificationToken, true, false)
if !gateOK {
return
}
// C6: a fallback-authorized save/charge must carry the customer's
// accepted consent (403 consent_required otherwise).
if !enforceSCAFallbackConsent(w, req.ConsentVersion, req.ConsentAccepted, twoFAFallbackUsed) {
if gateOK, _ := requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, tipVerificationToken, true, false); !gateOK {
return
}
}
@@ -4763,7 +4681,20 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
return
}
if err := ValidateCardInfo(req.CardID, req.NewCardToken); err != nil {
// maxOnlineTipPence bound: ValidateAmount's generic £10,000 cap is the
// money-minting ceiling for booking charges, but a tip is gratuity on a
// percentage of the service — a single online tip over £250 is not a
// legitimate business transaction (the till's embedded-gratuity bound is
// only £50). Reject BEFORE any charge-source resolution, idempotency
// handling, or Square call, so no pending record or charge is ever minted
// for an over-bound tip.
if req.Amount > maxOnlineTipPence {
log.Printf("Tip rejected: booking %s requested a tip of %d pence which exceeds the £250 online tip cap", bookingID, req.Amount)
http.Error(w, "Tip exceeds the maximum allowed amount (£250)", http.StatusBadRequest)
return
}
if err := ValidateCardInfo(req.CardID, nil, req.NewCardToken); err != nil {
log.Printf("Failed to process request: %v", err)
http.Error(w, "Invalid request", http.StatusBadRequest)
return
@@ -4858,14 +4789,17 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
// Serialize tip attempts for this booking to prevent concurrent duplicate
// tip payments across browser tabs or retries. Uses a PostgreSQL session-level
// advisory lock scoped to the booking ID.
// advisory lock scoped to the booking ID. Shares the crussell:payment:<id>
// key with the booking-payment handlers so a tip cannot race an in-flight
// payment on the same booking (they don't nest — no handler re-acquires
// this lock while holding it — so sharing the key cannot deadlock).
// Bounded try-lock (R6) so a contended lock never blocks the pool across
// the Square round-trip.
pinConn, lockOK := acquireBookingPaymentLock(r.Context(), w, "crussell:tip:"+bookingID, "Payment in progress, try again")
pinConn, lockOK := acquireBookingPaymentLock(r.Context(), w, "crussell:payment:"+bookingID, "Payment in progress, try again")
if !lockOK {
return
}
defer releaseBookingPaymentLock(pinConn, "crussell:tip:"+bookingID)
defer releaseBookingPaymentLock(pinConn, "crussell:payment:"+bookingID)
// Step 1: Insert payment record in 'pending' state inside a DB transaction.
// Square is NOT called yet — if the tx fails, no harm done.
@@ -4984,25 +4918,11 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
// enforced. New-card (nonce) charges are not gated. This runs AFTER the
// idempotency dedup's completed short-circuit (Loop B MEDIUM): a same-key
// lost-response retry returns the already-completed payment above without
// re-entering the gate, so its single-use code (already consumed by the
// original attempt) is never re-rejected as "expired". consume=!reusePendingRecord
// (finding 4): a FRESH charge verifies WITH consumption — the code is
// single-use at the gate, closing the TOCTOU where a verified-but-
// unconsumed code could authorize a second charge — and a pending-reuse
// retry verifies WITHOUT consuming, so a retry that fails again keeps its
// code for one more attempt (the completed-charge transaction consumes it
// on terminal success). A charge carrying a Square verification_token (SCA
// performed) skips the gate; a token-less charge falls back to 2FA and
// twoFAFallbackUsed is set for the charge-success audit.
// re-entering the gate. A charge carrying a Square verification_token (SCA
// performed) skips the gate; a token-less charge is refused 402
// verification_required (SCA-only — the homegrown 2FA fallback was removed).
if req.CardID != nil && *req.CardID != "" {
var gateOK bool
gateOK, twoFAFallbackUsed = requireTwoFactorForCardAccess(w, r, service, userID, req.VerificationCode, tipVerificationToken, !reusePendingRecord)
if !gateOK {
return
}
// C6: a fallback-authorized charge must carry the customer's accepted
// consent (403 consent_required otherwise).
if !enforceSCAFallbackConsent(w, req.ConsentVersion, req.ConsentAccepted, twoFAFallbackUsed) {
if gateOK, _ := requireTwoFactorForCardAccess(w, r, service, userID, tipVerificationToken, !reusePendingRecord); !gateOK {
return
}
}
@@ -5103,18 +5023,6 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
if err != nil {
log.Printf("Failed to create tip payment: %v (error_code=%q)", err, square.ErrorCode(err))
// Payment record intentionally left as 'pending' for manual retry.
// The gate consumed the 2FA code for a fresh saved-card charge —
// re-issue so the same-key retry has a live code to verify (mirrors
// CreateBookingPayment's post-failure re-issue, finding 4). A NEW-CARD
// (cnon) charge never gated and involves no code — re-issuing would
// overwrite a standing pending code with an undelivered one (finding
// 5). A NEW-CARD charge with save_card=true DID gate (the SAVE gate
// consumed the code), so the code is re-issued there too (finding:
// save-gate burned code never re-issued). A pending-reuse retry verified
// WITHOUT consuming, so its code is still live and no re-issue runs.
if (req.CardID != nil && *req.CardID != "") || req.SaveCard {
reissueTwoFACodeAfterFailedCharge(r.Context(), db.Conn, userID, true, twoFAFallbackUsed && !reusePendingRecord, r)
}
// SCA-required failures must surface the structured verification_required
// body so the frontend triggers the 3DS challenge, not a plain decline.
if isVerificationRequiredError(err) {
@@ -5187,13 +5095,6 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
return
}
// The 2FA BACKUP authorized this token-less saved-card tip charge (SCA was
// unavailable) — record the strict fallback audit row. The actor is the
// customer's own userID (customer-initiated online charge).
if twoFAFallbackUsed {
insertTwoFAFallbackAudit(r.Context(), userID, userID, paymentResult.CardLast4, bookingID, "saved-card tip charge authorized via 2FA fallback (SCA unavailable)", consentVersionValue(req.ConsentVersion), req.ConsentAccepted)
}
if err := json.NewEncoder(w).Encode(PaymentResponse{
ID: paymentID,
BookingID: bookingID,
backend/handlers/payments/handlers_test.go
+14 -9
View File
@@ -437,9 +437,10 @@ func TestTwoFactorEnforced_CreatePaymentMethod_SCATokenizeResult_Save_Succeeds(t
// TestTwoFactorEnforced_CreatePaymentMethod_ForgeVerificationToken_402 pins
// auth-F1 on the add-card surface: a verification_token is CLIENT-ASSERTED and
// never forwarded to Square on a SAVE surface (CreateCardOnFile takes no token),
// so it must NOT skip the gate — enforced + a raw nonce card_token + a (forged)
// non-empty verification_token is refused 402 verification_required and no card
// is persisted. The M11 fix cannot be a client-asserted token bypass.
// so it must NOT skip the gate — enforced + a NON-token-like card_token (a raw
// PAN — the only shape the gate still refuses) + a (forged) non-empty
// verification_token is refused 402 verification_required and no card is
// persisted. The M11 fix cannot be a client-asserted token bypass.
func TestTwoFactorEnforced_CreatePaymentMethod_ForgeVerificationToken_402(t *testing.T) {
helperEnvEnforce2FAStaging(t)
ctx, tx := testutils.SetupTestTx(t)
@@ -450,7 +451,7 @@ func TestTwoFactorEnforced_CreatePaymentMethod_ForgeVerificationToken_402(t *tes
forged := "forged-verification-token"
req := CreatePaymentMethodRequest{
CardToken: "cnon:2fa-forged-save",
CardToken: "4111111111111111",
VerificationToken: &forged,
}
w := makePaymentRequest(CreatePaymentMethod, "POST", "/api/user/payment-methods", req, token, ctx)
@@ -466,10 +467,14 @@ func TestTwoFactorEnforced_CreatePaymentMethod_ForgeVerificationToken_402(t *tes
}
// TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_Refused402 pins the M11
// refusal half in the SCA suite's own staging helper: a legacy token-less save
// (a raw card.tokenize() nonce carrying no SCA proof) is refused 402
// verification_required in an enforced deployment and no card is persisted.
// Companion to errors_test.go's TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_402.
// refusal half in the SCA suite's own staging helper: a NON-token-like save
// source (a raw PAN — the gate's only remaining refusal shape) is refused 402
// verification_required in an enforced deployment and no card is persisted. A
// GENUINE token-like source (cnon:/ccof:) is SCA-proven — the STORE-intent SCA
// ran at tokenization — and skips the gate (see
// TestTwoFactorEnforced_CreatePaymentMethod_SCATokenizeResult_Save_Succeeds).
// Companion to TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_402
// (errors_test.go).
func TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_Refused402(t *testing.T) {
helperEnvEnforce2FAStaging(t)
ctx, tx := testutils.SetupTestTx(t)
@@ -478,7 +483,7 @@ func TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_Refused402(t *testing.T
require.NoError(t, err)
token := jwt.GenerateUserToken(userID)
req := CreatePaymentMethodRequest{CardToken: "cnon:2fa-tokenless-save"}
req := CreatePaymentMethodRequest{CardToken: "4111111111111111"}
w := makePaymentRequest(CreatePaymentMethod, "POST", "/api/user/payment-methods", req, token, ctx)
require.Equal(t, http.StatusPaymentRequired, w.Code,
"a token-less save must be refused 402 in an enforced deployment, body: %s", w.Body.String())
backend/handlers/payments/loop_b_money_fixes_test.go
+7 -2
View File
@@ -165,11 +165,16 @@ func TestBookingPayment_DepositCoveredDiscount_RecordsDiscount_NoOvercharge(t *t
assert.Equal(t, 50.00, discountAmount, "the promised £50 discount must be recorded at the skip path")
// Ledger: real £50 + discount £50 = £100 = total. The customer pays the
// discounted £50, never the full £100.
// discounted £50, never the full £100. GetBookingRemainingBalancePence
// counts only REAL money — a discount row is a ledger entry, not a payment
// toward the balance (the real-money convention every other paid
// computation applies) — so the remaining balance reports the full £50 even
// though the booking auto-completed (bookingIsFullyPaid counts the discount
// row toward completion).
var remainingPence int64
remainingPence, err = NewPaymentService().GetBookingRemainingBalancePence(ctx, bookingID)
require.NoError(t, err)
assert.Equal(t, int64(0), remainingPence, "the discounted booking must have £0 remaining after the covered deposit")
assert.Equal(t, int64(5000), remainingPence, "the remaining balance counts real money only — the discount row is not 'paid'")
// A later unconfirmed balance charge of £50 must be REJECTED — the booking
// auto-completed when the deposit + discount settled it, so the completed-
backend/handlers/payments/loyalty.go
+11 -4
View File
@@ -57,7 +57,14 @@ func ApplyLoyaltyRedemption(w http.ResponseWriter, r *http.Request) {
http.Error(w, "internal server error", http.StatusInternalServerError)
return
}
if bookingUserID != userID {
// Ownership: the booking's own customer may redeem their pending
// redemption, and an admin acting on ANY booking may too (the admin
// "Take Payment" PaymentModal applies the customer's redemption on their
// behalf — the admin route /admin/bookings/{id}/apply-redemption mounts
// this same handler under RequireAdmin). All the writes below target the
// BOOKING's user (bookingUserID), never the acting admin.
userRole, _ := r.Context().Value(mw.UserRoleKey).(string)
if userRole != "admin" && bookingUserID != userID {
http.Error(w, "Unauthorized", http.StatusForbidden)
return
}
@@ -148,7 +155,7 @@ func ApplyLoyaltyRedemption(w http.ResponseWriter, r *http.Request) {
if _, err := tx.Exec(r.Context(), `
INSERT INTO booking_discounts (booking_id, user_id, discount_source, source_id, campaign_type, milestone_type, discount_percent, original_total, discount_amount)
VALUES ($1, $2, 'loyalty', $3, NULL, NULL, $6, $4, $5)
`, bookingID, userID, redemptionID, bookingTotal, discountAmount, LoyaltyDiscountPercent); err != nil {
`, bookingID, bookingUserID, redemptionID, bookingTotal, discountAmount, LoyaltyDiscountPercent); err != nil {
log.Printf("Failed to insert booking discount: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
@@ -157,7 +164,7 @@ func ApplyLoyaltyRedemption(w http.ResponseWriter, r *http.Request) {
if _, err := tx.Exec(r.Context(), `
INSERT INTO payments (booking_id, payment_type, payment_method, amount, status, created_by)
VALUES ($1, 'partial', 'discount', $2, 'completed', $3)
`, bookingID, discountAmount, userID); err != nil {
`, bookingID, discountAmount, bookingUserID); err != nil {
log.Printf("Failed to insert payment record: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
@@ -174,7 +181,7 @@ func ApplyLoyaltyRedemption(w http.ResponseWriter, r *http.Request) {
if _, err := tx.Exec(r.Context(), `
UPDATE users SET loyalty_stamps = GREATEST(0, loyalty_stamps - $1) WHERE id = $2
`, LoyaltyStampCost, userID); err != nil {
`, LoyaltyStampCost, bookingUserID); err != nil {
log.Printf("Failed to update loyalty stamps: %v", err)
http.Error(w, "internal server error", http.StatusInternalServerError)
return
backend/handlers/payments/max_online_tip_test.go
+243
View File
@@ -0,0 +1,243 @@
//go:build test && dev
package payments
import (
"encoding/json"
"net/http"
"testing"
"time"
"crussell/clock"
"crussell/testutils"
"crussell/testutils/fixtures"
"crussell/testutils/jwt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// =============================================================================
// maxOnlineTipPence: online tip business bound (CreateTipPayment)
// =============================================================================
// TestTipPayment_AtBound_Accepted pins the online tip bound's inclusive edge: a
// tip exactly at maxOnlineTipPence (£250) is a legitimate business amount and
// must flow through the normal happy path (charge, completed tip record).
func TestTipPayment_AtBound_Accepted(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
_, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed")
require.NoError(t, err)
userToken := jwt.GenerateUserToken(userID)
cardToken := "cnon:tip-at-bound"
req := CreateTipPaymentRequest{
Amount: maxOnlineTipPence,
NewCardToken: &cardToken,
}
handler := CreateTipPayment
w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, ctx)
require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
var resp PaymentResponse
require.NoError(t, json.NewDecoder(w.Body).Decode(&resp))
assert.Equal(t, "tip", resp.PaymentType)
assert.Equal(t, "completed", resp.Status)
assert.Equal(t, maxOnlineTipPence, resp.Amount, "the at-bound tip amount must be charged verbatim")
}
// TestTipPayment_OverBound_Rejected pins the online tip bound's exclusive edge:
// any tip above maxOnlineTipPence (£250) is rejected with 400 and a clear
// message, and the rejection fires before any pending record insert or Square
// charge — no tip payment row may be created.
func TestTipPayment_OverBound_Rejected(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
_, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed")
require.NoError(t, err)
userToken := jwt.GenerateUserToken(userID)
cardToken := "cnon:tip-over-bound"
req := CreateTipPaymentRequest{
Amount: maxOnlineTipPence + 1,
NewCardToken: &cardToken,
}
handler := CreateTipPayment
w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, ctx)
require.Equal(t, http.StatusBadRequest, w.Code, "body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "Tip exceeds the maximum allowed amount")
var tipCount int
err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_type = 'tip'`, bookingID).Scan(&tipCount)
require.NoError(t, err)
assert.Equal(t, 0, tipCount, "an over-bound tip must not create any payment record")
}
// =============================================================================
// maxOnlineTipPence applies to the CreateBookingPayment OVERFLOW carve too:
// the B12 gate (confirm_overflow_tip) must cap the tip portion it mints at
// £250, mirroring the dedicated tip endpoint. A confirmed £10,000 payment on a
// booking with £50 remaining would otherwise carve a £9,950 tip row.
// =============================================================================
// TestBookingPayment_OverflowTip_OverCap_Rejected_PostStart is the confirmed
// bypass regression: a POST-START booking (the carve at handlers.go's
// buildSplitRecords post-start branch) with £50 remaining and a confirmed
// £10,000 payment must be rejected 400 with the tip-cap message — even though
// confirm_overflow_tip=true — and must not create any payment row.
func TestBookingPayment_OverflowTip_OverCap_Rejected_PostStart(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
cardToken := "cnon:overflow-over-cap-post"
req := CreateBookingPaymentRequest{
Amount: 1000000, // £10,000 on a £50-remaining booking → £9,950 tip portion
PaymentType: "full",
NewCardToken: &cardToken,
IdempotencyKey: "overflow-over-cap-post-" + bookingID,
ConfirmOverflowTip: true,
}
handler := CreateBookingPayment
w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusBadRequest, w.Code, "an over-cap overflow must be rejected even when confirmed, body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "Tip exceeds the maximum allowed amount")
var payCount int
err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1`, bookingID).Scan(&payCount)
require.NoError(t, err)
assert.Zero(t, payCount, "the rejected over-cap overflow must not create any payment record")
}
// TestBookingPayment_OverflowTip_OverCap_Rejected_PreStart is the same bypass
// regression for the PRE-START carve (deposit/balance/tip split): the
// confirmed £10,000 payment must be rejected before any pending row or Square
// charge, so no tip can be minted over the cap.
func TestBookingPayment_OverflowTip_OverCap_Rejected_PreStart(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestData(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
cardToken := "cnon:overflow-over-cap-pre"
req := CreateBookingPaymentRequest{
Amount: 1000000, // £10,000 on a £50-remaining booking → £9,950 tip portion
PaymentType: "full",
NewCardToken: &cardToken,
IdempotencyKey: "overflow-over-cap-pre-" + bookingID,
ConfirmOverflowTip: true,
}
handler := CreateBookingPayment
w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusBadRequest, w.Code, "an over-cap overflow must be rejected even when confirmed, body: %s", w.Body.String())
assert.Contains(t, w.Body.String(), "Tip exceeds the maximum allowed amount")
var payCount int
err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1`, bookingID).Scan(&payCount)
require.NoError(t, err)
assert.Zero(t, payCount, "the rejected over-cap overflow must not create any payment record")
}
// TestBookingPayment_OverflowTip_WithinCap_CarvesTip pins the inclusive edge:
// a confirmed £250 payment on a £50-remaining booking (tip portion £200 — under
// the £250 cap) proceeds and buildSplitRecords carves a tip row of EXACTLY
// £200 alongside a £50 booking portion.
func TestBookingPayment_OverflowTip_WithinCap_CarvesTip(t *testing.T) {
t.Parallel()
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
cardToken := "cnon:overflow-within-cap"
req := CreateBookingPaymentRequest{
Amount: 25000, // £250 on a £50-remaining booking → tip portion £200
PaymentType: "full",
NewCardToken: &cardToken,
IdempotencyKey: "overflow-within-cap-" + bookingID,
ConfirmOverflowTip: true,
}
handler := CreateBookingPayment
w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusOK, w.Code, "a within-cap overflow must proceed, body: %s", w.Body.String())
var tipCount int
var tipAmount float64
err := tx.QueryRow(ctx, `SELECT COUNT(*), COALESCE(SUM(amount), 0) FROM payments WHERE booking_id = $1 AND status = 'completed' AND payment_type = 'tip'`, bookingID).Scan(&tipCount, &tipAmount)
require.NoError(t, err)
assert.Equal(t, 1, tipCount, "a within-cap overflow must carve exactly one tip record")
assert.InDelta(t, 200.0, tipAmount, 0.001, "the carved tip must equal the £200 overflow, not more")
var bookingPortion float64
err = tx.QueryRow(ctx, `SELECT COALESCE(SUM(amount), 0) FROM payments WHERE booking_id = $1 AND status = 'completed' AND payment_type = 'full'`, bookingID).Scan(&bookingPortion)
require.NoError(t, err)
assert.InDelta(t, 50.0, bookingPortion, 0.001, "the booking portion must remain the £50 obligation")
}
// TestBuildSplitRecords_OverCapTip_Rejected proves the belt-and-braces cap in
// buildSplitRecords itself: even a caller that skips the B12 gate cannot mint a
// tip row over maxOnlineTipPence — both the post-start and pre-start carve
// return an error instead of minting the over-cap tip. A £10,000 charge on a
// £50 booking would otherwise carve a £9,950 tip row.
func TestBuildSplitRecords_OverCapTip_Rejected(t *testing.T) {
t.Parallel()
record := makeTestRecord("b-cap-reject", "full", 10000)
info := &BookingPaymentInfo{
StartTime: clock.Now().Add(-2 * time.Hour),
TotalAmount: 50,
TotalPaid: 0,
}
// Post-start carve: booking portion £50, tip portion £9,950 — over the cap.
records, err := buildSplitRecords(record, "full", info, 10000)
require.Error(t, err, "the post-start carve must reject an over-cap tip portion")
assert.Contains(t, err.Error(), "exceeding the £250 online tip cap")
assert.Nil(t, records)
// Pre-start carve: deposit £25 + balance £25, tip portion £9,950 — over the cap.
preInfo := &BookingPaymentInfo{
StartTime: clock.Now().Add(48 * time.Hour),
TotalAmount: 50,
TotalPaid: 0,
}
records, err = buildSplitRecords(record, "full", preInfo, 10000)
require.Error(t, err, "the pre-start carve must reject an over-cap tip portion")
assert.Contains(t, err.Error(), "exceeding the £250 online tip cap")
assert.Nil(t, records)
// Inclusive boundary: a £250 charge on the £50 booking (tip portion £200)
// stays below the cap and splits normally (single post-start record pair).
withinRecord := makeTestRecord("b-cap-within", "full", 250)
withinInfo := &BookingPaymentInfo{
StartTime: clock.Now().Add(-2 * time.Hour),
TotalAmount: 50,
TotalPaid: 0,
}
within, err := buildSplitRecords(withinRecord, "full", withinInfo, 250)
require.NoError(t, err, "a within-cap tip portion must split normally")
require.Len(t, within, 2)
assert.Equal(t, "full", within[0].PaymentType)
assert.InDelta(t, 50.0, within[0].Amount, 0.001)
assert.Equal(t, "tip", within[1].PaymentType)
assert.InDelta(t, 200.0, within[1].Amount, 0.001)
}
backend/handlers/payments/money_edge_test.go
+82
View File
@@ -0,0 +1,82 @@
//go:build test && dev
package payments
import (
"math"
"testing"
"github.com/stretchr/testify/require"
)
// TestPenceLess_RoundingBoundary pins the pence rounding that decides whether a
// gift-card clawback was PARTIAL: penceLess compares pound-float balances by
// rounding to integer pence, so sub-penny residue around roundingEpsilon
// (0.004) must not flip the comparison. Amounts at or below the epsilon (0.4
// pence) round to zero pence; 0.5+ pence rounds up.
func TestPenceLess_RoundingBoundary(t *testing.T) {
t.Parallel()
tests := []struct {
name string
a, b float64
less bool
}{
{"both at the epsilon round to 0 pence — not less", 0.004, 0.004, false},
{"0.004 vs 0.0041 both round to 0 pence", 0.004, 0.0041, false},
{"0.0039 vs 0.004 both round to 0 pence", 0.0039, 0.004, false},
{"0.0041 vs 0.0039 both round to 0 pence", 0.0041, 0.0039, false},
{"0.004 (0 pence) is less than 0.005 (1 pence)", 0.004, 0.005, true},
{"0.005 (1 pence) is NOT less than 0.004 (0 pence)", 0.005, 0.004, false},
{"0.004 is less than 0.01 (1 pence)", 0.004, 0.01, true},
{"equal amounts are never less", 12.34, 12.34, false},
{"£12.34 is less than £12.35", 12.34, 12.35, true},
{"£12.35 is NOT less than £12.34", 12.35, 12.34, false},
{"a full pound difference", 0.0, 1.0, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.less, penceLess(tt.a, tt.b), "penceLess(%v, %v)", tt.a, tt.b)
})
}
}
// TestPenceLess_FloatToPenceRoundingEdges pins the float64→pence rounding the
// comparison is built on: Go's math.Round rounds half away from zero, so
// exactly 0.5 pence rounds UP (0.005 → 1 pence) while anything below rounds
// down. The clawback's partial-detection must agree with this everywhere a
// pound-denominated balance is compared.
func TestPenceLess_FloatToPenceRoundingEdges(t *testing.T) {
t.Parallel()
// Exactly 0.5 pence rounds up: 0.005 → 1p, so 0.005 < 0.006 is false
// (both round to 1) and 0.004999999 < 0.005000001 is true (0p vs 1p).
require.Equal(t, int64(1), int64(math.Round(0.005*100)), "0.005 pounds must round to 1 pence")
require.Equal(t, int64(0), int64(math.Round(0.004999999*100)), "0.004999999 pounds must round to 0 pence")
require.False(t, penceLess(0.005, 0.006), "0.005 and 0.006 both round to 1 pence")
require.True(t, penceLess(0.004999999, 0.005000001), "0p vs 1p — the partial boundary")
// A whole-pence gap always compares by rounded pence, never raw floats.
require.False(t, penceLess(1.009, 1.01), "1.009 rounds to 1.01 → equal pence")
require.False(t, penceLess(1.009, 1.011), "1.009 and 1.011 both round to 1.01 → equal pence")
require.True(t, penceLess(1.009, 1.021), "1.009 rounds to 1.01, 1.021 rounds to 1.02 → less")
}
// TestRoundingEpsilon_Value pins the "effectively zero" threshold shared by the
// split builders and the cash/gift-card terminal branches. It must stay 0.004
// (0.4 pence): sub-penny float residue from dividing pence by 100 must round to
// zero pence, so a phantom payment row is never created, while a real 0.5-penny
// value still rounds to 1 pence.
func TestRoundingEpsilon_Value(t *testing.T) {
t.Parallel()
require.Equal(t, 0.004, roundingEpsilon, "roundingEpsilon must be 0.004 (0.4 pence)")
// The epsilon itself rounds to zero pence; the first rounding-up value is
// 0.005. Any epsilon above 0.004 would silently drop legitimate half-pence
// amounts; anything below would over-report residue.
require.Equal(t, int64(0), int64(math.Round(roundingEpsilon*100)))
require.Equal(t, int64(1), int64(math.Round(0.005*100)))
require.Equal(t, int64(0), int64(math.Round(0.0039*100)))
require.Equal(t, int64(0), int64(math.Round(0.0041*100)))
}
backend/handlers/payments/payments_test.go
+35 -22
View File
@@ -45,26 +45,29 @@ func adminRequestCtx(r *http.Request) *http.Request {
func TestValidateCardInfo(t *testing.T) {
empty := ""
cardID := "card_123"
savedCardID := "card_456"
token := "cnon:test"
tests := []struct {
name string
cardID *string
newToken *string
wantError bool
name string
cardID *string
savedCardID *string
newToken *string
wantError bool
}{
{"both set rejected", &cardID, &token, true},
{"card_id only ok", &cardID, nil, false},
{"token only ok", nil, &token, false},
{"neither set rejected", nil, nil, true},
{"empty card_id rejected", &empty, nil, true},
{"empty token rejected", nil, &empty, true},
{"both empty rejected", &empty, &empty, true},
{"card_id + token ok (SCA tokenize-result)", &cardID, nil, &token, false},
{"saved_card_id + token ok (SCA tokenize-result)", nil, &savedCardID, &token, false},
{"card_id only ok", &cardID, nil, nil, false},
{"token only ok", nil, nil, &token, false},
{"neither set rejected", nil, nil, nil, true},
{"empty card_id rejected", &empty, nil, nil, true},
{"empty token rejected", nil, nil, &empty, true},
{"both empty rejected", &empty, nil, &empty, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateCardInfo(tt.cardID, tt.newToken)
err := ValidateCardInfo(tt.cardID, tt.savedCardID, tt.newToken)
if tt.wantError {
assert.Error(t, err)
} else {
@@ -2181,7 +2184,8 @@ func TestBuildSplitRecords_FutureBooking_FullPayment_Splits(t *testing.T) {
TotalAmount: 50,
TotalPaid: 0,
}
records := buildSplitRecords(record, "full", info, 50)
records, err := buildSplitRecords(record, "full", info, 50)
require.NoError(t, err)
if len(records) != 2 {
t.Fatalf("expected 2 records, got %d", len(records))
@@ -2220,7 +2224,8 @@ func TestBuildSplitRecords_PastBooking_NoSplit(t *testing.T) {
TotalAmount: 50,
TotalPaid: 0,
}
records := buildSplitRecords(record, "full", info, 50)
records, err := buildSplitRecords(record, "full", info, 50)
require.NoError(t, err)
if len(records) != 1 {
t.Fatalf("expected 1 record (no split), got %d", len(records))
@@ -2238,7 +2243,8 @@ func TestBuildSplitRecords_DepositWithinCap_NoSplit(t *testing.T) {
TotalAmount: 50,
TotalPaid: 0,
}
records := buildSplitRecords(record, "deposit", info, 20)
records, err := buildSplitRecords(record, "deposit", info, 20)
require.NoError(t, err)
if len(records) != 1 {
t.Fatalf("expected 1 record (within cap), got %d", len(records))
@@ -2256,7 +2262,8 @@ func TestBuildSplitRecords_PaymentLessThanDepositMax_NoSplit(t *testing.T) {
TotalAmount: 100,
TotalPaid: 0,
}
records := buildSplitRecords(record, "deposit", info, 25)
records, err := buildSplitRecords(record, "deposit", info, 25)
require.NoError(t, err)
if len(records) != 1 {
t.Fatalf("expected 1 record (under 50%%), got %d", len(records))
@@ -2273,7 +2280,8 @@ func TestBuildSplitRecords_OverflowBeyondTotal_BecomesTip(t *testing.T) {
TotalAmount: 50,
TotalPaid: 0,
}
records := buildSplitRecords(record, "full", info, 60)
records, err := buildSplitRecords(record, "full", info, 60)
require.NoError(t, err)
if len(records) != 3 {
t.Fatalf("expected 3 records (deposit + balance + tip), got %d", len(records))
@@ -2328,7 +2336,8 @@ func TestBuildSplitRecords_DiscountBooking_TipOverflow_SumNeverExceedsCharge(t *
TotalAmount: 100,
TotalPaid: 0, // the £10 discount payment row is excluded here
}
records := buildSplitRecords(record, "full", info, 120)
records, err := buildSplitRecords(record, "full", info, 120)
require.NoError(t, err)
var sum float64
for _, r := range records {
@@ -2351,7 +2360,8 @@ func TestBuildSplitRecords_DiscountBooking_TipOverflow_SumNeverExceedsCharge(t *
TotalAmount: 100,
TotalPaid: 60,
}
records2 := buildSplitRecords(record2, "full", info2, 70)
records2, err2 := buildSplitRecords(record2, "full", info2, 70)
require.NoError(t, err2)
var sum2 float64
for _, r := range records2 {
sum2 += r.Amount
@@ -2399,7 +2409,8 @@ func TestGetBookingPaymentInfo_ExcludesDiscountPayments(t *testing.T) {
// A £70 tip-overflow charge on top must partition exactly into £70 of split
// records — the discount can never push the recorded sum past the charge.
record := makeTestRecord(bookingID, "full", 70)
records := buildSplitRecords(record, "full", info, 70)
records, err := buildSplitRecords(record, "full", info, 70)
require.NoError(t, err)
var sum float64
for _, r := range records {
sum += r.Amount
@@ -2448,7 +2459,8 @@ func TestBuildSplitRecords_DepositCarve_PenceExact(t *testing.T) {
TotalAmount: tc.total,
TotalPaid: tc.paid,
}
records := buildSplitRecords(record, "full", info, chargedPounds)
records, err := buildSplitRecords(record, "full", info, chargedPounds)
require.NoError(t, err)
var depositPence, balancePence, tipPence, partitionPence int64
for _, r := range records {
@@ -4555,7 +4567,8 @@ func TestBuildSplitRecords_TipOverflow_SeparateTipRecord(t *testing.T) {
TotalAmount: 50,
TotalPaid: 0,
}
records := buildSplitRecords(record, "full", info, 60)
records, err := buildSplitRecords(record, "full", info, 60)
require.NoError(t, err)
if len(records) != 3 {
t.Fatalf("expected 3 records (deposit + balance + tip), got %d", len(records))
backend/handlers/payments/policy_ts_crosscheck_test.go
+74
View File
@@ -0,0 +1,74 @@
//go:build test && dev
package payments
import (
"os"
"path/filepath"
"regexp"
"strconv"
"testing"
"github.com/stretchr/testify/require"
)
// policyTSValue extracts the numeric value of a `NAME: value,` entry from the
// frontend's POLICY object literal (policy.ts), tolerating the tab-indented
// formatting the file uses. Returns "" when the name is absent.
func policyTSValue(t *testing.T, src, name string) string {
t.Helper()
re := regexp.MustCompile(`(?m)^\s*` + regexp.QuoteMeta(name) + `:\s*(\d+(?:\.\d+)?),`)
m := re.FindStringSubmatch(src)
if m == nil {
return ""
}
return m[1]
}
// TestPolicyTS_CrossCheck pins the frontend's single-source policy constants
// (frontend/src/lib/constants/policy.ts) to THIS package's refund_policy.go
// values by REACTING to drift: the test reads the .ts file and asserts each
// entry equals the backend constant, so a one-sided change on either side fails
// CI. The frontend's own vitest suite (policy.test.ts) pins the same values in
// the other direction, closing the drift loop both ways.
func TestPolicyTS_CrossCheck(t *testing.T) {
path := filepath.Join("..", "..", "..", "frontend", "src", "lib", "constants", "policy.ts")
data, err := os.ReadFile(path)
require.NoError(t, err, "policy.ts not found at %s (tests run from the package dir; repo layout is <repo>/backend/handlers/payments + <repo>/frontend)", path)
src := string(data)
cases := []struct {
name string
want string
}{
{"REQUIRED_DEPOSIT_PCT", "0.2"},
{"PROTECTED_DEPOSIT_MAX_PCT", "0.5"},
{"FULL_REFUND_THRESHOLD_HOURS", "72"},
{"PARTIAL_REFUND_THRESHOLD_HOURS", "24"},
{"NO_SHOW_THRESHOLD_HOURS", "24"},
{"DEPOSIT_ADVANCE_HOURS", "36"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
got := policyTSValue(t, src, tc.name)
require.NotEmpty(t, got, "policy.ts no longer declares %q — the constant may have been renamed or removed", tc.name)
require.Equal(t, tc.want, got, "%s drifted from backend/handlers/payments/refund_policy.go", tc.name)
})
}
// The backend has no named reschedule constants; the frontend values must
// track the refund tiers they were derived from.
for _, tc := range []struct {
name string
want string
}{
{"RESCHEDULE_BLOCK_HOURS_WITH_PAYMENTS", strconv.Itoa(int(FullRefundThreshold.Hours()))},
{"RESCHEDULE_BLOCK_HOURS_NO_PAYMENTS", strconv.Itoa(int(PartialRefundThreshold.Hours()))},
} {
t.Run(tc.name, func(t *testing.T) {
got := policyTSValue(t, src, tc.name)
require.NotEmpty(t, got, "policy.ts no longer declares %q", tc.name)
require.Equal(t, tc.want, got, "%s must track the backend refund threshold", tc.name)
})
}
}
backend/handlers/payments/refunds.go
+1
View File
@@ -1136,6 +1136,7 @@ func insertRefundFailedNotifications(ctx context.Context, refundIDs []string) {
AND NOT EXISTS (
SELECT 1 FROM admin_notifications an
WHERE an.reason = 'refund_failed' AND an.booking_id = refunds.booking_id
AND an.acknowledged_at IS NULL
)
AND (SELECT COUNT(*) FROM admin_notifications _an
WHERE _an.reason = 'refund_failed'
backend/handlers/payments/sca_mock_parity_test.go
+163
View File
@@ -0,0 +1,163 @@
//go:build test && dev
package payments
import (
"encoding/json"
"net/http"
"testing"
"crussell/internal/square"
"crussell/testutils"
"crussell/testutils/fixtures"
"crussell/testutils/jwt"
"github.com/stretchr/testify/require"
)
// =============================================================================
// MEDIUM-HIGH: dev mock SCA error-shape parity at the HANDLER level.
//
// square_dev.go's SimulateSavedCardVerificationRequired is exercised heavily in
// internal/square's own suite, but the frontend only ever sees the MOCK's SCA
// rejection through a payment HANDLER — the 402 + {"code":"verification_required"}
// body is what the SCA challenge flow keys on. These tests pin that the mock's
// simulated SCA rejection surfaces the structured body end to end, and that the
// ENFORCED-deployment gate refuses token-less saved-card charges while genuine
// SCA tokenize-results (new_card_token = cnon:sca-...) sail through.
// =============================================================================
// TestBookingPayment_MockSavedCardSCA_Tokenless_402_VerificationRequired drives
// the mock's SimulateSavedCardVerificationRequired toggle through
// CreateBookingPayment: a token-less saved-card (ccof) charge is rejected by
// the mock with CARD_DECLINED_VERIFICATION_REQUIRED, and the handler must
// surface it as 402 + the structured verification_required body — the exact
// shape the frontend's isVerificationRequiredSignal parses to trigger the
// client-side challenge.
func TestBookingPayment_MockSavedCardSCA_Tokenless_402_VerificationRequired(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestData(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:mock_sca_booking", "VISA", "4242")
require.NoError(t, err)
origClient := SquareClient
mc := square.NewDevClient().(*square.MockClient)
mc.SimulateSavedCardVerificationRequired = true
SquareClient = mc
defer func() { SquareClient = origClient }()
req := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
CardID: &cardID,
IdempotencyKey: "mock-sca-booking-tokenless",
}
w := makePaymentRequest(withNonGuest(CreateBookingPayment), "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
assertStructuredVerificationRequired(t, w)
// A refused charge must not record a completed payment.
var payCount int
require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND status = 'completed'`, bookingID).Scan(&payCount))
require.Zero(t, payCount, "a mock-SCA-refused saved-card charge must not record a completed payment")
}
// TestBookingPayment_MockSavedCardSCA_SCATokenizeResult_Succeeds proves the
// other half of the mock parity: under the SAME SimulateSavedCardVerificationRequired
// toggle, a saved-card charge carrying a GENUINE tokenize-result
// (new_card_token = cnon:sca-... — card.tokenize(verificationDetails, cardId))
// is accepted by the mock (the token IS the buyer verification) and the handler
// completes the charge.
func TestBookingPayment_MockSavedCardSCA_SCATokenizeResult_Succeeds(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestData(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:mock_sca_booking_ok", "VISA", "4242")
require.NoError(t, err)
origClient := SquareClient
mc := square.NewDevClient().(*square.MockClient)
mc.SimulateSavedCardVerificationRequired = true
SquareClient = mc
defer func() { SquareClient = origClient }()
scaToken := "cnon:sca-4242_2500_ok"
req := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
CardID: &cardID,
NewCardToken: &scaToken,
IdempotencyKey: "mock-sca-booking-tokenized",
}
w := makePaymentRequest(withNonGuest(CreateBookingPayment), "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusOK, w.Code, "a genuine SCA tokenize-result must complete the saved-card charge, body: %s", w.Body.String())
}
// TestBookingPayment_EnforcedSavedCard_Tokenless_402 pins the SCA-only gate on
// the booking surface in an ENFORCED deployment: a token-less saved-card charge
// is refused 402 verification_required up front (PSR 2017 reg 100 — no homegrown
// 2FA fallback), before any charge reaches Square.
func TestBookingPayment_EnforcedSavedCard_Tokenless_402(t *testing.T) {
helperEnvEnforce2FAStaging(t)
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestData(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:mock_enforced_booking", "VISA", "4242")
require.NoError(t, err)
req := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
CardID: &cardID,
IdempotencyKey: "enforced-booking-tokenless",
}
w := makePaymentRequest(withNonGuest(CreateBookingPayment), "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
assertStructuredVerificationRequired(t, w)
var body map[string]string
require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body))
require.Equal(t, "verification_required", body["code"])
}
// TestBookingPayment_EnforcedSavedCard_SCATokenizeResult_Succeeds proves the
// SCA-primary skip in an ENFORCED deployment: a saved-card charge carrying a
// genuine SCA tokenize-result (new_card_token = cnon:sca-...) skips BOTH 2FA
// gates (scaTokenizedSavedCard) and completes — the wire contract the frontend
// sends after a successful tokenizeWithVerification challenge.
func TestBookingPayment_EnforcedSavedCard_SCATokenizeResult_Succeeds(t *testing.T) {
helperEnvEnforce2FAStaging(t)
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestData(t, ctx, tx)
userToken := jwt.GenerateUserToken(userID)
cardID, err := fixtures.CreateTestPaymentMethod(tx, userID, "ccof:mock_enforced_booking_ok", "VISA", "4242")
require.NoError(t, err)
origClient := SquareClient
mc := square.NewDevClient().(*square.MockClient)
mc.SimulateSavedCardVerificationRequired = true
SquareClient = mc
defer func() { SquareClient = origClient }()
scaToken := "cnon:sca-4242_2500_ok"
req := CreateBookingPaymentRequest{
Amount: 2500,
PaymentType: "deposit",
CardID: &cardID,
NewCardToken: &scaToken,
IdempotencyKey: "enforced-booking-tokenized",
}
w := makePaymentRequest(withNonGuest(CreateBookingPayment), "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
require.Equal(t, http.StatusOK, w.Code, "an SCA tokenize-result must skip the enforced gate and complete, body: %s", w.Body.String())
var payCount int
require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND status = 'completed'`, bookingID).Scan(&payCount))
require.Equal(t, 1, payCount, "the SCA-tokenized charge must record exactly one completed payment")
}
backend/handlers/payments/service.go
+7
View File
@@ -550,6 +550,13 @@ func (s *PaymentService) GetBookingRemainingBalancePence(ctx context.Context, bo
-- A tip is money paid beyond the booking total — it does not
-- reduce the balance owed, so it must not count as "paid".
AND payment_type <> 'tip'
-- A discount / on_the_house row is a ledger entry, not real money
-- toward the booking (the real-money convention every other paid
-- computation applies — e.g. the deposit-threshold query and
-- GetBookingRefundableAmountPence). Counting them as "paid" would
-- understate the remaining balance, and a fully-discounted
-- booking would appear fully paid and refuse legitimate charges.
AND payment_method NOT IN ('discount', 'on_the_house')
),
refunded_total AS (
SELECT COALESCE(SUM(r.amount), 0) AS refunded_pounds
backend/handlers/payments/sweep.go
+15 -1
View File
@@ -75,6 +75,15 @@ const stalePendingPaymentAge = 24 * time.Hour
// trustworthily and fall back to the legacy blind-fail + WARN.
const stalePendingKeyedAge = 22 * time.Hour
// SweepKeyedReplayAge returns the age at which a keyed pending row (stored
// idempotency key, no square_payment_id) becomes eligible for the sweep's
// replay-by-key reconcile. Exported for the webhooks package so the
// payment.completed orphan-replay detection can gate on whether a pending
// origin row could actually have been replayed by the sweep: a pending row
// younger than this age can never be a sweep-minted duplicate's origin, so it
// must not be marked failed by a webhook that raced the charge response.
func SweepKeyedReplayAge() time.Duration { return stalePendingKeyedAge }
// b1DuplicateRefundAttemptCap caps how many times the sweep may auto-refund a
// replay-induced duplicate charge (B1) under one stale pending row's expired
// idempotency key. A REJECTED refund writes NO refunds row, so the in-flight
@@ -729,7 +738,12 @@ func buildStaleRescueRecords(ctx context.Context, tx pgx.Tx, r staleRow, squareP
k := r.IdempotencyKey
primary.IdempotencyKey = &k
}
return buildSplitRecords(primary, paymentType, info, amount)
records, err := buildSplitRecords(primary, paymentType, info, amount)
if err != nil {
log.Printf("MEDIUM-3: buildSplitRecords rejected the rescue split for payment %s (booking %s): %v — the row is completed un-split; manual reconciliation recommended", r.ID, *r.BookingID, err)
return nil
}
return records
}
// applyStaleRescueRecords applies the pre-computed split records inside the
backend/handlers/payments/terminal_sca_test.go
-2
View File
@@ -171,7 +171,6 @@ func TestTerminalSavedCard_Tokenless_402(t *testing.T) {
PaymentMethod: strPtr("saved_card"),
UserSavedCardID: &cardID,
IdempotencyKey: "terminal-tokenless-" + bookingID,
VerificationCode: "334411",
}
w := makePaymentRequest(CreateTerminalPayment, "POST", "/api/admin/bookings/"+bookingID+"/payment", req, adminToken, ctx)
@@ -213,7 +212,6 @@ func TestTillSavedCard_Tokenless_402(t *testing.T) {
UserSavedCardID: &cardID,
UserID: &userID,
IdempotencyKey: key,
VerificationCode: "665544",
}
w := makePaymentRequest(CreateTillSale, "POST", "/api/admin/till/sale", req, adminToken, ctx)
backend/handlers/payments/till.go
+25 -48
View File
@@ -33,6 +33,13 @@ type TillSaleRequest struct {
PaymentMethod string `json:"payment_method" validate:"required"`
UserSavedCardID *string `json:"user_saved_card_id,omitempty"`
UserID *string `json:"user_id,omitempty"`
// NewCardToken is the SCA tokenize-result token (card.tokenize(
// verificationDetails, cardId)) for a saved-card till sale. When present it
// coexists with user_saved_card_id: the token is the one-time charge SOURCE
// and the saved-card row supplies the customer (mirrors the booking SCA
// tokenize-result wire contract). Without it the stored ccof: card id is
// the source (legacy saved-card charge).
NewCardToken *string `json:"new_card_token,omitempty"`
// 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
@@ -44,15 +51,6 @@ type TillSaleRequest struct {
CardToken string `json:"card_token,omitempty"`
RedeemToUserID *string `json:"redeem_to_user_id,omitempty"`
VerificationToken *string `json:"verification_token,omitempty"`
// VerificationCode is the card owner's current 2FA one-time code (B10): an
// enforced environment charges a saved card only when this matches the
// customer's pending code.
VerificationCode string `json:"verification_code,omitempty"`
// ConsentVersion / ConsentAccepted: the customer's explicit versioned
// consent to the SCA-unavailable → 2FA fallback (C6), enforced server-side
// (403 consent_required) and recorded on the 2fa_fallback_charge audit row.
ConsentVersion *string `json:"consent_version,omitempty"`
ConsentAccepted bool `json:"consent_accepted"`
}
type TillSaleResponse struct {
@@ -979,11 +977,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
// the post-charge 2FA consumption + audit after the Square call need it.
var cardUserID sql.NullString
// twoFAFallbackUsed records whether the 2FA BACKUP authorized a token-less
// saved-card charge (SCA unavailable). Hoisted: set in the saved_card case
// below, consumed by the strict fallback audit after the Square call.
twoFAFallbackUsed := false
// SCA verification token (if any) — hoisted so the 2FA gate in the
// saved_card case (a present token skips the gate: SCA-primary) and the
// CreatePaymentReq after the switch share one extraction.
@@ -1056,32 +1049,28 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
}
// 2FA gating (C5): charging a customer's saved card requires 2FA when
// the feature is enforced. consume=!reuse (LOW 6a): a FRESH charge
// verifies WITH consumption — the code is single-use at the gate,
// closing the TOCTOU where a verified-but-unconsumed code could
// authorize a second charge within its lifetime — and a failed Square
// charge re-issues a fresh code (reissueTwoFACodeAfterFailedCharge
// below). A pending-reuse retry (existingPendingID != "") verifies
// WITHOUT consuming: the code was re-issued for exactly this retry and
// the post-charge success path below (ConsumePendingCode) burns it on
// terminal success, so a retry that fails again keeps its code for one
// more attempt. A charge carrying a Square verification_token (SCA
// performed) skips the gate; a token-less charge falls back to 2FA and
// twoFAFallbackUsed is set for the charge-success audit.
if cardUserID.Valid {
var gateOK bool
gateOK, twoFAFallbackUsed = requireTwoFactorForCardAccess(w, r, service, cardUserID.String, req.VerificationCode, tillVerificationToken, existingPendingID == "")
if !gateOK {
return
}
// C6: a fallback-authorized charge must carry the customer's
// accepted consent (403 consent_required otherwise).
if !enforceSCAFallbackConsent(w, req.ConsentVersion, req.ConsentAccepted, twoFAFallbackUsed) {
// the feature is enforced. A charge carrying a Square verification_token
// (SCA performed) skips the gate; a token-less charge is refused 402
// verification_required (SCA-only — the homegrown 2FA fallback was
// removed). An SCA tokenize-result token (new_card_token) also skips the
// gate: the token only exists after the issuer completed buyer
// verification for this card + amount (SCA-primary), mirroring the
// booking path's scaTokenizedSavedCard skip.
scaTokenizedSavedCard := req.NewCardToken != nil && *req.NewCardToken != ""
if cardUserID.Valid && !scaTokenizedSavedCard {
if gateOK, _ := requireTwoFactorForCardAccess(w, r, service, cardUserID.String, tillVerificationToken, existingPendingID == ""); !gateOK {
return
}
}
// SCA tokenize-result wire contract (mirrors resolveChargeSource's
// saved-card branch): when the request carries new_card_token, the
// token is the one-time charge source and the saved-card row supplies
// the customer; otherwise the stored ccof: card id is the source.
tillSquareSourceID = savedCardSqCardID
if req.NewCardToken != nil && *req.NewCardToken != "" {
tillSquareSourceID = *req.NewCardToken
}
saleStatus = "pending"
needsSquarePayment = true
case "card_machine":
@@ -1255,7 +1244,7 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
paymentReq := square.CreatePaymentReq{
Amount: penceAmount,
Currency: "GBP",
SourceID: savedCardSqCardID,
SourceID: tillSquareSourceID,
CustomerID: savedCardCustomerID,
IdempotencyKey: req.IdempotencyKey,
Note: "Gift Card " + req.Action,
@@ -1357,13 +1346,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
}
}
}
// The gate consumed the 2FA code for a FRESH saved-card charge —
// re-issue so the same-key retry has a live code to verify. A
// pending-reuse retry verified without consuming at the gate, so
// its code survives for one more attempt.
if req.PaymentMethod == "saved_card" && cardUserID.Valid {
reissueTwoFACodeAfterFailedCharge(ctx, db.Conn, cardUserID.String, true, twoFAFallbackUsed && existingPendingID == "", r)
}
// 402 only for definitive declines; ambiguous transport/5xx must be
// 503 so the pending sale stays resumable on a same-key retry
// (M3). The clawback decision above stays keyed on
@@ -1424,11 +1406,6 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) {
"card_last4": paymentResult.CardLast4,
"square_payment_id": paymentResult.SquarePayID,
})
// The 2FA BACKUP authorized this token-less saved-card till charge
// (SCA was unavailable); the actor is the admin from context.
if twoFAFallbackUsed {
insertTwoFAFallbackAudit(ctx, adminID, cardUserID.String, paymentResult.CardLast4, tillSaleID, "admin till saved-card charge authorized via 2FA fallback (SCA unavailable)", consentVersionValue(req.ConsentVersion), req.ConsentAccepted)
}
// F1 confused-deputy audit: a saved-card till charge that proceeded
// WITHOUT the request naming the customer (user_id omitted) charged
// the card under its RESOLVED owner — a card never associated with
backend/handlers/payments/twofa.go
+20 -345
View File
@@ -1,20 +1,9 @@
package payments
import (
"context"
"crypto/rand"
"errors"
"fmt"
"log"
"math/big"
"net/http"
"os"
"strings"
"time"
"crussell/clock"
"crussell/db"
"crussell/internal/twofa"
)
// require2FADisabled reports whether REQUIRE_2FA explicitly disables 2FA
@@ -50,104 +39,19 @@ func (s *PaymentService) TwoFactorEnforced() bool {
return twoFactorEnforced()
}
// errTwoFAPepperRequired is returned when TWO_FACTOR_PEPPER is unset in a
// production-style re-issue gate. Refusing to re-issue is the only safe outcome:
// without the pepper the fresh code would be persisted as an unsalted SHA-256
// digest in the 1M code space, which a log/DB leak could brute-force offline
// (mirrors handlers/user's errTwoFAPepperRequired). Defined here (no build tag)
// so the pure gate (twoFAReissueIssueAllowedStrict) and the test,dev suite
// share it.
var errTwoFAPepperRequired = errors.New("TWO_FACTOR_PEPPER is not set; refusing to issue a 2FA code (an unsalted digest would be offline-brute-forceable)")
// errTwoFADeliveryUnavailable is returned when a production-style re-issue has
// no delivery channel: email/SMS unwired and the operator has not opted into
// the insecure log-delivery mode (TWO_FACTOR_ALLOW_LOG_DELIVERY=true). A fresh
// code the customer could never receive would strand them on the saved-card
// gate (mirrors handlers/user's errTwoFADeliveryUnavailable).
var errTwoFADeliveryUnavailable = errors.New("2FA requires an email or SMS delivery channel; contact the salon")
// twoFAPepperConfigured reports whether TWO_FACTOR_PEPPER is set — the pure,
// build-agnostic read behind the strict re-issue gate.
func twoFAPepperConfigured() bool { return os.Getenv("TWO_FACTOR_PEPPER") != "" }
// twoFADeliveryChannelConfigured reports whether the deployment has explicitly
// configured a 2FA code delivery channel: TWO_FACTOR_ALLOW_LOG_DELIVERY set to
// exactly "true" (the only production channel today — email/SMS unwired, P6).
// Pure env read, build-agnostic: the build-tagged twoFADeliveryAvailable
// (twofa_delivery_dev.go / twofa_delivery_prod.go) is the runtime-facing
// wrapper that turns this into the always-true dev channel or the prod env
// check.
func twoFADeliveryChannelConfigured() bool { return os.Getenv("TWO_FACTOR_ALLOW_LOG_DELIVERY") == "true" }
// twoFAReissueIssueAllowedStrict is the pure, build-agnostic production-style
// re-issue gate: a re-issued 2FA code may be minted ONLY when BOTH
// TWO_FACTOR_PEPPER is set (an unsalted digest in the 1M code space would be
// offline-brute-forceable) AND a delivery channel is configured (otherwise the
// fresh code could never reach the customer). Either way it fails closed. The
// build-tagged twoFAReissueIssueAllowed wraps it for production builds;
// dev/test builds always allow re-issue and never consult it — but the test,dev
// suite exercises THIS function directly, so the fail-closed branches are
// CI-visible even though the prod file (!dev && !test) is excluded there.
func twoFAReissueIssueAllowedStrict() error {
if !twoFAPepperConfigured() {
return errTwoFAPepperRequired
}
if !twoFADeliveryChannelConfigured() {
return errTwoFADeliveryUnavailable
}
return nil
}
// Single source of truth for 2FA verification: crussell/internal/twofa owns
// the code hashing (HMAC-SHA256 keyed by TWO_FACTOR_PEPPER, legacy SHA-256
// fallback), the constant-time compare, the code-lifetime check, and the
// per-user brute-force lockout. The user package's interactive endpoints
// (setup/verify/disable) and this file's re-issue path share it; nothing is
// re-implemented locally here. The saved-card charge gate no longer verifies
// 2FA codes at all (SCA-only — the 2FA fallback was removed), so the only
// local consumer left is reissueTwoFACodeAfterFailedCharge below.
// enforceSCAFallbackConsent is retained as a compile-compatible NO-OP so the
// saved-card charge handlers (handlers.go / till.go / giftcards.go) keep
// compiling unchanged. It used to enforce the C6 consent notice on the
// SCA-unavailable → 2FA fallback path, which is now REMOVED ENTIRELY: PSR 2017
// reg 100 makes Strong Customer Authentication mandatory and non-waivable for
// customer-initiated stored-credential charges, and the homegrown 2FA (a
// merchant-side check with no bank involvement) cannot legally act as an SCA
// fallback — authorising a token-less charge via 2FA leaves the MERCHANT liable
// for ECI 7 / SLI 210 chargebacks and PSR 2017 reg 77(6) compensation, and
// customer consent does not cure that. The gate now refuses a token-less
// saved-card charge 402 verification_required BEFORE any fallback can be used,
// so fallbackUsed is always false and there is no consent to demand. The
// callers pass it through as before; it always returns true.
func enforceSCAFallbackConsent(w http.ResponseWriter, consentVersion *string, consentAccepted bool, fallbackUsed bool) bool {
return true
}
// consentVersionValue normalizes the request's optional consent_version pointer
// to a string ("" when absent). Retained because the saved-card charge handlers
// still read the field when writing their (now unreachable) fallback audit row.
func consentVersionValue(version *string) string {
if version == nil {
return ""
}
return *version
}
// requireTwoFactorForCardAccess gates the saved-card online payment paths.
// It returns (allowed, fallbackUsed): allowed is true when the request may
// proceed; fallbackUsed is ALWAYS false — the homegrown 2FA fallback for
// token-less saved-card charges was REMOVED ENTIRELY, so no charge is ever
// authorized by a 2FA code and no fallback audit row is ever written (the
// callers' insertTwoFAFallbackAudit branches are unreachable).
// authorized by a 2FA code and no fallback audit row is ever written.
//
// It is a thin wrapper over requireTwoFactorForCardAccessWithTokenValidation
// that passes tokenForwardedToSquare=true — the legacy signature is retained
// because the saved-card CHARGE surfaces (booking, tip, gift-card buy, till)
// forward the verification_token to Square in CreatePaymentReq.VerificationToken,
// so a non-empty token there IS Square-validated SCA and legitimately skips the
// gate. The card-SAVE surfaces call the WithTokenValidation variant directly
// with false (see that helper for the auth-F1 rationale).
// that passes tokenForwardedToSquare=true — the saved-card CHARGE surfaces
// (booking, tip, gift-card buy, till, terminal) forward the verification_token
// to Square in CreatePaymentReq.VerificationToken, so a non-empty token there
// IS Square-validated SCA and legitimately skips the gate. The card-SAVE
// surfaces call the WithTokenValidation variant directly with false (see that
// helper for the auth-F1 rationale).
//
// The decision model, in order:
//
@@ -175,8 +79,8 @@ func consentVersionValue(version *string) string {
// On any denial an error JSON is written (parseable by the frontend via
// extractErrorMessage) and allowed=false is returned — the caller must abort
// the charge.
func requireTwoFactorForCardAccess(w http.ResponseWriter, r *http.Request, service *PaymentService, userID, verificationCode, verificationToken string, consume bool) (allowed, fallbackUsed bool) {
return requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, verificationCode, verificationToken, consume, true)
func requireTwoFactorForCardAccess(w http.ResponseWriter, r *http.Request, service *PaymentService, userID, verificationToken string, consume bool) (allowed, fallbackUsed bool) {
return requireTwoFactorForCardAccessWithTokenValidation(w, r, service, userID, verificationToken, consume, true)
}
// requireTwoFactorForCardAccessWithTokenValidation is the real gate:
@@ -192,11 +96,17 @@ func requireTwoFactorForCardAccess(w http.ResponseWriter, r *http.Request, servi
// skip the gate — with tokenForwardedToSquare=false the token is ignored and
// the token-less refusal below applies. An authenticated client can therefore
// no longer persist a card to the account with `verification_token: "anything"`
// and no SCA. The one legitimate SAVE skip is the call-site's scaTokenizedSavedCard
// flow (new_card_token + saved_card_id): there the SCA tokenize-result token IS
// the charge source and Square validates it as source_id, so the gate is skipped
// by the caller before this helper is ever reached.
func requireTwoFactorForCardAccessWithTokenValidation(w http.ResponseWriter, r *http.Request, service *PaymentService, userID, verificationCode, verificationToken string, consume bool, tokenForwardedToSquare bool) (allowed, fallbackUsed bool) {
// and no SCA. The legitimate SAVE skips are handled by the call site before
// this helper is ever reached: a genuine Square token-like card token is
// SCA-proven (the STORE-intent SCA performed at tokenization IS the
// verification — see CreatePaymentMethod's save gate) and the
// scaTokenizedSavedCard flow (new_card_token + saved_card_id) never persists a
// card.
//
// The verification_code parameter that used to flow through this gate is GONE:
// the gate never read it (SCA-only), the request structs no longer carry it,
// and there is no homegrown fallback to authorise anything with it.
func requireTwoFactorForCardAccessWithTokenValidation(w http.ResponseWriter, r *http.Request, service *PaymentService, userID, verificationToken string, consume bool, tokenForwardedToSquare bool) (allowed, fallbackUsed bool) {
if !twoFactorEnforced() {
return true, false
}
@@ -219,238 +129,3 @@ func requireTwoFactorForCardAccessWithTokenValidation(w http.ResponseWriter, r *
writeVerificationRequiredResponse(w)
return false, false
}
// reissueTwoFACodeAfterFailedCharge mints a fresh 2FA code after a saved-card
// charge failed at Square — but ONLY when a code was actually consumed by a
// FRESH saved-card charge (fresh-only semantics). The charge gate consumes the
// verified code at gate time for fresh charges (single-use — closing the
// verify-then-consume TOCTOU where a verified-but-unconsumed code could
// authorize a second charge), so a failed fresh charge leaves no live code for
// the same-key retry; this re-issues one with the same 10-minute lifetime and
// delivery behaviour as the user package's code issuance (dev/test logs the
// code for the operator to relay; production logs only with
// TWO_FACTOR_ALLOW_LOG_DELIVERY=true, matching the fail-closed delivery
// contract). It is a NO-OP for every other outcome: a new-card (cnon) charge
// never gates (usedSavedCard=false), a pending-reuse retry verified WITHOUT
// consuming (fallbackUsed=false — its code survives for one more attempt and a
// re-issue would silently invalidate the one the customer holds), and an
// SCA-authorized charge never touched the 2FA gate at all.
//
// Callers pass:
// - usedSavedCard: whether this charge actually used a saved card (the 2FA
// gate applies only to saved-card ccof charges);
// - fallbackUsed: whether the 2FA fallback gate actually consumed a code on
// THIS attempt (true only for a fresh charge — the gate's
// twoFAFallbackUsed ANDed with the caller's not-a-pending-reuse test).
//
// LOW-MEDIUM (finding 2): the re-issue is routed through the same fail-closed
// issuance gate as the interactive mint paths (twoFAReissueIssueAllowed —
// mirrored from the user package's twoFAEnsureIssueAllowed via the build-tagged
// twofa_delivery_dev.go / twofa_delivery_prod.go): a production build refuses
// to re-issue when TWO_FACTOR_PEPPER is unset (an unsalted digest in the 1M
// code space would be offline-brute-forceable) or when no delivery channel is
// configured. It also respects the same per-user mint cooldown
// (twoFAMintCooldown via the shared twofa.AttemptState.LastMintAt), so a
// charge-failure loop cannot mint codes faster than the mint endpoints allow.
// Best-effort: a failure logs a CRITICAL line + raises a critical-payment
// admin notification (the operator must mint a code manually or fix the
// config) and the customer requests a fresh code through the normal 2FA flow.
//
// PEPPER-CHANGE NOTE (Loop B finding 2): TWO_FACTOR_PEPPER is the ONLY hard
// gate on issuance here (twoFAReissueIssueAllowed) AND on the interactive mint
// paths (twoFAEnsureIssueAllowed in handlers/user). The pepper keys the
// HMAC-SHA256 of every stored pending-code hash, so CHANGING it invalidates
// ALL pending codes — every stored hash was computed with the old pepper and
// can never match a code minted under the new one. A fresh saved-card charge
// that consumed a pre-change code then fails will re-issue a code hashed with
// the NEW pepper, which still cannot match anything the customer holds (their
// code was minted under the old pepper, or was consumed). Operators MUST NOT
// change the pepper without re-minting every user's code (or having the
// customer re-run 2FA setup); main.go's startup check should treat a changed
// pepper as a config incident.
func reissueTwoFACodeAfterFailedCharge(ctx context.Context, q db.Querier, userID string, usedSavedCard, fallbackUsed bool, r *http.Request) {
if userID == "" || !twoFactorEnforced() || !usedSavedCard || !fallbackUsed {
return
}
if err := twoFAReissueIssueAllowed(); err != nil {
// Loop B HIGH (finding 2): a REFUSED re-issue strands the customer. The
// gate consumed their code for the fresh charge (single-use) and the
// charge failed — with no re-issued code the same-key retry fails
// forever with 400 ErrMissingOrExpired and the customer is locked out.
// This is an OPERATOR-FACING incident, not a silent best-effort miss:
// log a CRITICAL line AND raise the per-issue-capped critical-payment
// admin notification (sweep.go's insertCriticalPaymentNotification — the
// DB-backed stand-in for the un-watched CRITICAL logs) so the operator
// knows the customer is blocked and can mint a code manually or fix the
// config (TWO_FACTOR_PEPPER / delivery channel).
log.Printf("CRITICAL: failed to re-issue a 2FA code for user %s after a failed saved-card charge (%v) — the customer's code was consumed by the fresh charge and NO live code remains, so the same-key retry cannot succeed; the operator must mint a code manually or configure TWO_FACTOR_PEPPER and a 2FA delivery channel", userID, err)
// Round 2 Loop B findings 2 + 7 — the alert is capped PER-ISSUE, NOT
// globally (see alertReissueFail below): at most ONE unacknowledged row
// per stranded customer, so one customer's alert can never be
// suppressed by OTHER users' rows filling the 'critical_payment_log'
// bucket, and the count-then-insert is atomic (a single INSERT ... WHERE
// NOT EXISTS — no TOCTOU). An attacker also cannot FLOOD the alert:
// raising it requires a real saved-card charge that consumed a real code
// AND a failed re-issue, and the per-issue dedup holds each user to one
// row until acknowledged. Fail-closed behaviour is unchanged (the
// CRITICAL log always fires); only the notification INSERT is bounded.
alertReissueFail(ctx, q, userID)
return
}
// Mint cooldown (B11a + Round 2 Loop A finding 2): the shared per-user mutex
// serializes the stamp read/write with the user package's mints and the
// gate's verify critical section. A successful verify NO LONGER clears the
// stamp (internal/twofa.Check keeps it — it is cleared only at terminal
// charge success via twofa.ConsumePendingCode), so this check now genuinely
// bounds a charge-failure loop: the first re-issue after a mint is skipped
// while clock.Now().Sub(LastMintAt) < twoFAMintCooldown, bounding code churn
// and dev log flooding. The customer requests a fresh code through the
// normal mint endpoint once the window elapses.
st := twofa.StateFor(userID)
st.Mu.Lock()
defer st.Mu.Unlock()
if !st.LastMintAt.IsZero() && clock.Now().Sub(st.LastMintAt) < twoFAMintCooldown {
// Round 2 Loop B finding 6b: this skip was SILENT before. A FRESH
// charge consumed the customer's code at the gate (single-use) and the
// charge failed; the re-issue is now skipped by the per-user mint
// cooldown — the customer holds NO live code for the same-key retry
// until the cooldown lapses. That is a stranded customer, so raise the
// same per-issue-capped reissue-fail alert the refused-issue branch
// above uses (alertReissueFail, deduped on reason+user_id) so the
// operator knows to mint a code manually. The alert is per-issue
// (finding 2): repeated skips for the same customer stay ONE row until
// acknowledged and can never be suppressed by other users' rows.
log.Printf("2FA: re-issue skipped for user %s after a failed charge (mint cooldown) — the customer's code was consumed by the fresh charge and NO live code remains until the cooldown lapses", userID)
alertReissueFail(ctx, q, userID)
return
}
code, err := generatePaymentsTwoFACode()
if err != nil {
log.Printf("2FA: failed to generate a re-issued code for user %s after a failed charge: %v", userID, err)
return
}
if _, err := q.Exec(ctx, `
UPDATE users
SET two_factor_pending_code_hash = $2,
two_factor_pending_code_expires = $3
WHERE id = $1
`, userID, twofa.Hash(code), clock.Now().Add(twoFAPendingCodeLifetime)); err != nil {
log.Printf("2FA: failed to store a re-issued code for user %s after a failed charge: %v", userID, err)
return
}
st.SetLastMintAtLocked(clock.Now())
// Delivery is build-dependent (twofa_delivery_dev.go / twofa_delivery_prod.go),
// mirroring the user package's twoFADeliverCode: dev/test builds always write
// the [2FA] log line (the operator relays the code); production writes it ONLY
// when the operator explicitly opted into log delivery
// (TWO_FACTOR_ALLOW_LOG_DELIVERY=true) — otherwise the plaintext code is never
// logged.
twoFAReissueDeliverCode(userID, code)
}
// twoFAMintCooldown bounds how often the re-issue path mints a fresh 2FA code
// for one user after a failed saved-card charge, mirroring the user package's
// mint cooldown (handlers/user/twofa.go). The shared stamp lives on the
// per-user twofa.AttemptState.LastMintAt so both mint paths cohere.
//
// Round 2 Loop A finding 2: the stamp survives a successful gate verify
// (internal/twofa.Check no longer clears it) and is cleared only at TERMINAL
// charge success via twofa.ConsumePendingCode — so this check below is what
// actually bounds a charge-failure loop: after a fresh charge consumed a code
// at the gate and failed, the re-issue is skipped while the last mint is
// inside the cooldown (logged, not silent), bounding code churn + dev log
// flooding. Round 2 Loop B finding 6a — COORDINATION (money agent): the
// FRESH-charge terminal-success path in handlers.go does NOT call
// ConsumePendingCode (the gate already burned the code with consume=true), so
// its mint-cooldown stamp survives — a customer who completes a fresh charge
// within the cooldown of their last mint and immediately requests a new code
// gets 429 until the window elapses. To re-arm immediate re-minting after a
// completed fresh charge, the money agent should call
// twofa.ClearMintCooldownForUser(userID) on that terminal-success path (the
// exported, coordination-actionable entry point documented on
// internal/twofa.ClearMintCooldownForUser).
const twoFAMintCooldown = 1 * time.Minute
// alertReissueFail surfaces a stranded-customer incident in the admin
// notification centre (reason 'critical_payment_log'). Round 2 Loop B finding
// 2: it is capped PER-ISSUE, NOT globally — the NOT EXISTS guard keeps exactly
// ONE unacknowledged row per (reason, user_id) (the booking is unresolvable at
// re-issue time), so one customer's alert is never suppressed by other users'
// rows filling the 'critical_payment_log' bucket, and the count-then-insert is
// atomic (a single INSERT ... WHERE NOT EXISTS — no TOCTOU). It is a dedicated
// local insert rather than the sweep's insertCriticalPaymentNotification so
// that the money agent's upcoming GLOBAL cap on the sweep insert (finding 1)
// can never swallow this alert — a stranded customer must always surface.
// Best-effort: a failure logs and the caller's CRITICAL log line still fires.
func alertReissueFail(ctx context.Context, q db.Querier, userID string) {
tag, err := q.Exec(ctx, `
INSERT INTO admin_notifications (reason, user_id, created_at)
SELECT 'critical_payment_log', $1, NOW()
WHERE NOT EXISTS (
SELECT 1 FROM admin_notifications an
WHERE an.reason = 'critical_payment_log'
AND an.user_id = $1
AND an.acknowledged_at IS NULL
)
`, userID)
if err != nil {
log.Printf("2FA: failed to insert critical-payment admin notification for reissue failure (user=%s): %v", userID, err)
return
}
if tag.RowsAffected() > 0 {
log.Printf("2FA: inserted critical-payment admin notification for reissue failure (user=%s) — customer stranded after a failed fresh saved-card charge", userID)
}
}
// generatePaymentsTwoFACode returns a random 6-digit verification code,
// mirroring the user package's generator (crypto/rand, uniform 0-999999).
func generatePaymentsTwoFACode() (string, error) {
n, err := rand.Int(rand.Reader, big.NewInt(1_000_000))
if err != nil {
return "", err
}
return fmt.Sprintf("%06d", n.Int64()), nil
}
// twoFAPendingCodeLifetime is how long a re-issued 2FA code stays valid,
// mirroring the user package's pending-code expiry.
const twoFAPendingCodeLifetime = 10 * time.Minute
// COORDINATION NOTE (Round 2 Loop B finding 1) — the shared notification flood
// cap now lives in crussell/internal/adminnotify
// (MaxUnacknowledgedCriticalLogs = 100 + CriticalLogsCapExceeded), applied
// ATOMICALLY (a conditional `INSERT ... SELECT ... WHERE (SELECT COUNT(*) ...)
// < $cap`) at every insert site this finding calls out. Status of each site:
//
// - THIS package's reissue-fail alert (reissueTwoFACodeAfterFailedCharge):
// capped PER-ISSUE instead (Round 2 Loop B finding 2) — the local
// alertReissueFail helper dedups atomically on (reason, user_id), so one
// customer's alert is never suppressed by other users' rows. NOT globally
// capped, and deliberately decoupled from sweep's generic insert so the
// money agent's global cap there can never swallow it.
//
// - auth/jwt.go VerifyRefreshToken's 'refresh_token_reuse' alert: NOW capped
// (Round 2 Loop B finding 1) — folded into its INSERT via the shared
// adminnotify cap.
//
// - handlers/webhooks/square.go (dispute/booking/unknown-event/orphan-replay)
// and handlers/user/account.go InsertSquareErasureCriticalNotification:
// NOW capped — same atomic fold.
//
// - handlers/payments/sweep.go:1556 insertCriticalPaymentNotification (the
// MONEY agent): STILL NEEDS the fold. Its INSERT ... SELECT ... WHERE NOT
// EXISTS is the same unbounded-across-accounts shape. Add
// `AND (SELECT COUNT(*) FROM admin_notifications _an WHERE _an.reason =
// 'critical_payment_log' AND _an.acknowledged_at IS NULL) < $N` (N =
// adminnotify.MaxUnacknowledgedCriticalLogs) to its WHERE clause.
//
// - handlers/scheduling/time-blockers.go:511 and internal/jobs/cleanup.go:333
// (jobs/scheduling agent): same shape on the GDPR-cleanup and log-scan
// paths — apply the identical fold.
//
// - main.go has NO admin_notifications insert sites (it only mounts the
// notification read/ack routes), so nothing to cap there.
//
// Fail-closed behaviour (a REFUSED re-issue still CRITICAL-logs and leaves the
// operator to mint manually) is unchanged — only the notification INSERT is
// bounded.
backend/handlers/payments/twofa_delivery_dev.go
-35
View File
@@ -1,35 +0,0 @@
//go:build dev || test
package payments
import "log"
// twoFADeliveryAvailable reports whether a 2FA code delivery channel exists in
// this build. Dev/test builds always have one — the [2FA] log line is the
// documented loose-fake delivery channel — so the 2FA BACKUP authorization
// (the saved-card gate when SCA is unavailable) is always usable here. Mirrors
// handlers/user/twofa_dev.go; production builds decide in
// twofa_delivery_prod.go.
func twoFADeliveryAvailable() bool { return true }
// twoFAReissueIssueAllowed is the re-issue path's issuance gate
// (reissueTwoFACodeAfterFailedCharge, handlers.go), mirroring the user
// package's twoFAEnsureIssueAllowed build-tagged semantics: dev/test builds
// always allow issuance — the [2FA] log line is the delivery channel and the
// unsalted-digest fallback is the documented loose-fake stand-in (matching
// twofa_dev.go). Production builds fail closed here — no pepper, no delivery
// channel, no codes (see twofa_delivery_prod.go).
func twoFAReissueIssueAllowed() error { return nil }
// twoFAReissueDeliverCode delivers a re-issued code (a fresh saved-card charge
// consumed the customer's code at the gate and the charge failed at Square).
// Dev/test builds always deliver via the [2FA] log line — the documented
// loose-fake delivery channel — so the operator can relay the fresh code to the
// customer. Mirrors the user package's twoFADeliverCode; production logs it
// ONLY with the explicit TWO_FACTOR_ALLOW_LOG_DELIVERY opt-in (see
// twofa_delivery_prod.go). MEDIUM-3b: the user id and the plaintext code go to
// SEPARATE log lines so a single record cannot trivially pair them.
func twoFAReissueDeliverCode(userID, code string) {
log.Printf("[2FA] code delivery requested (user=%s, purpose=re-issue after failed saved-card charge)", userID)
log.Printf("[2FA] code: %s", code)
}
backend/handlers/payments/twofa_delivery_prod.go
-63
View File
@@ -1,63 +0,0 @@
//go:build !dev && !test
package payments
import (
"log"
"os"
)
// twoFADeliveryAvailable reports whether a 2FA code delivery channel exists in
// this build. Production has no wired email/SMS transport (P6), so the ONLY
// channel is the operator's explicit opt-in to insecure log delivery
// (TWO_FACTOR_ALLOW_LOG_DELIVERY=true). Without a channel, codes can never
// reach the customer, so the 2FA BACKUP authorization (the saved-card gate
// when SCA is unavailable) cannot operate and a token-less saved-card charge is
// denied 503 (see requireTwoFactorForCardAccess). Delegates to the pure
// build-agnostic twoFADeliveryChannelConfigured (twofa.go); dev/test builds
// always deliver (twofa_delivery_dev.go).
func twoFADeliveryAvailable() bool {
return twoFADeliveryChannelConfigured()
}
// twoFAReissueIssueAllowed is the re-issue path's issuance gate
// (reissueTwoFACodeAfterFailedCharge, handlers.go), mirroring the user
// package's twoFAEnsureIssueAllowed (handlers/user/twofa_prod.go) build-tagged
// semantics: production requires BOTH a delivery channel and TWO_FACTOR_PEPPER.
// Without a channel the code could never reach the customer, and without the
// pepper every stored code would be an offline-brute-forceable unsalted digest
// — either way the re-issue refuses (fail-closed), exactly like the interactive
// mint paths. Delegates to the pure build-agnostic gate
// twoFAReissueIssueAllowedStrict (twofa.go), which the test,dev suite also
// exercises directly; dev/test builds always allow issuance
// (twofa_delivery_dev.go).
//
// The pepper check is the ONLY hard gate on the re-issue (plus the delivery
// channel). PEPPER-CHANGE HAZARD (Loop B finding 2): the pepper keys the
// HMAC-SHA256 of every stored pending-code hash, so CHANGING TWO_FACTOR_PEPPER
// invalidates ALL pending codes — a re-issued code under the new pepper can
// never match a customer's code minted under the old one. An operator who
// changes the pepper must re-mint every user's code (or the customer must
// re-run 2FA setup), or a fresh saved-card charge whose code was consumed at
// the gate will strand the customer with 400 ErrMissingOrExpired on retry.
func twoFAReissueIssueAllowed() error {
return twoFAReissueIssueAllowedStrict()
}
// twoFAReissueDeliverCode delivers a re-issued code after a failed fresh
// saved-card charge (the gate consumed the customer's code at verify time).
// Production's ONLY channel is the operator's explicit, insecure opt-in to log
// delivery (TWO_FACTOR_ALLOW_LOG_DELIVERY=true — anyone with backend log access
// could defeat the 2FA gate on saved-card charges). WITHOUT that flag the
// plaintext code is NEVER written to the log (issuance was already refused by
// twoFAReissueIssueAllowed, so this no-op is unreachable); with it, the code
// goes to the [2FA] log line for the operator to relay to the customer, exactly
// like the documented dev flow. MEDIUM-3b: the user id and the plaintext code
// go to SEPARATE log lines so a single record cannot trivially pair them.
func twoFAReissueDeliverCode(userID, code string) {
if os.Getenv("TWO_FACTOR_ALLOW_LOG_DELIVERY") == "true" {
log.Printf("[2FA] code delivery requested (user=%s, purpose=re-issue after failed saved-card charge)", userID)
log.Printf("[2FA] code: %s", code)
}
// Otherwise: deliberate no-op — never log the plaintext code by default.
}
backend/handlers/payments/twofa_delivery_prod_test.go
-84
View File
@@ -1,84 +0,0 @@
//go:build !dev
package payments
// Tests for the PRODUCTION 2FA delivery predicate (twofa_delivery_prod.go).
//
// LIMITATION (documented): the 503 "2FA requires an email or SMS delivery
// channel" branch in requireTwoFactorForCardAccess (twofa.go:193) is only
// reachable when twoFADeliveryAvailable() returns false, which happens ONLY in
// a production build (!dev && !test). Under BOTH required test runs — the
// "test,dev" run and the "test,!dev" prod-shape run — the dev/test delivery
// variant (twofa_delivery_dev.go, build tag `dev || test`) is the compiled
// function and is trivially true, so the 503 branch cannot be exercised there.
// The two test invocations DO however compile this file, and the prod-variant
// marker (twofaDeliveryProdVariant) tells the test which delivery function is
// live: a genuine production build (no dev/test tags, e.g. `go test ./...`)
// compiles twofa_delivery_prod.go, and this test then asserts the real prod
// predicate end to end.
import (
"os"
"testing"
"github.com/stretchr/testify/require"
)
// TestTwoFADeliveryAvailable_ProdPredicate asserts the production gating that
// twofa_delivery_prod.go implements: TWO_FACTOR_ALLOW_LOG_DELIVERY unset →
// no channel (false), exactly "true" → channel (true), any other value →
// no channel. In a dev/test build the marker is false and the test skips,
// because the always-true dev variant is compiled and the 503 branch is
// unreachable (documented limitation — see the file header).
func TestTwoFADeliveryAvailable_ProdPredicate(t *testing.T) {
if !twofaDeliveryProdVariant {
t.Skip("twoFADeliveryAvailable() is the dev/test build's trivially-true variant (twofa_delivery_dev.go, `dev || test`); the 503 delivery-unavailable branch is unreachable under the test tag — see the file header for the documented limitation")
}
t.Run("unset_env_is_no_channel", func(t *testing.T) {
os.Unsetenv("TWO_FACTOR_ALLOW_LOG_DELIVERY")
require.False(t, twoFADeliveryAvailable(), "production without the explicit opt-in must have NO 2FA delivery channel")
})
t.Run("empty_env_is_no_channel", func(t *testing.T) {
os.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", "")
require.False(t, twoFADeliveryAvailable())
})
t.Run("exact_true_is_a_channel", func(t *testing.T) {
os.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", "true")
require.True(t, twoFADeliveryAvailable(), "the explicit insecure log-delivery opt-in must open the channel")
})
t.Run("any_other_value_is_no_channel", func(t *testing.T) {
for _, v := range []string{"1", "yes", "on", "True", "TRUE", "false"} {
os.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", v)
require.False(t, twoFADeliveryAvailable(), "value %q must NOT open the delivery channel (exact 'true' only)", v)
}
})
}
// TestTwoFAReissueIssueAllowed_ProdPredicate pins the finding 2 re-issue
// issuance gate in a genuine production build (no dev/test tags): it fails
// closed without TWO_FACTOR_PEPPER (an unsalted digest would be
// offline-brute-forceable) or without a delivery channel, and allows issuance
// only when both are configured. In a dev/test build the marker is false and
// the test skips, because the always-allowed dev variant is compiled
// (twofa_delivery_dev.go) — same documented limitation as the delivery
// predicate above.
func TestTwoFAReissueIssueAllowed_ProdPredicate(t *testing.T) {
if !twofaDeliveryProdVariant {
t.Skip("twoFAReissueIssueAllowed() is the dev/test build's always-allowed variant (twofa_delivery_dev.go, `dev || test`); the prod fail-closed branches are unreachable under the test tag — see the file header for the documented limitation")
}
os.Unsetenv("TWO_FACTOR_PEPPER")
os.Unsetenv("TWO_FACTOR_ALLOW_LOG_DELIVERY")
require.Error(t, twoFAReissueIssueAllowed(), "a production re-issue without the pepper must fail closed")
os.Setenv("TWO_FACTOR_PEPPER", "test-pepper")
os.Unsetenv("TWO_FACTOR_ALLOW_LOG_DELIVERY")
require.Error(t, twoFAReissueIssueAllowed(), "a production re-issue without a delivery channel must fail closed")
os.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", "true")
require.NoError(t, twoFAReissueIssueAllowed(), "a production re-issue with both the pepper and a delivery channel is allowed")
}
backend/handlers/payments/twofa_delivery_variant_dev_test.go
-11
View File
@@ -1,11 +0,0 @@
//go:build dev || test
package payments
// twofaDeliveryProdVariant reports whether the PRODUCTION delivery predicate
// (twofa_delivery_prod.go, !dev && !test) is the compiled function in this
// build. Under `dev` OR `test` tags the dev/test delivery variant
// (twofa_delivery_dev.go) is compiled instead — always true, so the 503
// delivery-unavailable branch is unreachable there.
//lint:ignore U1000 referenced only from the prod-tag test (twofa_delivery_prod_test.go, !dev && !test); deliberately unused under dev/test tags
const twofaDeliveryProdVariant = false
backend/handlers/payments/twofa_delivery_variant_prod_test.go
-9
View File
@@ -1,9 +0,0 @@
//go:build !dev && !test
package payments
// twofaDeliveryProdVariant reports whether the PRODUCTION delivery predicate
// (twofa_delivery_prod.go, !dev && !test) is the compiled function in this
// build. True only in a genuine production build — neither dev nor test tag —
// where twoFADeliveryAvailable() gates on TWO_FACTOR_ALLOW_LOG_DELIVERY.
const twofaDeliveryProdVariant = true
backend/handlers/payments/twofa_failclosed_test.go
-60
View File
@@ -1,60 +0,0 @@
//go:build test
package payments
// M17 follow-up (ITEM 2): twofa_delivery_prod.go is excluded from the test,dev
// suite (`!dev && !test`), so its fail-closed re-issue branches were never
// exercised in CI — the old twofa_delivery_prod_test.go only runs its
// assertions in a genuine production build and skips under the test tag. The
// pure decision logic now lives build-agnostically in twofa.go
// (twoFAReissueIssueAllowedStrict / twoFAPepperConfigured /
// twoFADeliveryChannelConfigured); these tests exercise those branches in the
// STANDARD test,dev run, so a regression in the prod fail-closed behaviour is
// CI-visible even though the prod file itself is only compiled in a genuine
// production build.
import (
"testing"
"github.com/stretchr/testify/require"
)
// TestTwoFAReissueIssueAllowedStrict_FailClosed pins the production-style
// re-issue gate that twofa_delivery_prod.go's twoFAReissueIssueAllowed
// delegates to (reissueTwoFACodeAfterFailedCharge): (a) pepper unset →
// re-issue refused (errTwoFAPepperRequired — an unsalted digest in the 1M code
// space would be offline-brute-forceable); (b) delivery channel absent →
// re-issue refused (errTwoFADeliveryUnavailable — the 503-style error);
// (c) both configured → re-issue succeeds.
func TestTwoFAReissueIssueAllowedStrict_FailClosed(t *testing.T) {
t.Run("pepper_unset_refuses_reissue", func(t *testing.T) {
t.Setenv("TWO_FACTOR_PEPPER", "")
t.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", "true")
require.ErrorIs(t, twoFAReissueIssueAllowedStrict(), errTwoFAPepperRequired)
})
t.Run("delivery_channel_absent_refuses_reissue", func(t *testing.T) {
t.Setenv("TWO_FACTOR_PEPPER", "test-pepper")
t.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", "")
require.ErrorIs(t, twoFAReissueIssueAllowedStrict(), errTwoFADeliveryUnavailable)
})
t.Run("pepper_and_channel_present_allows_reissue", func(t *testing.T) {
t.Setenv("TWO_FACTOR_PEPPER", "test-pepper")
t.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", "true")
require.NoError(t, twoFAReissueIssueAllowedStrict())
})
}
// TestPaymentsTwoFADeliveryChannelConfigured pins the pure delivery-channel
// predicate behind the payments 503 refusal: only the exact value "true" opens
// the channel.
func TestPaymentsTwoFADeliveryChannelConfigured(t *testing.T) {
t.Setenv("TWO_FACTOR_PEPPER", "test-pepper")
for _, v := range []string{"", "1", "yes", "on", "True", "TRUE", "false"} {
t.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", v)
require.False(t, twoFADeliveryChannelConfigured(), "value %q must NOT open the delivery channel (exact 'true' only)", v)
}
t.Setenv("TWO_FACTOR_ALLOW_LOG_DELIVERY", "true")
require.True(t, twoFADeliveryChannelConfigured())
}
backend/handlers/payments/twofa_gate_consume_test.go
-52
View File
@@ -25,9 +25,7 @@ import (
"encoding/json"
"net/http"
"testing"
"time"
"crussell/db"
"crussell/internal/square"
"crussell/internal/twofa"
"crussell/testutils"
@@ -69,7 +67,6 @@ func TestTwoFactorEnforced_CreateTillSale_SavedCard_Tokenless_RefusedWithoutCons
UserSavedCardID: &cardID,
UserID: &userID,
IdempotencyKey: "2fa-till-fresh-decline",
VerificationCode: "556677",
}
w := makePaymentRequest(CreateTillSale, "POST", "/api/admin/till/sale", req, adminToken, ctx)
@@ -133,7 +130,6 @@ func TestTwoFactorEnforced_CreateTillSale_SavedCard_PendingReuse_Tokenless_Refus
UserSavedCardID: &cardID,
UserID: &userID,
IdempotencyKey: key,
VerificationCode: "667788",
}
w := makePaymentRequest(CreateTillSale, "POST", "/api/admin/till/sale", req, adminToken, ctx)
@@ -171,7 +167,6 @@ func TestTwoFactorEnforced_BuyGiftCard_SavedCard_Tokenless_RefusedWithoutConsumi
RecipientType: "self",
CardID: &cardID,
IdempotencyKey: "2fa-buy-gc-fresh-decline",
VerificationCode: "112233",
}
w := makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", req, token, ctx)
@@ -221,7 +216,6 @@ func TestTwoFactorEnforced_BuyGiftCard_SavedCard_PendingReuse_Tokenless_Refused(
RecipientType: "self",
CardID: &cardID,
IdempotencyKey: key,
VerificationCode: "334455",
}
w := makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", req, token, ctx)
@@ -258,7 +252,6 @@ func TestTwoFactorEnforced_CreateBookingPayment_NewCardSaveCard_Tokenless_Refuse
NewCardToken: &cardToken,
SaveCard: true,
IdempotencyKey: "2fa-booking-newcard-savecard-decline",
VerificationCode: "999001",
}
w := makePaymentRequest(withNonGuest(CreateBookingPayment), "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
@@ -296,7 +289,6 @@ func TestTwoFactorEnforced_CreateTipPayment_NewCardSaveCard_Tokenless_Refused(t
NewCardToken: &cardToken,
SaveCard: true,
IdempotencyKey: "2fa-tip-newcard-savecard-decline",
VerificationCode: "999002",
}
w := makePaymentRequest(withNonGuest(CreateTipPayment), "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, ctx)
@@ -307,47 +299,3 @@ func TestTwoFactorEnforced_CreateTipPayment_NewCardSaveCard_Tokenless_Refused(t
require.True(t, hash.Valid, "the pending 2FA code must survive a refused token-less charge")
require.Equal(t, twofa.Hash("999002"), hash.String, "the save gate must not consume the code — it was never consulted")
}
// TestReissueTwoFACodeAfterFailedCharge_RespectsMintCooldown pins the
// LOW-MEDIUM finding 2 contract on the retained re-issue helper: the re-issue
// mints a live code after a FRESH charge consumed one at the gate (in the
// pre-removal world), 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 helper is retained because handlers.go / till.go / giftcards.go
// still call it with their (now always-false) fallbackUsed flag.
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)")
// Round 2 Loop B finding 6b: the cooldown-skipped re-issue must NOT be
// silent — the fresh charge consumed the customer's code at the gate, so
// they have NO live code for the same-key retry until the cooldown lapses.
// The per-issue-capped reissue-fail alert raises so the operator knows the
// customer is stranded (deduped on reason+user_id: one row per customer).
var alertCount int
require.NoError(t, tx.QueryRow(ctx, `SELECT COUNT(*) FROM admin_notifications WHERE reason = 'critical_payment_log' AND user_id = $1 AND acknowledged_at IS NULL`, userID).Scan(&alertCount))
require.Equal(t, 1, alertCount, "a cooldown-skipped re-issue after a fresh consumed charge must raise the reissue-fail alert (finding 6b)")
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")
}
backend/handlers/payments/twofa_test.go
+19 -41
View File
@@ -119,7 +119,7 @@ func TestRequireTwoFactorForCardAccess_VerificationTokenSkips(t *testing.T) {
require.NoError(t, err)
req := httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx)
w := httptest.NewRecorder()
allowed, fallbackUsed := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "", "vrf_sca_token_123", false)
allowed, fallbackUsed := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "vrf_sca_token_123", false)
require.True(t, allowed, "SCA performed — the gate must be skipped")
require.False(t, fallbackUsed, "SCA is primary — no fallback is ever used")
require.Equal(t, http.StatusOK, w.Code, "no denial response may be written when the token skips the gate")
@@ -144,7 +144,7 @@ func TestRequireTwoFactorForCardAccess_TokenForwardedDistinction(t *testing.T) {
_, err = tx.Exec(ctx, "UPDATE users SET two_factor_enabled = true WHERE id = $1", userID)
require.NoError(t, err)
w := httptest.NewRecorder()
allowed, fallbackUsed := requireTwoFactorForCardAccessWithTokenValidation(w, req, NewPaymentService(), userID, "", "forged-token", true, false)
allowed, fallbackUsed := requireTwoFactorForCardAccessWithTokenValidation(w, req, NewPaymentService(), userID, "forged-token", true, false)
require.False(t, allowed, "a forged non-empty token must not skip the gate on the save path")
require.False(t, fallbackUsed)
require.Equal(t, http.StatusPaymentRequired, w.Code)
@@ -158,7 +158,7 @@ func TestRequireTwoFactorForCardAccess_TokenForwardedDistinction(t *testing.T) {
require.NoError(t, err)
seedTwoFAPendingCode(t, tx, userID, "424242")
w := httptest.NewRecorder()
allowed, fallbackUsed := requireTwoFactorForCardAccessWithTokenValidation(w, req, NewPaymentService(), userID, "424242", "forged-token", true, false)
allowed, fallbackUsed := requireTwoFactorForCardAccessWithTokenValidation(w, req, NewPaymentService(), userID, "forged-token", true, false)
require.False(t, allowed, "a valid 2FA code cannot authorise a save (SCA-only — the 2FA fallback was removed)")
require.False(t, fallbackUsed, "the 2FA fallback was removed — fallbackUsed is always false")
require.Equal(t, http.StatusPaymentRequired, w.Code)
@@ -171,7 +171,7 @@ func TestRequireTwoFactorForCardAccess_TokenForwardedDistinction(t *testing.T) {
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
w := httptest.NewRecorder()
allowed, _ := requireTwoFactorForCardAccessWithTokenValidation(w, req, NewPaymentService(), userID, "424242", "forged-token", true, false)
allowed, _ := requireTwoFactorForCardAccessWithTokenValidation(w, req, NewPaymentService(), userID, "forged-token", true, false)
require.False(t, allowed, "a user without 2FA setup cannot save a card even with a token")
require.Equal(t, http.StatusPaymentRequired, w.Code)
var body map[string]string
@@ -183,7 +183,7 @@ func TestRequireTwoFactorForCardAccess_TokenForwardedDistinction(t *testing.T) {
userID, err := fixtures.CreateTestUser(tx)
require.NoError(t, err)
w := httptest.NewRecorder()
allowed, fallbackUsed := requireTwoFactorForCardAccessWithTokenValidation(w, req, NewPaymentService(), userID, "", "vrf_sca_token_123", false, true)
allowed, fallbackUsed := requireTwoFactorForCardAccessWithTokenValidation(w, req, NewPaymentService(), userID, "vrf_sca_token_123", false, true)
require.True(t, allowed, "a token-forwarded charge path still skips on a non-empty token (SCA)")
require.False(t, fallbackUsed)
require.Equal(t, http.StatusOK, w.Code)
@@ -247,7 +247,6 @@ func TestTwoFactorEnforced_CreateBookingPayment_SaveCard_ForgeToken_With2FA_Bloc
NewCardToken: &cardToken,
SaveCard: true,
IdempotencyKey: "2fa-save-card-forge-token-ok",
VerificationCode: "112233",
VerificationToken: &forged,
}
@@ -318,7 +317,7 @@ func TestRequireTwoFactorForCardAccess_Tokenless_402Structured(t *testing.T) {
seedTwoFAPendingCode(t, tx, userID, "123456")
req := httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx)
w := httptest.NewRecorder()
allowed, fallbackUsed := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "123456", "", false)
allowed, fallbackUsed := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "", false)
require.False(t, allowed, "SCA-only: no 2FA fallback for a token-less charge")
require.False(t, fallbackUsed, "fallbackUsed must be false — the 2FA fallback was removed")
require.Equal(t, http.StatusPaymentRequired, w.Code)
@@ -348,7 +347,6 @@ func TestTwoFactorEnforced_BookingSavedCard_Tokenless_402(t *testing.T) {
PaymentType: "full",
CardID: &cardID,
IdempotencyKey: "2fa-fallback-audit",
VerificationCode: "445566",
}
w := makePaymentRequest(withNonGuest(CreateBookingPayment), "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
@@ -415,7 +413,6 @@ func TestTwoFactorEnforced_TipSavedCard_Tokenless_402(t *testing.T) {
NewCardToken: &cardToken,
SaveCard: true,
IdempotencyKey: "2fa-tip-save-audit",
VerificationCode: "556600",
}
w := makePaymentRequest(withNonGuest(CreateTipPayment), "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, ctx)
@@ -445,7 +442,6 @@ func TestTwoFactorEnforced_TipChargeSavedCard_Tokenless_402(t *testing.T) {
Amount: 500,
CardID: &cardID,
IdempotencyKey: "2fa-tip-charge-audit",
VerificationCode: "112211",
}
w := makePaymentRequest(withNonGuest(CreateTipPayment), "POST", "/api/bookings/"+bookingID+"/tip", req, userToken, ctx)
@@ -475,7 +471,6 @@ func TestTwoFactorEnforced_BuyGiftCard_SavedCard_Tokenless_402(t *testing.T) {
RecipientType: "self",
CardID: &cardID,
IdempotencyKey: key,
VerificationCode: "778811",
}
w := makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", req, token, ctx)
@@ -490,9 +485,10 @@ func TestTwoFactorEnforced_BuyGiftCard_SavedCard_Tokenless_402(t *testing.T) {
}
// TestTwoFactorEnforced_PaymentMethodSave_Tokenless_402 pins the SCA-only
// posture on the add-card save gate (handlers.go CreatePaymentMethod): the
// dedicated add-card endpoint carries no verification_token field, so a save is
// refused 402 verification_required even with a valid 2FA code (SCA-only).
// posture on the add-card save gate (handlers.go CreatePaymentMethod): a save
// from a NON-token-like source (a raw PAN — the gate's only remaining refusal
// shape) is refused 402 verification_required even with a valid 2FA code
// (SCA-only). A genuine token-like source is SCA-proven and skips the gate.
func TestTwoFactorEnforced_PaymentMethodSave_Tokenless_402(t *testing.T) {
helperEnvEnforce2FA(t)
ctx, tx := testutils.SetupTestTx(t)
@@ -503,8 +499,7 @@ func TestTwoFactorEnforced_PaymentMethodSave_Tokenless_402(t *testing.T) {
seedTwoFAPendingCode(t, tx, userID, "998877")
req := CreatePaymentMethodRequest{
CardToken: "cnon:2fa-pm-save-audit",
VerificationCode: "998877",
CardToken: "4111111111111111",
}
w := makePaymentRequest(CreatePaymentMethod, "POST", "/api/user/payment-methods", req, token, ctx)
@@ -527,7 +522,7 @@ func TestRequireTwoFactorForCardAccess_NotEnforced(t *testing.T) {
t.Setenv("SQUARE_ENVIRONMENT", "mock")
req := httptest.NewRequest(http.MethodPost, "/", nil)
w := httptest.NewRecorder()
allowed, fallbackUsed := requireTwoFactorForCardAccess(w, req, nil, "000000000001", "", "", false)
allowed, fallbackUsed := requireTwoFactorForCardAccess(w, req, nil, "000000000001", "", false)
require.True(t, allowed, "no response must be written when not enforced")
require.False(t, fallbackUsed, "not enforced — no fallback is ever used")
require.Equal(t, http.StatusOK, w.Code)
@@ -546,7 +541,7 @@ func TestRequireTwoFactorForCardAccess_Enforced(t *testing.T) {
require.NoError(t, err)
req := httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx)
w := httptest.NewRecorder()
ok, _ := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "123456", "", false)
ok, _ := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "", false)
require.False(t, ok)
require.Equal(t, http.StatusPaymentRequired, w.Code)
var body map[string]string
@@ -561,7 +556,7 @@ func TestRequireTwoFactorForCardAccess_Enforced(t *testing.T) {
require.NoError(t, err)
req := httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx)
w := httptest.NewRecorder()
ok, _ := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "", "", false)
ok, _ := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "", false)
require.False(t, ok)
require.Equal(t, http.StatusPaymentRequired, w.Code)
})
@@ -572,7 +567,7 @@ func TestRequireTwoFactorForCardAccess_Enforced(t *testing.T) {
seedTwoFAPendingCode(t, tx, userID, "424242")
req := httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx)
w := httptest.NewRecorder()
allowed, fallbackUsed := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "424242", "", false)
allowed, fallbackUsed := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "", false)
require.False(t, allowed, "a valid 2FA code cannot authorise a token-less charge (SCA-only)")
require.False(t, fallbackUsed, "fallbackUsed must be false — the 2FA fallback was removed")
require.Equal(t, http.StatusPaymentRequired, w.Code)
@@ -584,7 +579,7 @@ func TestRequireTwoFactorForCardAccess_Enforced(t *testing.T) {
seedTwoFAPendingCode(t, tx, userID, "424242")
req := httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx)
w := httptest.NewRecorder()
ok, _ := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "000000", "", false)
ok, _ := requireTwoFactorForCardAccess(w, req, NewPaymentService(), userID, "", false)
require.False(t, ok, "the gate does not verify codes anymore — any token-less charge is refused 402")
require.Equal(t, http.StatusPaymentRequired, w.Code)
})
@@ -592,7 +587,7 @@ func TestRequireTwoFactorForCardAccess_Enforced(t *testing.T) {
t.Run("unknown_user_writes_402_json", func(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx)
w := httptest.NewRecorder()
ok, _ := requireTwoFactorForCardAccess(w, req, NewPaymentService(), "000000000000", "123456", "", false)
ok, _ := requireTwoFactorForCardAccess(w, req, NewPaymentService(), "000000000000", "", false)
require.False(t, ok)
require.Equal(t, http.StatusPaymentRequired, w.Code)
var body map[string]string
@@ -682,7 +677,6 @@ func TestTwoFactorEnforced_CreateBookingPayment_SaveCard_With2FA_Blocked(t *test
NewCardToken: &cardToken,
SaveCard: true,
IdempotencyKey: "2fa-save-card-ok",
VerificationCode: "112233",
}
w := makePaymentRequest(withNonGuest(CreateBookingPayment), "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
@@ -714,7 +708,6 @@ func TestTwoFactorEnforced_CreateBookingPayment_SavedCard_With2FA_Blocked(t *tes
PaymentType: "full",
CardID: &cardID,
IdempotencyKey: "2fa-saved-card-ok",
VerificationCode: "334455",
}
w := makePaymentRequest(withNonGuest(CreateBookingPayment), "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
@@ -819,7 +812,6 @@ func TestTwoFactorEnforced_CreateTillSale_SavedCard_With2FA_Blocked(t *testing.T
UserSavedCardID: &cardID,
UserID: &userID,
IdempotencyKey: "2fa-till-saved-ok",
VerificationCode: "556677",
}
bodyBytes, _ := json.Marshal(reqBody)
@@ -870,17 +862,6 @@ func TestSCASuccess_BookingSavedCard_NeverRequiresConsent(t *testing.T) {
require.Zero(t, auditCount, "an SCA-success charge must not write a fallback audit row")
}
// TestTwoFADeliveryAvailable_DevBuild_TriviallyTrue documents the dev/test
// delivery predicate: twofa_delivery_dev.go (`dev || test`) always reports a
// delivery channel (the [2FA] log relay), so the 503 "2FA requires an email or
// SMS delivery channel" branch is UNREACHABLE in this build. The production
// predicate — TWO_FACTOR_ALLOW_LOG_DELIVERY gating — is covered by
// twofa_delivery_prod_test.go in a !dev build (see its header for the
// documented limitation).
func TestTwoFADeliveryAvailable_DevBuild_TriviallyTrue(t *testing.T) {
require.True(t, twoFADeliveryAvailable())
}
// TestNotificationsCapExceeded pins Round 2 Loop B finding 1: the GLOBAL cap on
// unacknowledged 'critical_payment_log' admin notifications
// (adminnotify.MaxUnacknowledgedCriticalLogs, the shared cap living in
@@ -890,11 +871,8 @@ func TestTwoFADeliveryAvailable_DevBuild_TriviallyTrue(t *testing.T) {
// re-arms inserts. The same cap is now applied atomically (a conditional
// INSERT ... SELECT ... WHERE (SELECT COUNT(*) ...) < $cap) at every
// 'critical_payment_log' / 'refresh_token_reuse' insert site (webhooks,
// account erasure, jwt reuse); this test pins the shared count helper. NOTE:
// the REISSUE-FAIL alert (reissueTwoFACodeAfterFailedCharge) is intentionally
// NOT globally capped — Round 2 Loop B finding 2 caps it PER-ISSUE (dedup on
// reason+user_id) so one customer's alert is never suppressed by other users'
// rows.
// account erasure, jwt reuse, the payment sweep); this test pins the shared
// count helper.
func TestNotificationsCapExceeded(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
backend/handlers/payments/validators.go
+24 -9
View File
@@ -53,16 +53,31 @@ func ValidateRefundReason(reason string) error {
return nil
}
// ValidateCardInfo checks that exactly one of cardID or newCardToken is provided,
// non-nil, and non-empty.
func ValidateCardInfo(cardID, newCardToken *string) error {
hasCardID := cardID != nil && *cardID != ""
// ValidateCardInfo checks the card source shape of a payment request. Three
// shapes are legal:
//
// - saved-card only: a non-empty saved-card reference (card_id or
// saved_card_id — they are the same user_saved_cards.id, so callers pass
// the effective reference) with no new_card_token — a plain saved-card
// (ccof:) charge.
// - new-card only: a non-empty new_card_token with NO saved-card reference —
// a new-card (cnon:) one-off charge.
// - saved-card reference + new_card_token together: the SCA tokenize-result
// wire contract — the token (card.tokenize(verificationDetails, cardId)
// result) is the one-time charge SOURCE and the saved-card row supplies the
// Square customer. resolveChargeSource implements exactly this coexistence
// (see charge_helpers.go), so the validation must not reject it.
//
// Both absent is invalid ("either a card reference or new_card_token is
// required"). A bare new_card_token remains valid (the new-card path), and a
// card reference with no token remains valid (the legacy saved-card path);
// only the coexistence that used to be rejected — card ref + token — is now
// legal, resolved as the SCA tokenize-result source.
func ValidateCardInfo(cardID, savedCardID, newCardToken *string) error {
hasCardRef := (cardID != nil && *cardID != "") || (savedCardID != nil && *savedCardID != "")
hasToken := newCardToken != nil && *newCardToken != ""
if hasCardID && hasToken {
return errors.New("provide either card_id or new_card_token, not both")
}
if !hasCardID && !hasToken {
return errors.New("either card_id or new_card_token is required")
if !hasCardRef && !hasToken {
return errors.New("either a saved card reference (card_id/saved_card_id) or new_card_token is required")
}
return nil
}
backend/handlers/payments/validators_test.go
+146
View File
@@ -3,6 +3,7 @@
package payments
import (
"strings"
"testing"
"github.com/stretchr/testify/require"
@@ -33,3 +34,148 @@ func TestValidateAmount(t *testing.T) {
t.Errorf("expected exactly £10,000 to be allowed, got %v", err)
}
}
// TestValidateCardInfo_SCACoexistence pins the SCA saved-card wire contract on
// the card-source validator: a saved-card reference (card_id or saved_card_id)
// riding along WITH new_card_token is now VALID — the tokenize-result token is
// the one-time charge source and the saved-card row supplies the customer
// (resolveChargeSource implements the coexistence). A card reference alone
// (legacy saved-card), a token alone (new-card), empty references, and neither
// present keep their prior semantics.
func TestValidateCardInfo_SCACoexistence(t *testing.T) {
t.Parallel()
cardID := "card-1"
savedCardID := "card-2"
token := "cnon:sca-tokenize"
empty := ""
// card_id + new_card_token — now valid (SCA tokenize-result source).
require.NoError(t, ValidateCardInfo(&cardID, nil, &token),
"card_id + new_card_token must be valid (SCA tokenize-result)")
// saved_card_id + new_card_token — valid (SCA tokenize-result).
require.NoError(t, ValidateCardInfo(nil, &savedCardID, &token),
"saved_card_id + new_card_token must be valid (SCA tokenize-result)")
// card_id only — valid legacy saved-card charge.
require.NoError(t, ValidateCardInfo(&cardID, nil, nil),
"card_id alone must remain valid")
// new_card_token only — valid new-card charge.
require.NoError(t, ValidateCardInfo(nil, nil, &token),
"a bare new_card_token must remain valid (new-card path)")
// Neither present — invalid.
require.Error(t, ValidateCardInfo(nil, nil, nil),
"neither a card reference nor new_card_token must be rejected")
// Empty strings are treated as absent — invalid when nothing is present.
require.Error(t, ValidateCardInfo(&empty, &empty, &empty),
"empty-string values must be rejected as absent")
require.Error(t, ValidateCardInfo(&empty, nil, &empty),
"an empty card_id with an empty token must be rejected")
// An empty card reference with a real token is still the valid new-card path.
require.NoError(t, ValidateCardInfo(&empty, nil, &token),
"an empty card_id with a real new_card_token is the new-card path and must be valid")
}
// TestValidateCardInfo_Table covers the full card-source shape space as a table:
// the three legal wire shapes (saved-card reference only, new-card token only,
// and both together as the SCA tokenize-result source) plus the both-absent and
// empty-string rejections. This is the drift guard for the SCA wire contract —
// every shape the frontend can send is enumerated here.
func TestValidateCardInfo_Table(t *testing.T) {
t.Parallel()
cardID := "card-1"
savedCardID := "saved-2"
token := "cnon:sca-tokenize-3"
empty := ""
tests := []struct {
name string
cardID *string
savedCard *string
token *string
wantValid bool
}{
{"saved card reference (card_id) only — legacy saved-card charge", &cardID, nil, nil, true},
{"saved card reference (saved_card_id) only", nil, &savedCardID, nil, true},
{"new card token only — new-card charge", nil, nil, &token, true},
{"saved-card reference + new card token — SCA tokenize-result source", &cardID, nil, &token, true},
{"saved_card_id + new card token — SCA tokenize-result source", nil, &savedCardID, &token, true},
{"neither present — invalid", nil, nil, nil, false},
{"empty card_id + empty saved_card_id + empty token — all absent", &empty, &empty, &empty, false},
{"empty card_id + empty token — absent card ref", &empty, nil, &empty, false},
{"empty saved_card_id + empty token — absent card ref", nil, &empty, &empty, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateCardInfo(tt.cardID, tt.savedCard, tt.token)
if tt.wantValid {
require.NoErrorf(t, err, "expected shape to be valid: %v", tt)
} else {
require.Errorf(t, err, "expected shape to be rejected: %v", tt)
}
})
}
}
// TestValidatePartialAmount_PenceSemantics pins the pence comparison: amounts
// are integer pence, so the boundary is exact — equal pence passes, one penny
// over fails, and non-positive amounts are rejected regardless of the balance.
func TestValidatePartialAmount_PenceSemantics(t *testing.T) {
t.Parallel()
tests := []struct {
name string
amountPence int64
remaining int64
wantValid bool
}{
{"partial equal to the remaining balance passes", 2500, 2500, true},
{"partial below the remaining balance passes", 1000, 2500, true},
{"one penny over the remaining balance fails", 2501, 2500, false},
{"large partial over the balance fails", 5000, 2500, false},
{"zero amount rejected even with balance", 0, 2500, false},
{"negative amount rejected", -100, 2500, false},
{"zero remaining rejects any positive partial", 1, 0, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidatePartialAmount(tt.amountPence, tt.remaining)
if tt.wantValid {
require.NoErrorf(t, err, "expected %d pence against %d remaining to be valid", tt.amountPence, tt.remaining)
} else {
require.Errorf(t, err, "expected %d pence against %d remaining to be rejected", tt.amountPence, tt.remaining)
}
})
}
}
// TestValidateVerificationToken_Bound pins the 512-char bound on Square's
// verification token: empty/nil is fine (token-less charges are valid input),
// anything at or under 512 chars passes, and 513+ is rejected.
func TestValidateVerificationToken_Bound(t *testing.T) {
t.Parallel()
ok := strings.Repeat("t", 512)
tooLong := strings.Repeat("t", 513)
tests := []struct {
name string
token *string
wantErr bool
}{
{"nil token passes (token-less is valid)", nil, false},
{"empty token passes", strPtr(""), false},
{"exactly 512 chars passes", &ok, false},
{"513 chars rejected", &tooLong, true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateVerificationToken(tt.token)
if tt.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}
backend/handlers/payments/vat_test.go
+6 -3
View File
@@ -3524,9 +3524,12 @@ func TestVAT_DiscountAndCashPayment_RemainingBalance(t *testing.T) {
if summary.TotalNetAmount != 35.00 {
t.Errorf("expected TotalNetAmount 35.00 (25.00 cash net + 10.00 discount), got %.2f", summary.TotalNetAmount)
}
// RemainingAmount = total - paid = 100.00 - 40.00 = 60.00
if summary.RemainingAmount != 60.00 {
t.Errorf("expected RemainingAmount 60.00 (100.00 - 40.00), got %.2f", summary.RemainingAmount)
// RemainingAmount routes through the authoritative
// GetBookingRemainingBalancePence, which counts REAL money only — the £10
// discount row is a ledger entry, not a payment toward the balance — so
// remaining = 100.00 - 30.00 (cash) = 70.00.
if summary.RemainingAmount != 70.00 {
t.Errorf("expected RemainingAmount 70.00 (100.00 - 30.00 cash, discount not counted as paid), got %.2f", summary.RemainingAmount)
}
}
backend/handlers/scheduling/scheduled_cleanup_test.go
+143
View File
@@ -3,11 +3,14 @@
package scheduling
import (
"context"
"database/sql"
"fmt"
"testing"
"time"
"crussell/clock"
"crussell/db"
"crussell/internal/adminnotify"
"crussell/testutils/fixtures"
)
@@ -677,6 +680,146 @@ func TestCleanupExpiredRefreshTokens_PreservesValid(t *testing.T) {
}
}
// ============================================================
// ApplyScheduledDefaultHours Tests
// ============================================================
// seedStagedDefaultHoursChange inserts one pending default-hours change due on
// the given London date and returns its id. Weekday 1 (Monday) is staged to
// 10:00-18:00 open (the seeded default is 09:00-17:00 open), so an applied
// change is observable on the working_hours row.
func seedStagedDefaultHoursChange(t *testing.T, ctx context.Context, tx db.Querier, effectiveDate string) int {
t.Helper()
var id int
err := tx.QueryRow(ctx, `
INSERT INTO default_hours_scheduled_changes (effective_date, hours)
VALUES ($1::date, $2::jsonb)
RETURNING id
`, effectiveDate, `[{"weekday":1,"startTime":"10:00","endTime":"18:00","isOpen":true}]`).Scan(&id)
if err != nil {
t.Fatalf("failed to seed default hours change: %v", err)
}
return id
}
func getWorkingHoursForWeekday(t *testing.T, ctx context.Context, q db.Querier, weekday int) (start, end string, isOpen bool) {
t.Helper()
if err := q.QueryRow(ctx, `SELECT start_time, end_time, is_open FROM working_hours WHERE weekday = $1`, weekday).Scan(&start, &end, &isOpen); err != nil {
t.Fatalf("failed to read working_hours for weekday %d: %v", weekday, err)
}
return start, end, isOpen
}
// TestApplyScheduledDefaultHours_AppliesStagedChange seeds a default-hours
// change with effective_date YESTERDAY (London), runs ApplyScheduledDefaultHours
// and asserts the working_hours row was updated, the change marked applied_at,
// and the flood-capped 'default_hours_changed' admin notification inserted.
func TestApplyScheduledDefaultHours_AppliesStagedChange(t *testing.T) {
ctx, tx := resetTestData(t)
yesterday := LondonDateString(clock.Now().Add(-24 * time.Hour))
changeID := seedStagedDefaultHoursChange(t, ctx, tx, yesterday)
n, err := ApplyScheduledDefaultHours(ctx)
if err != nil {
t.Fatalf("ApplyScheduledDefaultHours failed: %v", err)
}
if n != 1 {
t.Errorf("expected 1 applied change, got %d", n)
}
start, end, isOpen := getWorkingHoursForWeekday(t, ctx, tx, 1)
if start != "10:00:00" || end != "18:00:00" || !isOpen {
t.Errorf("expected working_hours weekday 1 updated to 10:00-18:00 open, got %q-%q open=%v", start, end, isOpen)
}
var appliedAt sql.NullTime
if err := tx.QueryRow(ctx, `SELECT applied_at FROM default_hours_scheduled_changes WHERE id = $1`, changeID).Scan(&appliedAt); err != nil {
t.Fatalf("failed to read change applied_at: %v", err)
}
if !appliedAt.Valid {
t.Error("expected the scheduled change to be marked applied_at")
}
var notifCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM admin_notifications WHERE reason = 'default_hours_changed'`).Scan(&notifCount); err != nil {
t.Fatalf("failed to count notifications: %v", err)
}
if notifCount != 1 {
t.Errorf("expected 1 default_hours_changed admin notification, got %d", notifCount)
}
}
// TestApplyScheduledDefaultHours_NoDueChange_Noop verifies the no-op path: with
// no change due (effective_date in the future) the job returns 0 and touches
// nothing.
func TestApplyScheduledDefaultHours_NoDueChange_Noop(t *testing.T) {
ctx, tx := resetTestData(t)
tomorrow := LondonDateString(clock.Now().Add(24 * time.Hour))
seedStagedDefaultHoursChange(t, ctx, tx, tomorrow)
n, err := ApplyScheduledDefaultHours(ctx)
if err != nil {
t.Fatalf("ApplyScheduledDefaultHours failed: %v", err)
}
if n != 0 {
t.Errorf("expected 0 applied changes for a future effective_date, got %d", n)
}
start, end, isOpen := getWorkingHoursForWeekday(t, ctx, tx, 1)
if start != "09:00:00" || end != "17:00:00" || !isOpen {
t.Errorf("expected working_hours untouched, got %q-%q open=%v", start, end, isOpen)
}
}
// TestApplyScheduledDefaultHours_NotificationFloodCap pins C5 for the
// 'default_hours_changed' insert site: the unacknowledged queue is flood-capped
// at adminnotify.MaxUnacknowledgedCriticalLogs, so the change still applies
// (money/schedule-first) but no notification row is added past the cap.
func TestApplyScheduledDefaultHours_NotificationFloodCap(t *testing.T) {
ctx, tx := resetTestData(t)
yesterday := LondonDateString(clock.Now().Add(-24 * time.Hour))
changeID := seedStagedDefaultHoursChange(t, ctx, tx, yesterday)
for i := 0; i < adminnotify.MaxUnacknowledgedCriticalLogs; i++ {
if _, err := tx.Exec(ctx, `
INSERT INTO admin_notifications (reason, created_at)
VALUES ('default_hours_changed', NOW())
`); err != nil {
t.Fatalf("failed to seed default_hours_changed notification %d: %v", i, err)
}
}
if !adminnotify.CriticalLogsCapExceeded(ctx, tx, "default_hours_changed") {
t.Fatal("expected the unacknowledged default_hours_changed queue to be at the cap")
}
n, err := ApplyScheduledDefaultHours(ctx)
if err != nil {
t.Fatalf("ApplyScheduledDefaultHours failed: %v", err)
}
if n != 1 {
t.Errorf("expected the change to still apply at the notification cap, got %d", n)
}
var appliedAt sql.NullTime
if err := tx.QueryRow(ctx, `SELECT applied_at FROM default_hours_scheduled_changes WHERE id = $1`, changeID).Scan(&appliedAt); err != nil {
t.Fatalf("failed to read change applied_at: %v", err)
}
if !appliedAt.Valid {
t.Error("expected the scheduled change to be marked applied_at despite the notification cap")
}
var dbCount int
if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM admin_notifications WHERE reason = 'default_hours_changed'`).Scan(&dbCount); err != nil {
t.Fatalf("failed to count notifications: %v", err)
}
if dbCount != adminnotify.MaxUnacknowledgedCriticalLogs {
t.Errorf("expected the queue to stay capped at %d, got %d", adminnotify.MaxUnacknowledgedCriticalLogs, dbCount)
}
}
// ============================================================
// Multi-row Count Tests
// ============================================================
backend/handlers/webhooks/square.go
+39 -2
View File
@@ -826,16 +826,28 @@ func squarePaymentKnown(ctx context.Context, squarePaymentID string) (bool, erro
// verbatim, preserving both). Only 'pending' rows WITHOUT a square_payment_id
// are candidates — that is exactly the population the keyed stale-pending sweep
// replays (sweep.go), so a match is the sweep-minted duplicate's origin.
//
// AGE GATE (webhook-vs-response race): the candidates are additionally limited
// to rows old enough to have actually been replayed by the sweep
// (created_at <= NOW() - payments.SweepKeyedReplayAge()). The sweep only
// replays keyed rows past that age (sweep.go stalePendingKeyedAge), so a fresh
// pending row can never be a sweep-minted duplicate's origin — it is a legit
// charge whose completion webhook raced the handler's own square_payment_id
// write (response loss), and marking it failed would kill the customer's real
// payment. When the gate blocks a match the caller leaves the row pending
// (critical-log), never fails it — the sweep will reconcile it later.
func findPendingByOrphanKeys(ctx context.Context, payment squarePaymentPayload) (paymentID, bookingID string, found bool, err error) {
replayEligibleSince := time.Now().Add(-payments.SweepKeyedReplayAge())
if payment.IdempotencyKey != "" {
var pid string
var bid *string
err := db.Conn.QueryRow(ctx, `
SELECT id, booking_id FROM payments
WHERE status = 'pending' AND idempotency_key = $1 AND square_payment_id IS NULL
AND created_at <= $2
ORDER BY created_at DESC, id DESC
LIMIT 1
`, payment.IdempotencyKey).Scan(&pid, &bid)
`, payment.IdempotencyKey, replayEligibleSince).Scan(&pid, &bid)
if err == nil {
if bid != nil {
bookingID = *bid
@@ -859,9 +871,10 @@ func findPendingByOrphanKeys(ctx context.Context, payment squarePaymentPayload)
WHERE status = 'pending' AND square_payment_id IS NULL
AND ABS(amount - $2) < 0.005
AND (booking_id = $1 OR gift_card_id = $1)
AND created_at <= $3
ORDER BY created_at DESC, id DESC
LIMIT 1
`, payment.ReferenceID, amount).Scan(&pid, &bid)
`, payment.ReferenceID, amount, replayEligibleSince).Scan(&pid, &bid)
if err == nil {
if bid != nil {
bookingID = *bid
@@ -957,6 +970,30 @@ func detectOrphanedReplayCharge(ctx context.Context, payment squarePaymentPayloa
log.Printf("[SQUARE-WEBHOOK] payment.updated: COMPLETED square payment %s matches no local row and no pending origin row by idempotency key/reference_id — acknowledging (not a sweep-minted duplicate)", payment.ID)
return nil
}
// B1-EVIDENCE GATE: only treat the origin as a sweep-minted duplicate when
// the sweep actually minted+attempted to refund it (b1_attempts > 0 or a
// refunds row with the sweep-duplicate reason). Without that evidence a
// COMPLETED payment is far more likely the ORIGINAL charge whose completion
// webhook was delayed past the age gate — marking the origin failed would
// kill a customer's real payment. Leave it pending: the keyed sweep rescues
// the original 'completed' or triggers B1 if a duplicate was truly minted.
var b1Evidence bool
if err := db.Conn.QueryRow(ctx, `
SELECT EXISTS(
SELECT 1 FROM payments
WHERE id = $1 AND b1_attempts > 0
UNION ALL
SELECT 1 FROM refunds
WHERE payment_id = $1 AND reason = 'duplicate charge — sweep replay'
)
`, originID).Scan(&b1Evidence); err != nil {
log.Printf("[SQUARE-WEBHOOK] Orphan-replay B1-evidence lookup failed for origin payment %s: %v", originID, err)
return err
}
if !b1Evidence {
log.Printf("[SQUARE-WEBHOOK] payment.updated: COMPLETED square payment %s matches pending origin %s by idempotency key but NO B1 evidence (b1_attempts=0, no sweep-duplicate refund) — leaving origin pending for the sweep to reconcile (delayed legit completion or the sweep has not replayed yet); not marking failed", payment.ID, originID)
return nil
}
tag, err := db.Conn.Exec(ctx,
`UPDATE payments SET status = 'failed', updated_at = NOW() WHERE id = $1 AND status = 'pending'`,
originID)
backend/handlers/webhooks/webhooks_completion_asymmetry_test.go
+1 -1
View File
@@ -77,7 +77,7 @@ func TestWebhook_AndSweep_DoNotDoubleComplete(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_webhook_and_sweep_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
backend/handlers/webhooks/webhooks_dispatch_error_test.go
+2 -2
View File
@@ -27,7 +27,7 @@ func TestHandleSquareWebhook_DispatchError_NoDedup_RetryReDispatches(t *testing.
overflowEvent := SquareWebhookEvent{
Type: "dispute.created",
EventID: "evt_dispatch_err_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_dispatch_err_1",
@@ -55,7 +55,7 @@ func TestHandleSquareWebhook_DispatchError_NoDedup_RetryReDispatches(t *testing.
retryEvent := SquareWebhookEvent{
Type: "dispute.created",
EventID: "evt_dispatch_err_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_dispatch_err_1",
backend/handlers/webhooks/webhooks_round7_test.go
+6 -6
View File
@@ -144,7 +144,7 @@ func TestWebhook_DisputeStateUpdated_Lost_EmptyPaymentID_FallsBackToDisputeRow(t
event := SquareWebhookEvent{
Type: "dispute.state.updated",
EventID: "evt_dispute_fallback_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_fallback_1",
@@ -194,7 +194,7 @@ func TestWebhook_DisputeStateUpdated_EmptyPaymentID_NoDisputeRow_RaisesCritical(
event := SquareWebhookEvent{
Type: "dispute.state.updated",
EventID: "evt_dispute_no_row_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_no_dispute_row_1",
@@ -310,7 +310,7 @@ func TestWebhook_DisputeEvidence_InformationalOnly(t *testing.T) {
event := SquareWebhookEvent{
Type: tc.eventType,
EventID: tc.eventID,
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "` + tc.disputeID + `",
@@ -348,7 +348,7 @@ func TestWebhook_DisputeEvidence_InformationalOnly(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.evidence.created",
EventID: "evt_evidence_log_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_evidence_log_1",
@@ -379,7 +379,7 @@ func TestWebhook_TerminalCheckout_InformationalOnly(t *testing.T) {
event := SquareWebhookEvent{
Type: tc.eventType,
EventID: tc.eventID,
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"type": "terminal.checkout", "id": "` + tc.checkoutID + `"}`),
}
w := deliverWebhook(t, event)
@@ -407,7 +407,7 @@ func TestWebhook_TerminalCheckout_InformationalOnly(t *testing.T) {
event := SquareWebhookEvent{
Type: "terminal.checkout.updated",
EventID: "evt_terminal_log_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"type": "terminal.checkout", "id": "chk_round7_log_1"}`),
}
if w := deliverWebhook(t, event); w.Code != http.StatusOK {
backend/handlers/webhooks/webhooks_state_test.go
+120 -38
View File
@@ -10,12 +10,22 @@ import (
"net/http/httptest"
"strings"
"testing"
"time"
"unicode/utf8"
"crussell/clock"
"crussell/db"
"crussell/testutils/fixtures"
)
// nowInRFC3339 returns the current UTC instant (plus an offset) as an RFC3339
// string. Webhook-event timestamps and gift-card/till-sale created_at values
// must be clock-relative so sweep/expiry-window logic stays correct forever
// instead of drifting against a hardcoded 2025 date.
func nowInRFC3339(offset time.Duration) string {
return clock.Now().Add(offset).UTC().Format(time.RFC3339)
}
// =============================================================================
// Helpers — DB-backed state assertions
// =============================================================================
@@ -157,7 +167,7 @@ func deliverPaymentUpdatedFailed(t *testing.T, squarePaymentID string) *httptest
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_" + squarePaymentID,
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -202,7 +212,7 @@ func getGiftCardFunding(t *testing.T, id string) (totalFundsAdded, amountRemaini
// does.
func TestWebhook_PaymentUpdated_Failed_ClawsBackCreatedCard(t *testing.T) {
const squarePaymentID = "sqp_clawback_create"
const cardCreatedAt = "2025-01-01T00:00:00Z"
cardCreatedAt := nowInRFC3339(0)
saleID, giftCardID := createWebhookTestGiftCardAndSale(t, squarePaymentID, cardCreatedAt, cardCreatedAt, 40.00)
w := deliverPaymentUpdatedFailed(t, squarePaymentID)
@@ -222,7 +232,8 @@ func TestWebhook_PaymentUpdated_Failed_ClawsBackCreatedCard(t *testing.T) {
// of the card and the sale is marked failed.
func TestWebhook_PaymentUpdated_Failed_ClawsBackTopup(t *testing.T) {
const squarePaymentID = "sqp_clawback_topup"
saleID, giftCardID := createWebhookTestGiftCardAndSale(t, squarePaymentID, "2025-01-01T00:00:00Z", "2025-01-02T00:00:00Z", 60.00)
cardCreatedAt := nowInRFC3339(0)
saleID, giftCardID := createWebhookTestGiftCardAndSale(t, squarePaymentID, cardCreatedAt, nowInRFC3339(24*time.Hour), 60.00)
w := deliverPaymentUpdatedFailed(t, squarePaymentID)
if w.Code != http.StatusOK {
@@ -243,7 +254,8 @@ func TestWebhook_PaymentUpdated_Failed_ClawsBackTopup(t *testing.T) {
// untouched.
func TestWebhook_PaymentUpdated_Failed_AlreadyResolved_Skipped(t *testing.T) {
const squarePaymentID = "sqp_clawback_resolved"
saleID, giftCardID := createWebhookTestGiftCardAndSale(t, squarePaymentID, "2025-01-01T00:00:00Z", "2025-01-01T00:00:00Z", 40.00)
createdAt := nowInRFC3339(0)
saleID, giftCardID := createWebhookTestGiftCardAndSale(t, squarePaymentID, createdAt, createdAt, 40.00)
if _, err := db.Conn.Exec(context.Background(),
"UPDATE till_sales SET status = 'completed', updated_at = NOW() WHERE id = $1", saleID); err != nil {
t.Fatalf("failed to resolve till sale: %v", err)
@@ -282,7 +294,7 @@ func TestWebhook_DisputeCreated_InsertsDisputeRow(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.created",
EventID: "evt_dispute_created_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_dispute_created_1",
@@ -347,7 +359,7 @@ func TestWebhook_DisputeCreated_NoLocalPayment_NoRow(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.created",
EventID: "evt_dispute_orphan_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_orphan_1",
@@ -407,7 +419,7 @@ func TestWebhook_DisputeCreated_Untracked_DistinctDisputes_DistinctNotifications
event := SquareWebhookEvent{
Type: "dispute.created",
EventID: fmt.Sprintf("evt_untracked_distinct_%d", i),
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "` + d.disputeID + `",
@@ -455,7 +467,7 @@ func TestWebhook_DisputeCreated_Untracked_SameDisputeRedelivered_SingleNotificat
event := SquareWebhookEvent{
Type: "dispute.created",
EventID: eventID,
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "` + disputeID + `",
@@ -494,7 +506,7 @@ func TestWebhook_DisputeCreated_LongReason_Truncated(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.created",
EventID: "evt_dispute_longreason_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_longreason_1",
@@ -544,7 +556,7 @@ func TestWebhook_DisputeCreated_Utf8Reason_StoredValid(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.created",
EventID: "evt_dispute_utf8_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_utf8_1",
@@ -594,7 +606,7 @@ func TestWebhook_DisputeStateUpdated_Lost_MarksPaymentFailed(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.state.updated",
EventID: "evt_dispute_lost_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_lost_1",
@@ -633,7 +645,7 @@ func TestWebhook_DisputeStateUpdated_Won_KeepsPaymentCompleted(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.state.updated",
EventID: "evt_dispute_won_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_won_1",
@@ -672,7 +684,7 @@ func TestWebhook_DisputeStateUpdated_Open_KeepsOpen(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.state.updated",
EventID: "evt_dispute_open_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "dispute",
"id": "dts_open_1",
@@ -709,7 +721,7 @@ func TestWebhook_PaymentUpdated_UpdatesPaymentStatus(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_payment_updated_completed_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -737,7 +749,7 @@ func TestWebhook_PaymentUpdated_FailedStatus(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_payment_updated_failed_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -765,7 +777,7 @@ func TestWebhook_PaymentUpdated_NonTerminal_LeavesPending(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_payment_updated_approved_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -798,7 +810,7 @@ func TestWebhook_PaymentUpdated_DoesNotRevertRefunded(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_payment_updated_refunded_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -833,7 +845,7 @@ func TestWebhook_PaymentUpdated_Completed_RescuesPendingTillSale(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_payment_updated_till_rescue_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -864,7 +876,7 @@ func TestWebhook_PaymentUpdated_IdempotentReplay(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_payment_updated_idem_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -905,9 +917,15 @@ func TestWebhook_PaymentUpdated_IdempotentReplay(t *testing.T) {
func createWebhookTestPendingOrigin(t *testing.T, idempotencyKey string) string {
t.Helper()
var id string
// The origin row is aged past the sweep's keyed-replay age
// (payments.SweepKeyedReplayAge — 22h): the orphan detection only treats a
// pending row as a sweep-minted duplicate's origin once the row is old
// enough that the sweep could have replayed it (a fresh pending row is a
// legit charge whose response was lost and must never be failed by a
// webhook that raced it).
err := db.Conn.QueryRow(context.Background(), `
INSERT INTO payments (payment_type, payment_method, status, amount, idempotency_key, created_at, updated_at)
VALUES ('full', 'online_square', 'pending', 10.00, $1, NOW(), NOW())
VALUES ('full', 'online_square', 'pending', 10.00, $1, NOW() - INTERVAL '23 hours', NOW())
RETURNING id
`, idempotencyKey).Scan(&id)
if err != nil {
@@ -916,6 +934,18 @@ func createWebhookTestPendingOrigin(t *testing.T, idempotencyKey string) string
return id
}
// seedWebhookTestB1Evidence simulates the sweep having replayed the origin's
// expired key, minted a duplicate charge, and attempted its B1 auto-refund
// (b1_attempts > 0). The orphan-detection B1-evidence gate requires this
// before marking the origin failed.
func seedWebhookTestB1Evidence(t *testing.T, originID string) {
t.Helper()
if _, err := db.Conn.Exec(context.Background(),
`UPDATE payments SET b1_attempts = 1 WHERE id = $1`, originID); err != nil {
t.Fatalf("failed to seed b1_attempts on origin payment: %v", err)
}
}
func countOrphanReplayNotifications(t *testing.T, originID string) int {
t.Helper()
var n int
@@ -938,11 +968,14 @@ func TestWebhook_PaymentUpdated_OrphanedReplay_MarksOriginFailed(t *testing.T) {
idemKey = "b1-orphan-key-001"
)
originID := createWebhookTestPendingOrigin(t, idemKey)
// The sweep replayed the origin's expired key and attempted the B1
// auto-refund — the evidence the orphan-detection gate requires.
seedWebhookTestB1Evidence(t, originID)
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_orphan_c2_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + orphanSquareID + `",
@@ -993,7 +1026,7 @@ func TestWebhook_PaymentUpdated_OrphanedReplay_NoOrigin_Noop(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_orphan_noorigin_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + orphanSquareID + `",
@@ -1019,6 +1052,50 @@ func TestWebhook_PaymentUpdated_OrphanedReplay_NoOrigin_Noop(t *testing.T) {
}
}
// TestWebhook_PaymentUpdated_OrphanedReplay_NoB1Evidence_LeavesPending
// verifies the B1-evidence gate: a COMPLETED payment.updated that matches a
// pending origin row by idempotency key but with NO B1 evidence (the sweep
// never replayed + auto-refunded) is treated as a delayed legit completion —
// the origin is LEFT pending, never marked failed, and no orphan notification
// is raised. The stale-pending sweep reconciles the row instead.
func TestWebhook_PaymentUpdated_OrphanedReplay_NoB1Evidence_LeavesPending(t *testing.T) {
const (
orphanSquareID = "sqp_orphan_noevidence"
idemKey = "b1-orphan-key-no-evidence"
)
originID := createWebhookTestPendingOrigin(t, idemKey)
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_orphan_noevidence_1",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + orphanSquareID + `",
"object": {
"payment": {
"id": "` + orphanSquareID + `",
"status": "COMPLETED",
"idempotency_key": "` + idemKey + `",
"amount_money": {"amount": 1000, "currency": "GBP"}
}
}
}`),
}
w := deliverWebhook(t, event)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
// The origin must remain pending — the delayed legit completion must never
// be failed without sweep B1 evidence.
if got := getPaymentStatus(t, originID); got != "pending" {
t.Errorf("expected origin pending payment to stay 'pending', got %q", got)
}
if n := countOrphanReplayNotifications(t, originID); n != 0 {
t.Errorf("expected 0 orphan-replay notifications without B1 evidence, got %d", n)
}
}
// TestWebhook_PaymentUpdated_OrphanedReplay_ReferenceFallback locks the
// reference_id fallback of the origin lookup: a COMPLETED orphan event whose
// payload carries no idempotency key still finds its pending origin row via the
@@ -1029,18 +1106,22 @@ func TestWebhook_PaymentUpdated_OrphanedReplay_ReferenceFallback(t *testing.T) {
refID = "b16bad0000aa"
)
var originID string
// The origin row is aged past the sweep's keyed-replay age (22h) so the
// orphan detection treats it as a sweep-replayable origin (a fresh pending
// row is never failed by the orphan detection — see the age gate).
if err := db.Conn.QueryRow(context.Background(), `
INSERT INTO payments (payment_type, payment_method, status, amount, gift_card_id, created_at, updated_at)
VALUES ('full', 'online_square', 'pending', 10.00, $1, NOW(), NOW())
VALUES ('full', 'online_square', 'pending', 10.00, $1, NOW() - INTERVAL '23 hours', NOW())
RETURNING id
`, refID).Scan(&originID); err != nil {
t.Fatalf("failed to create reference-origin payment: %v", err)
}
seedWebhookTestB1Evidence(t, originID)
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_orphan_refc2_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + orphanSquareID + `",
@@ -1078,7 +1159,7 @@ func TestWebhook_PaymentUpdated_SettledRow_NoOrphanDetection(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_settled_replay_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -1119,7 +1200,7 @@ func TestWebhook_RefundUpdated_UpdatesRefundStatus(t *testing.T) {
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_updated_completed_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1152,7 +1233,7 @@ func TestWebhook_RefundUpdated_FailedStatus(t *testing.T) {
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_updated_failed_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1189,7 +1270,7 @@ func TestWebhook_RefundUpdated_RejectedStatus(t *testing.T) {
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_updated_rejected_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1229,7 +1310,7 @@ func TestWebhook_RefundUpdated_Failed_RaisesAdminNotification(t *testing.T) {
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_updated_failed_notify_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1275,7 +1356,7 @@ func TestWebhook_RefundUpdated_NonTerminal_LeavesPending(t *testing.T) {
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_updated_pending_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1316,7 +1397,7 @@ func TestWebhook_RefundUpdated_Approved_IsNonTerminal(t *testing.T) {
approved := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_updated_approved_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1342,7 +1423,7 @@ func TestWebhook_RefundUpdated_Approved_IsNonTerminal(t *testing.T) {
failed := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_updated_approved_fail_1",
CreatedAt: "2025-01-01T00:00:01Z",
CreatedAt: nowInRFC3339(time.Second),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1387,7 +1468,7 @@ func TestWebhook_RefundUpdated_Completed_SweepDupResolvesParent(t *testing.T) {
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_sweepdup_parent_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1418,7 +1499,8 @@ func TestWebhook_RefundUpdated_Completed_SweepDupResolvesParent(t *testing.T) {
// gift card and mark the sale failed — mirroring the sweep's re-poll resolution.
func TestWebhook_RefundUpdated_Completed_SweepDupResolvesTillSale(t *testing.T) {
const squareRefundID = "sqr_sweepdup_tillsale"
saleID, giftCardID := createWebhookTestGiftCardAndSale(t, "sqp_sweepdup_tillsale_pay", "2025-01-01T00:00:00Z", "2025-01-01T00:00:00Z", 40.00)
createdAt := nowInRFC3339(0)
saleID, giftCardID := createWebhookTestGiftCardAndSale(t, "sqp_sweepdup_tillsale_pay", createdAt, createdAt, 40.00)
// A synthetic completed payments row anchors the refund (mirrors
// recordSweepDuplicateRefundRow); the till_sale id lives in the reason.
@@ -1439,7 +1521,7 @@ func TestWebhook_RefundUpdated_Completed_SweepDupResolvesTillSale(t *testing.T)
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_sweepdup_tillsale_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1482,7 +1564,7 @@ func TestWebhook_RefundUpdated_DoesNotDemoteCompleted(t *testing.T) {
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_demote_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "` + squareRefundID + `",
@@ -1524,7 +1606,7 @@ func TestWebhook_EventTypeAliases_RouteToUpdatedHandlers(t *testing.T) {
payEvent := SquareWebhookEvent{
Type: "payment.created",
EventID: "evt_alias_payment_created_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + sqPayID + `",
@@ -1554,7 +1636,7 @@ func TestWebhook_EventTypeAliases_RouteToUpdatedHandlers(t *testing.T) {
refundEvent := SquareWebhookEvent{
Type: "refund.created",
EventID: "evt_alias_refund_created_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "refund",
"id": "sqr_alias_refund",
backend/handlers/webhooks/webhooks_test.go
+20 -20
View File
@@ -211,7 +211,7 @@ func TestHandleSquareWebhook_PaymentUpdated(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_payment_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_1"}`),
}
body, _ := json.Marshal(event)
@@ -229,7 +229,7 @@ func TestHandleSquareWebhook_RefundUpdated(t *testing.T) {
event := SquareWebhookEvent{
Type: "refund.updated",
EventID: "evt_refund_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"refund_1"}`),
}
body, _ := json.Marshal(event)
@@ -244,7 +244,7 @@ func TestHandleSquareWebhook_DisputeCreated(t *testing.T) {
event := SquareWebhookEvent{
Type: "dispute.created",
EventID: "evt_dispute_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"dispute_1"}`),
}
body, _ := json.Marshal(event)
@@ -267,7 +267,7 @@ func TestHandleSquareWebhook_UnknownEventType(t *testing.T) {
event := SquareWebhookEvent{
Type: "frobnicator.created",
EventID: "evt_unknown_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"frob_1"}`),
}
body, _ := json.Marshal(event)
@@ -305,7 +305,7 @@ func TestHandleSquareWebhook_KnownNonMoneyEvent_Acknowledged(t *testing.T) {
event := SquareWebhookEvent{
Type: "customer.created",
EventID: "evt_nonmoney_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"cust_1"}`),
}
body, _ := json.Marshal(event)
@@ -344,7 +344,7 @@ func TestHandleSquareWebhook_UnhandledMoneyEvent_NotAcknowledged(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.checkout_offer_created",
EventID: "evt_money_unhandled_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"pco_1"}`),
}
body, _ := json.Marshal(event)
@@ -427,7 +427,7 @@ func TestHandleSquareWebhook_ValidSignatureWithEnvKey(t *testing.T) {
// returns 503 without a dedup row (Square retries), which is NOT this
// test's intent. The unique event_id and square_payment_id avoid colliding
// with the other tests' dedup rows and payment fixtures.
body := []byte(`{"type":"payment.updated","event_id":"evt_envkey_1","created_at":"2025-01-01T00:00:00Z","data":{"object":{"payment":{"id":"sqp_env_key_1","status":"COMPLETED","amount_money":{"amount":5000,"currency":"GBP"},"updated_at":"2025-01-01T00:00:00Z"}}}}`)
body := []byte(fmt.Sprintf(`{"type":"payment.updated","event_id":"evt_envkey_1","created_at":%q,"data":{"object":{"payment":{"id":"sqp_env_key_1","status":"COMPLETED","amount_money":{"amount":5000,"currency":"GBP"},"updated_at":%q}}}}`, nowInRFC3339(0), nowInRFC3339(0)))
key := "env-signing-key"
notificationURL := "http://localhost:8080/webhooks/square"
@@ -486,7 +486,7 @@ func TestHandleSquareWebhook_NoRawPayloadInLogs(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_pii_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_pii_1","buyer_email_address":"secret@example.com",
"card_details":{"card":{"brand":"VISA","last_4":"1234","cardholder_name":"Jane Doe"}}}`),
}
@@ -525,7 +525,7 @@ func TestHandleSquareWebhook_DuplicateEventID(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_http_dup_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_dup_1"}`),
}
body, _ := json.Marshal(event)
@@ -557,7 +557,7 @@ func TestHandleSquareWebhook_DistinctEventIDs(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: id,
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"p"}`),
}
body, _ := json.Marshal(event)
@@ -621,7 +621,7 @@ func TestHandleSquareWebhook_DedupDispatchOnce(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_dispatch_once_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_dispatch_once_1"}`),
}
body, _ := json.Marshal(event)
@@ -663,7 +663,7 @@ func TestHandleSquareWebhook_DedupPersistsAcrossRestart(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_restart_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_restart_1"}`),
}
body, _ := json.Marshal(event)
@@ -718,7 +718,7 @@ func TestHandleSquareWebhook_ConcurrentSameEvent_Serialized(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: eventID,
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -795,7 +795,7 @@ func TestHandleSquareWebhook_DedupCacheEviction_Redispatches(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_dedup_eviction_target",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{
"type": "payment",
"id": "` + squarePaymentID + `",
@@ -858,7 +858,7 @@ func TestHandleSquareWebhook_DedupInsertFails_FailsClosed(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_db_down_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_db_down_1"}`),
}
body, _ := json.Marshal(event)
@@ -888,7 +888,7 @@ func TestHandleSquareWebhook_DedupNilConn_FailsClosed(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_nil_conn_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_nil_conn_1"}`),
}
body, _ := json.Marshal(event)
@@ -920,7 +920,7 @@ func TestHandleSquareWebhook_EnvMismatch_Rejected(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_env_mismatch_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_env_mismatch_1"}`),
}
body, _ := json.Marshal(event)
@@ -942,7 +942,7 @@ func TestHandleSquareWebhook_EnvMatch_Accepted(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_env_match_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_env_match_1"}`),
}
body, _ := json.Marshal(event)
@@ -965,7 +965,7 @@ func TestHandleSquareWebhook_EnvHeaderAbsent_Allowed(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_env_absent_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_env_absent_1"}`),
}
body, _ := json.Marshal(event)
@@ -986,7 +986,7 @@ func TestHandleSquareWebhook_EnvMismatch_DevNotEnforced(t *testing.T) {
event := SquareWebhookEvent{
Type: "payment.updated",
EventID: "evt_env_dev_1",
CreatedAt: "2025-01-01T00:00:00Z",
CreatedAt: nowInRFC3339(0),
Data: json.RawMessage(`{"id":"payment_env_dev_1"}`),
}
body, _ := json.Marshal(event)