diff --git a/backend/handlers/payments/charge_helpers_test.go b/backend/handlers/payments/charge_helpers_test.go index cdde329..1ade0f2 100644 --- a/backend/handlers/payments/charge_helpers_test.go +++ b/backend/handlers/payments/charge_helpers_test.go @@ -204,3 +204,31 @@ func TestEncryptDecryptSnapshot_WrongKeyFails(t *testing.T) { _, err = decryptSnapshot(enc) require.Error(t, err, "a snapshot encrypted with a different key must not decrypt") } + +// TestResolveChargeSource_SCATokenizeResult_UsesTokenAsSource pins the SCA +// wire contract at the source-resolution level: a call carrying BOTH a +// new-card token (the SCA tokenize-result) and a saved card id must return the +// TOKEN as the charge source — never the stored ccof id — while still deriving +// the Square customer from the saved card row. +func TestResolveChargeSource_SCATokenizeResult_UsesTokenAsSource(t *testing.T) { + ctx := context.Background() + userID, err := fixtures.CreateTestUser(db.Conn) + require.NoError(t, err) + defer func() { + InvalidateSquareCustomerCache(userID) + _, _ = db.Conn.Exec(ctx, `DELETE FROM user_saved_cards WHERE user_id = $1`, userID) + _, _ = db.Conn.Exec(ctx, `DELETE FROM users WHERE id = $1`, userID) + }() + + cardID, err := fixtures.CreateTestPaymentMethod(db.Conn, userID, "ccof:sca-tokenize-unit", "VISA", "4242") + require.NoError(t, err) + + token := "cnon:sca-tokenize-unit" + sourceID, savedCardID, sqCustID, ok := resolveChargeSource(ctx, httptest.NewRecorder(), NewPaymentService(), userID, &token, &cardID, false, "") + require.True(t, ok, "the tokenize-result source must resolve") + require.Equal(t, token, sourceID, "the tokenize-result token must be the charge source") + require.NotEqual(t, "ccof:sca-tokenize-unit", sourceID, "the stored ccof id must NOT be the source") + require.NotNil(t, savedCardID, "the saved-card row id must be returned") + require.Equal(t, cardID, *savedCardID, "the saved-card row id must match the input card") + require.NotEmpty(t, sqCustID, "customer_id must derive from the saved card row") +} diff --git a/backend/handlers/payments/errors_test.go b/backend/handlers/payments/errors_test.go index ebaef81..cd0d995 100644 --- a/backend/handlers/payments/errors_test.go +++ b/backend/handlers/payments/errors_test.go @@ -729,10 +729,11 @@ func TestVerificationRequiredSurfacing_AllChargeSites(t *testing.T) { }) } -// the dedicated add-card endpoint: with REQUIRE_2FA enforced and the user NOT -// having completed 2FA setup, persisting a card is blocked with 403 and no card -// row is created — the save-card endpoint is not an un-gated side door. -func TestTwoFactorEnforced_CreatePaymentMethod_Blocked_403(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. +func TestTwoFactorEnforced_CreatePaymentMethod_Tokenless_402(t *testing.T) { t.Setenv("REQUIRE_2FA", "true") t.Setenv("SQUARE_ENVIRONMENT", "production") ctx, tx := testutils.SetupTestTx(t) @@ -745,26 +746,26 @@ func TestTwoFactorEnforced_CreatePaymentMethod_Blocked_403(t *testing.T) { handler := CreatePaymentMethod w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "cnon:2fa-blocked"}, token, ctx) - if w.Code != http.StatusForbidden { - t.Fatalf("expected 403 when 2FA is enforced and the user has not enabled it, got %d: %s", w.Code, w.Body.String()) + 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()) } var body map[string]string require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) - require.Contains(t, body["error"], "Two-factor") + require.Equal(t, "verification_required", body["code"]) var cardCount int if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, userID).Scan(&cardCount); err != nil { t.Fatalf("failed to count saved cards: %v", err) } if cardCount != 0 { - t.Errorf("a blocked 2FA save must not persist a card, got %d rows", cardCount) + t.Errorf("a refused token-less save must not persist a card, got %d rows", cardCount) } } -// TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Succeeds verifies the gate -// lets a user WHO HAS enabled 2FA (and provides a matching one-time code) save -// a card through the add-card endpoint. -func TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Succeeds(t *testing.T) { +// TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Blocked verifies that even +// a VALID 2FA code cannot save a card through the add-card endpoint when +// enforced (SCA-only — the homegrown 2FA fallback was removed entirely). +func TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Blocked(t *testing.T) { t.Setenv("REQUIRE_2FA", "true") t.Setenv("SQUARE_ENVIRONMENT", "production") ctx, tx := testutils.SetupTestTx(t) @@ -782,24 +783,25 @@ func TestTwoFactorEnforced_CreatePaymentMethod_With2FA_Succeeds(t *testing.T) { handler := CreatePaymentMethod w := makePaymentRequest(handler, "POST", "/api/user/payment-methods", CreatePaymentMethodRequest{CardToken: "cnon:2fa-ok", VerificationCode: "778899"}, token, ctx) - if w.Code != http.StatusOK { - t.Fatalf("expected 200 when 2FA is enabled and the code matches, got %d: %s", w.Code, w.Body.String()) + 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()) } var cardCount int if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, userID).Scan(&cardCount); err != nil { t.Fatalf("failed to count saved cards: %v", err) } - if cardCount != 1 { - t.Errorf("expected exactly 1 saved card, got %d", cardCount) + if cardCount != 0 { + t.Errorf("a valid 2FA code must not persist a card (SCA-only), got %d", cardCount) } } -// TestTwoFactorEnforced_BuyGiftCard_SaveCard_Blocked_403 verifies the H4 gate +// TestTwoFactorEnforced_BuyGiftCard_SaveCard_Tokenless_402 verifies the H4 gate // fires on the gift-card purchase path too: BuyGiftCard with req.SaveCard=true -// requires 2FA when enforced, mirroring CreatePaymentMethod/CreateBookingPayment. -// The purchase is rejected with 403 BEFORE any payment row is inserted. -func TestTwoFactorEnforced_BuyGiftCard_SaveCard_Blocked_403(t *testing.T) { +// is refused 402 verification_required (SCA-only) when enforced, mirroring +// CreatePaymentMethod/CreateBookingPayment. The purchase is rejected BEFORE any +// payment row is inserted. +func TestTwoFactorEnforced_BuyGiftCard_SaveCard_Tokenless_402(t *testing.T) { t.Setenv("REQUIRE_2FA", "true") t.Setenv("SQUARE_ENVIRONMENT", "production") ctx, tx := testutils.SetupTestTx(t) @@ -820,19 +822,19 @@ func TestTwoFactorEnforced_BuyGiftCard_SaveCard_Blocked_403(t *testing.T) { } w := makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", req, token, ctx) - if w.Code != http.StatusForbidden { - t.Fatalf("expected 403 for BuyGiftCard with SaveCard=true without 2FA, got %d: %s", w.Code, w.Body.String()) + if w.Code != http.StatusPaymentRequired { + t.Fatalf("expected 402 for BuyGiftCard with SaveCard=true without SCA, got %d: %s", w.Code, w.Body.String()) } var body map[string]string require.NoError(t, json.Unmarshal(w.Body.Bytes(), &body)) - require.Contains(t, body["error"], "Two-factor") + require.Equal(t, "verification_required", body["code"]) var payCount int if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE created_by = $1`, userID).Scan(&payCount); err != nil { t.Fatalf("failed to count payments: %v", err) } if payCount != 0 { - t.Errorf("a blocked gift-card purchase must not create a payment row, got %d", payCount) + t.Errorf("a refused gift-card purchase must not create a payment row, got %d", payCount) } } diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index b9d18f6..9edb43e 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -48,6 +48,11 @@ type TillSaleRequest struct { // 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 { @@ -631,6 +636,24 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // idempotency dedup so a same-key retry of an already-completed sale is // returned (not blocked) even on a capped day. Runs before any gift-card // write. + // + // F5: the cap check and the sale's gift-card creation/top-up run under the + // SAME per-admin advisory lock the admin API surfaces use + // (acquireGiftCardDailyCapLock, giftcards.go). The per-sale + // crussell:till: lock above serializes ONE sale's retries, + // but two concurrent DISTINCT till sales by the same admin could otherwise + // both read a below-cap day's value before either commits and overshoot the + // £5,000 ceiling. The lock is acquired BEFORE the adminGiftCardValueToday + // read and held (via the defers) through the transaction that records the + // new value, exactly like CreateGiftCard / TopUpGiftCard / + // TransferGiftCard. + capPinConn, capLockOK := acquireGiftCardDailyCapLock(ctx, w, adminID) + if !capLockOK { + return + } + defer capPinConn.Release() + defer releasePaymentLock(capPinConn, giftCardDailyCapLockKey+adminID) + adminValueToday, dailyErr := adminGiftCardValueToday(ctx, db.Conn, adminID) if dailyErr != nil { log.Printf("Failed to query admin gift-card value today for %s: %v", adminID, dailyErr) @@ -778,13 +801,31 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { var isInventory bool var previousTotal float64 - err = tx.QueryRow(ctx, `SELECT is_inventory, total_funds_added FROM gift_cards WHERE id = $1`, cardID).Scan(&isInventory, &previousTotal) + var expiryDate sql.NullTime + err = tx.QueryRow(ctx, `SELECT is_inventory, total_funds_added, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&isInventory, &previousTotal, &expiryDate) if err != nil { log.Printf("Failed to check gift card state: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) 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 giftcards.go RedeemGiftCard's expiry + // gate (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 + } + _, err = tx.Exec(ctx, ` UPDATE gift_cards SET total_funds_added = total_funds_added + $1, @@ -957,24 +998,14 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { dbPaymentMethod = "cash" case "saved_card": dbPaymentMethod = "online_square" - if req.UserID != nil && *req.UserID != "" { - _, err = service.GetCardByIDQuerier(ctx, tx, *req.UserSavedCardID, *req.UserID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - http.Error(w, "Saved card not found", http.StatusNotFound) - return - } - log.Printf("Failed to verify saved card: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - } // The till path is not user-scoped, so fetch the card's owner along with - // the charge details — the owner is needed to lazily provision a Square - // customer if the row predates P14 (R6). cardUserID is declared at - // function scope (before the switch) because the post-charge 2FA - // consumption + audit need it after the Square call. + // the charge details — the owner-agnostic SELECT is the source of truth + // for ownership. The owner is needed to lazily provision a Square + // customer if the row predates P14 (R6), and to key the 2FA/consent + // gate. cardUserID is declared at function scope (before the switch) + // because the post-charge 2FA consumption + audit need it after the + // Square call. err = tx.QueryRow(ctx, ` SELECT user_id, COALESCE(square_card_id, ''), COALESCE(square_customer_id, '') FROM user_saved_cards @@ -986,6 +1017,20 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { return } + // F1 ownership invariant: every other charge surface verifies the card + // belongs to the request's user before charging. When the admin names + // the customer being served (req.UserID), the card MUST be owned by + // that customer — an ownerless or foreign card is a confused-deputy + // signal and is rejected outright. An OMITTED user_id is the admin-till + // legit use (the admin may charge any customer's card): the charge + // proceeds under the RESOLVED owner, which keys the 2FA/consent gate + // below and is the target of the confused-deputy CRITICAL audit on + // success. + if req.UserID != nil && *req.UserID != "" && (!cardUserID.Valid || cardUserID.String != *req.UserID) { + http.Error(w, "Saved card does not belong to the specified user", http.StatusBadRequest) + return + } + // A ccof: source can NEVER be charged without a CustomerID — Square // rejects the payment. Legacy pre-P14 rows have an empty // square_customer_id; provision + persist for the card's owner BEFORE @@ -1029,6 +1074,11 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { 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) { + return + } } tillSquareSourceID = savedCardSqCardID @@ -1296,7 +1346,15 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // attempt's responsibility". if isDefinitiveChargeFailure(squareErr) { if revErr := revertGiftCardFunding(ctx, req.Action, giftCardID, req.Amount, req.RedeemToUserID, tillSaleID); revErr != nil { - log.Printf("CRITICAL: till sale %s charge definitively failed (%v) but gift-card clawback also failed: %v — MANUAL RECONCILIATION REQUIRED: gift card %s may still be funded", tillSaleID, squareErr, revErr, giftCardID) + // M6: the clawback ALWAYS reverts the funding on a failed + // sale; the two failure shapes here are a partial reversal + // (funding was already spent — a CRITICAL admin notification + // is already inserted by the clawback) and a hard failure. + if errors.Is(revErr, errClawbackPartiallyReversed) { + log.Printf("CRITICAL: till sale %s charge definitively failed (%v) — gift-card funding was PARTIALLY clawed back (already spent) — MANUAL RECONCILIATION REQUIRED: gift card %s may retain a residual to reconcile", tillSaleID, squareErr, giftCardID) + } else { + log.Printf("CRITICAL: till sale %s charge definitively failed (%v) but gift-card clawback also failed: %v — MANUAL RECONCILIATION REQUIRED: gift card %s may still be funded", tillSaleID, squareErr, revErr, giftCardID) + } } } // The gate consumed the 2FA code for a FRESH saved-card charge — @@ -1369,7 +1427,18 @@ func CreateTillSale(w http.ResponseWriter, r *http.Request) { // 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)") + 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 + // the sale's stated user. Surface a CRITICAL operator notification + // (deduped per owner) so an admin who charges a card under an + // unintended owner is caught. A provided-and-matched user_id needs + // no such flag; a provided-and-mismatched one never reaches a charge. + if req.UserID == nil || *req.UserID == "" { + ownerID := cardUserID.String + insertCriticalPaymentNotification(ctx, nil, &ownerID) } } diff --git a/backend/handlers/payments/till_test.go b/backend/handlers/payments/till_test.go index 5bbae33..678962b 100644 --- a/backend/handlers/payments/till_test.go +++ b/backend/handlers/payments/till_test.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "log" + "math" "net/http" "net/http/httptest" "strings" @@ -2102,12 +2103,13 @@ func TestCreateTillSale_PendingRetry_CardMachineSwitch_Rejected(t *testing.T) { // revertGiftCardFunding — top-up guard-blocked reversal still fails the sale // ============================================================================= -// TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale locks -// the claim-first top-up guard: the gating claim succeeds (sale pending), then -// the guarded reversal is blocked because some of the top-up was already spent. -// The clawback must NOT fail (the sale still has to be marked failed) and must -// leave the card amounts untouched. -func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t *testing.T) { +// TestRevertGiftCardFunding_TopupPartiallySpent_ClampsAndFlags locks the M6 +// fix: the clawback's balance guard no longer leaves the funding in place when +// some of the top-up was already spent. The reversal CLAMPS the card to zero +// (everything still on the card is reclaimed), the sale is still marked failed, +// and the unreclaimable spent portion surfaces as a CRITICAL admin notification +// + errClawbackPartiallyReversed so every caller flags reconciliation. +func TestRevertGiftCardFunding_TopupPartiallySpent_ClampsAndFlags(t *testing.T) { ctx, tx := testutils.SetupTestTx(t) adminID, err := fixtures.CreateTestAdminUser(tx) @@ -2140,20 +2142,21 @@ func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t t.Fatalf("failed to seed gift card transaction: %v", err) } - // amount_remaining (10.00) < top-up (50.00) — the guarded UPDATE matches 0 - // rows. The clawback must NOT fail (the sale still has to be marked failed) - // and must log CRITICAL for manual reconciliation. + // amount_remaining (10.00) < top-up (50.00) — some of the funding was + // already spent. M6: the clawback must CLAMP the card to zero and return + // errClawbackPartiallyReversed (funding reverts except the unrecoverable + // spent portion), never leave the funding on the card. err = revertGiftCardFunding(ctx, "topup", cardID, 50.00, nil, saleID) - if err != nil { - t.Fatalf("revertGiftCardFunding must not fail when the guard blocks the reversal, got: %v", err) + if !errors.Is(err, errClawbackPartiallyReversed) { + t.Fatalf("expected errClawbackPartiallyReversed when the top-up was partially spent, got %v", err) } var remaining, totalAdded float64 if err := tx.QueryRow(ctx, `SELECT amount_remaining, total_funds_added FROM gift_cards WHERE id = $1`, cardID).Scan(&remaining, &totalAdded); err != nil { t.Fatalf("failed to query gift card: %v", err) } - if remaining != 10.00 || totalAdded != 50.00 { - t.Errorf("guard-blocked reversal must leave the card amounts untouched, got remaining=%.2f total_funds_added=%.2f", remaining, totalAdded) + if remaining != 0.00 || totalAdded != 0.00 { + t.Errorf("expected the partially-spent top-up clamped to zero, got remaining=%.2f total_funds_added=%.2f", remaining, totalAdded) } // The sale must still be marked failed — the whole point of the clawback. @@ -2162,11 +2165,11 @@ func TestRevertGiftCardFunding_TopupPartiallySpent_LogsCritical_StillFailsSale(t t.Fatalf("failed to query till sale: %v", err) } if saleStatus != "failed" { - t.Errorf("expected till sale marked failed despite the guard-blocked reversal, got %q", saleStatus) + t.Errorf("expected till sale marked failed despite the partial reversal, got %q", saleStatus) } - // This request's top-up transaction must be removed even though the card - // amount could not be reversed. + // This request's top-up transaction must be removed — the sale failed and + // its funding is gone from the card. var txCount int if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1 AND reference_type = 'till_sale' AND reference_id = $2`, cardID, saleID).Scan(&txCount); err != nil { t.Fatalf("failed to count gift card transactions: %v", err) @@ -2894,3 +2897,473 @@ func TestCreateTillSale_SweepResolvedMidFlight_NoResurrect(t *testing.T) { t.Errorf("expected the CRITICAL 0-rows reconciliation log, got: %s", got) } } + +// TestCreateTillSale_SavedCard_UserMismatch_Rejected locks the money-F1 fix: +// when the admin names the customer being served (user_id), the saved card MUST +// be owned by that customer. A card owned by a different user is rejected with +// 400 before any Square charge — the ownership invariant every other charge +// surface enforces, now enforced unconditionally at the till. +func TestCreateTillSale_SavedCard_UserMismatch_Rejected(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + // The customer the admin claims to be serving. + claimedUserID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create claimed user: %v", err) + } + // The customer who actually owns the card. + ownerID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create card owner user: %v", err) + } + cardID, err := fixtures.CreateTestPaymentMethod(tx, ownerID, "ccof:sq_test_card_id", "VISA", "1234") + if err != nil { + t.Fatalf("failed to create saved card: %v", err) + } + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "saved_card", + UserSavedCardID: &cardID, + UserID: &claimedUserID, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + 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/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400 (ownership mismatch), got %d. body: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "does not belong to the specified user") { + t.Errorf("expected ownership-mismatch error body, got: %s", w.Body.String()) + } + + // The rejected request must not have created a sale or funded a gift card + // (the transaction is rolled back before commit). + var saleCount int + if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM till_sales WHERE user_saved_card_id = $1`, cardID).Scan(&saleCount); err != nil { + t.Fatalf("failed to query till_sales: %v", err) + } + if saleCount != 0 { + t.Errorf("expected no till_sale for a rejected ownership mismatch, got %d", saleCount) + } + var gcCount int + if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_cards WHERE created_by = $1`, adminID).Scan(&gcCount); err != nil { + t.Fatalf("failed to query gift_cards: %v", err) + } + if gcCount != 0 { + t.Errorf("expected no gift card funded by a rejected ownership mismatch, got %d", gcCount) + } +} + +// TestCreateTillSale_SavedCard_OmittedUser_ChargesResolvedOwner locks the +// money-F1 omitted-user path: when the admin does not name a customer, the till +// still resolves the card's owner from the row and charges under it (the +// admin-till legit use — the admin may charge any customer's card). Because the +// card was never associated with a user named by the request, the charge +// surfaces a CRITICAL 'critical_payment_log' operator notification for the +// RESOLVED owner — the confused-deputy catch. +func TestCreateTillSale_SavedCard_OmittedUser_ChargesResolvedOwner(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + ownerID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create card owner user: %v", err) + } + cardID, err := fixtures.CreateTestPaymentMethod(tx, ownerID, "ccof:sq_test_card_id", "VISA", "1234") + if err != nil { + t.Fatalf("failed to create saved card: %v", err) + } + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "saved_card", + UserSavedCardID: &cardID, + // UserID intentionally omitted — the admin-till legit use. + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + 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/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201 (resolved-owner charge), got %d. body: %s", w.Code, w.Body.String()) + } + var resp TillSaleResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if resp.Status != "completed" { + t.Errorf("expected status 'completed', got '%s'", resp.Status) + } + + // No user was named, so the sale row stores no user attribution. + var saleUserID *string + if err := tx.QueryRow(ctx, `SELECT user_id FROM till_sales WHERE id = $1`, resp.ID).Scan(&saleUserID); err != nil { + t.Fatalf("failed to query till sale user: %v", err) + } + if saleUserID != nil { + t.Errorf("expected the till sale to have no user attribution (user_id omitted), got %q", *saleUserID) + } + + // The unassociated (omitted-user) charge must have raised the CRITICAL + // operator notification for the RESOLVED owner. + var notifCount int + if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM admin_notifications WHERE reason = 'critical_payment_log' AND user_id = $1 AND booking_id IS NULL`, ownerID).Scan(¬ifCount); err != nil { + t.Fatalf("failed to count admin notifications: %v", err) + } + if notifCount < 1 { + t.Errorf("expected a CRITICAL admin notification for the resolved owner, got %d", notifCount) + } +} + +// TestCreateTillSale_SavedCard_ProvidedUserMatch_NoOwnerAudit locks the +// money-F1 provided-user match path: when the admin names the customer and the +// card IS owned by them, the charge proceeds normally and no confused-deputy +// notification is raised (the card was associated with the sale's stated user). +func TestCreateTillSale_SavedCard_ProvidedUserMatch_NoOwnerAudit(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + ownerID, err := fixtures.CreateTestUser(tx) + if err != nil { + t.Fatalf("failed to create card owner user: %v", err) + } + cardID, err := fixtures.CreateTestPaymentMethod(tx, ownerID, "ccof:sq_test_card_id", "VISA", "1234") + if err != nil { + t.Fatalf("failed to create saved card: %v", err) + } + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: 50.00, + PaymentMethod: "saved_card", + UserSavedCardID: &cardID, + UserID: &ownerID, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + 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/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201 (matching-owner charge), got %d. body: %s", w.Code, w.Body.String()) + } + var resp TillSaleResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("failed to decode response: %v", err) + } + if resp.Status != "completed" { + t.Errorf("expected status 'completed', got '%s'", resp.Status) + } + + var notifCount int + if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM admin_notifications WHERE reason = 'critical_payment_log' AND user_id = $1`, ownerID).Scan(¬ifCount); err != nil { + t.Fatalf("failed to count admin notifications: %v", err) + } + if notifCount != 0 { + t.Errorf("expected no confused-deputy notification when the card owner matches the stated user, got %d", notifCount) + } +} + +// TestCreateTillSale_TopupExpiredCard_Rejected locks the money-F4 fix: the till +// top-up must reject an EXPIRED gift card with 400 and must NOT reset its +// expiry_date — the top-up UPDATE rolls expiry forward to NOW()+months and +// would otherwise resurrect a card the nightly cleanup job already forfeited. +func TestCreateTillSale_TopupExpiredCard_Rejected(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + // An expired card that still carries balance (the nightly cleanup has not + // run yet). + var cardID string + err = tx.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, expiry_date) + VALUES (50.00, 50.00, $1, FALSE, NOW() - INTERVAL '1 day') + RETURNING id + `, adminID).Scan(&cardID) + if err != nil { + t.Fatalf("failed to insert expired gift card: %v", err) + } + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "topup", + Amount: 25.00, + PaymentMethod: "on_the_house", + GiftCardID: &cardID, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + 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/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400 (expired gift card), got %d. body: %s", w.Code, w.Body.String()) + } + if !strings.Contains(w.Body.String(), "Gift card has expired") { + t.Errorf("expected expired-card error body, got: %s", w.Body.String()) + } + + // The card must be untouched: no funds added, no transaction, no sale, and + // the expiry_date must NOT be reset to the future. + var totalAdded, remaining float64 + var expiry time.Time + if err := tx.QueryRow(ctx, `SELECT total_funds_added, amount_remaining, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&totalAdded, &remaining, &expiry); err != nil { + t.Fatalf("failed to query gift card: %v", err) + } + if totalAdded != 50.00 || remaining != 50.00 { + t.Errorf("expected funds untouched (50.00/50.00), got %.2f/%.2f", totalAdded, remaining) + } + if !expiry.Before(time.Now().UTC()) { + t.Errorf("expected the expired card's expiry_date to NOT be reset (still in the past), got %v", expiry) + } + var txCount int + if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM gift_card_transactions WHERE gift_card_id = $1`, cardID).Scan(&txCount); err != nil { + t.Fatalf("failed to count gift_card_transactions: %v", err) + } + if txCount != 0 { + t.Errorf("expected no gift_card_transaction on the rejected top-up, got %d", txCount) + } + var saleCount int + if err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM till_sales WHERE item_id = $1`, cardID).Scan(&saleCount); err != nil { + t.Fatalf("failed to count till_sales: %v", err) + } + if saleCount != 0 { + t.Errorf("expected no till_sale on the rejected top-up, got %d", saleCount) + } +} + +// TestCreateTillSale_TopupUnexpiredCard_Succeeds is the positive control for +// the money-F4 expiry gate: a LIVE card can still be topped up and its expiry +// is rolled forward to NOW()+expiryMonths as designed. +func TestCreateTillSale_TopupUnexpiredCard_Succeeds(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + adminID, err := fixtures.CreateTestAdminUser(tx) + if err != nil { + t.Fatalf("failed to create admin user: %v", err) + } + adminToken := jwt.GenerateTestToken(adminID, "admin") + + var cardID string + err = tx.QueryRow(ctx, ` + INSERT INTO gift_cards (total_funds_added, amount_remaining, created_by, is_inventory, expiry_date) + VALUES (50.00, 50.00, $1, FALSE, NOW() + INTERVAL '1 year') + RETURNING id + `, adminID).Scan(&cardID) + if err != nil { + t.Fatalf("failed to insert gift card: %v", err) + } + + reqBody := TillSaleRequest{ + ItemType: "gift_card", + Action: "topup", + Amount: 25.00, + PaymentMethod: "on_the_house", + GiftCardID: &cardID, + } + bodyBytes, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/api/admin/till/sale", bytes.NewReader(bodyBytes)) + req.Header.Set("Authorization", "Bearer "+adminToken) + 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/till/sale", CreateTillSale) + r.ServeHTTP(w, req) + + if w.Code != http.StatusCreated { + t.Fatalf("expected 201 for a live-card top-up, got %d. body: %s", w.Code, w.Body.String()) + } + + var totalAdded, remaining float64 + var expiry time.Time + if err := tx.QueryRow(ctx, `SELECT total_funds_added, amount_remaining, expiry_date FROM gift_cards WHERE id = $1`, cardID).Scan(&totalAdded, &remaining, &expiry); err != nil { + t.Fatalf("failed to query gift card: %v", err) + } + if totalAdded != 75.00 || remaining != 75.00 { + t.Errorf("expected funds topped up to 75.00, got %.2f/%.2f", totalAdded, remaining) + } + if !expiry.After(time.Now().UTC()) { + t.Errorf("expected the live card's expiry to be rolled forward, got %v", expiry) + } +} + +// ============================================================================= +// F5 — £5,000/day admin gift-card cap is race-free across concurrent till sales +// ============================================================================= + +// TestCreateTillSale_DailyCap_Concurrent pins the F5 daily-cap serialization +// fix: N concurrent till gift-card sales by the same admin must never let the +// cumulative issued value exceed the £5,000/day cap. The till's cap check +// (adminGiftCardValueToday) and the sale's gift-card creation run under the +// SAME per-admin advisory lock the admin API surfaces use +// (acquireGiftCardDailyCapLock), so every check sees the previous sale's +// committed row — the excess requests are rejected. Without that per-admin lock +// two DISTINCT concurrent sales by the same admin both read the same +// pre-write cumulative value and both pass, over-issuing value (the per-sale +// crussell:till: lock serializes ONE sale's retries, not +// distinct concurrent sales). +// +// The per-transaction £250 cap bounds each till sale, so exceeding the £5,000 +// daily cap needs 21 sales of £250; a semaphore bounds how many run +// simultaneously (each handler holds one pool conn per advisory lock and one +// for its transaction). The admin and every handler invocation run directly +// against the REAL pool (no per-test transaction), so the batches exercise +// genuine cross-connection concurrency exactly like production. +func TestCreateTillSale_DailyCap_Concurrent(t *testing.T) { + ctx := context.Background() + + adminID, err := fixtures.CreateTestAdminUser(db.Conn) + if err != nil { + t.Fatalf("failed to create admin: %v", err) + } + t.Cleanup(func() { + cctx := context.Background() + _, _ = db.Conn.Exec(cctx, `DELETE FROM admin_audit_log WHERE admin_id = $1`, adminID) + _, _ = db.Conn.Exec(cctx, `DELETE FROM gift_card_transactions WHERE gift_card_id IN (SELECT id FROM gift_cards WHERE created_by = $1)`, adminID) + _, _ = db.Conn.Exec(cctx, `DELETE FROM till_sales WHERE created_by = $1`, adminID) + _, _ = db.Conn.Exec(cctx, `DELETE FROM gift_cards WHERE created_by = $1`, adminID) + _, _ = db.Conn.Exec(cctx, `DELETE FROM users WHERE id = $1`, adminID) + }) + + token := jwt.GenerateTestToken(adminID, "admin") + + const perSale = 250.00 // £250 per sale (at the £250 per-transaction cap) + const totalOps = 21 // 21 × £250 = £5,250 > the £5,000 daily cap + const concurrencyLimit = 4 // at most 4 handlers in flight (each holds 3 pool conns) + + sem := make(chan struct{}, concurrencyLimit) + start := make(chan struct{}) + var wg sync.WaitGroup + var mu sync.Mutex + successes := 0 + failCodes := map[int]int{} + for i := 0; i < totalOps; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + sem <- struct{}{} + defer func() { <-sem }() + body, _ := json.Marshal(TillSaleRequest{ + ItemType: "gift_card", + Action: "create", + Amount: perSale, + PaymentMethod: "cash", + }) + r := httptest.NewRequest(http.MethodPost, "/api/admin/till/sale", bytes.NewReader(body)) + r.Header.Set("Authorization", "Bearer "+token) + r.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + router := chi.NewRouter() + router.Use(mw.RequireAuth) + router.Post("/api/admin/till/sale", CreateTillSale) + router.ServeHTTP(w, r) + mu.Lock() + if w.Code == http.StatusCreated { + successes++ + } else { + failCodes[w.Code]++ + } + mu.Unlock() + }() + } + close(start) + wg.Wait() + + // Money-safety invariant under serialization: at most 20 of the 21 sales + // may succeed (20 × £250 = £5,000 = the inclusive cap; the 21st would land + // the day on £5,250 and must be rejected). A rejected attempt surfaces as + // either the 400 cap rejection or a 409 from the bounded try-lock giving up + // under heavy contention — both are the designed backpressure and neither + // records value. Without the per-admin cap lock each batch reads the + // pre-write cumulative value so all 21 succeed, overshooting the cap — + // `successes > 20` (or a cumulative over the cap below) is the regression + // signal this test must catch. + if successes < 1 || successes > 20 { + t.Errorf("expected between 1 and 20 of 21 concurrent till sales to succeed under the £5,000 cap, got %d (cumulative value £%.2f); failure codes: %v", successes, perSale*float64(successes), failCodes) + } + + // The day's issued value (the cap signal) must never exceed the cap. + issuedToday, err := adminGiftCardValueToday(ctx, db.Conn, adminID) + if err != nil { + t.Fatalf("failed to query today's issued value: %v", err) + } + if int64(math.Round(issuedToday*100)) > maxAdminGiftCardDailyPence { + t.Errorf("cumulative daily issued value £%.2f exceeds the £5,000 cap", issuedToday) + } + + // The created cards must hold exactly the value of the successful sales. + var totalCreated float64 + if err := db.Conn.QueryRow(ctx, `SELECT COALESCE(SUM(total_funds_added), 0) FROM gift_cards WHERE created_by = $1`, adminID).Scan(&totalCreated); err != nil { + t.Fatalf("failed to query created gift-card value: %v", err) + } + if totalCreated != perSale*float64(successes) { + t.Errorf("expected issued gift-card value £%.2f, got £%.2f", perSale*float64(successes), totalCreated) + } +}