diff --git a/backend/handlers/payments/handlers.go b/backend/handlers/payments/handlers.go index 8368017..8fda619 100644 --- a/backend/handlers/payments/handlers.go +++ b/backend/handlers/payments/handlers.go @@ -1941,6 +1941,13 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { // Step 2: DB transaction committed — safe to call Square now. // If Square fails, the record stays 'pending' for manual retry. + + // Resolve the user's email for Square receipt delivery. + var buyerEmail string + if err := db.Conn.QueryRow(r.Context(), `SELECT email FROM users WHERE id = $1`, userID).Scan(&buyerEmail); err != nil { + log.Printf("[SQUARE-PROD] Failed to resolve buyer email for user %s: %v (Square receipts will not be emailed)", userID, err) + } + paymentReq := square.CreatePaymentReq{ Amount: req.Amount, Currency: "GBP", @@ -1948,6 +1955,7 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { IdempotencyKey: idempotencyKey, ReferenceID: bookingID, Note: "tip", + BuyerEmail: buyerEmail, } paymentResult, err := SquareClient.CreatePayment(r.Context(), paymentReq) diff --git a/backend/handlers/payments/payments_test.go b/backend/handlers/payments/payments_test.go index 44494ff..e919540 100644 --- a/backend/handlers/payments/payments_test.go +++ b/backend/handlers/payments/payments_test.go @@ -15,6 +15,7 @@ import ( "crussell/clock" "crussell/db" + "crussell/internal/square" "crussell/mw" "crussell/testutils" "crussell/testutils/fixtures" @@ -22,6 +23,8 @@ import ( "github.com/go-chi/chi/v5" "github.com/jackc/pgx/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func makePaymentRequest(handler http.HandlerFunc, method, path string, body interface{}, token string, ctx context.Context) *httptest.ResponseRecorder { @@ -1858,6 +1861,59 @@ func TestTipPayment_MultipleTipsAllowed(t *testing.T) { } } +func TestTipPayment_WithSavedCard(t *testing.T) { + t.Parallel() + ctx, tx := testutils.SetupTestTx(t) + + userID, bookingID, _ := setupTestData(t, ctx, tx) + userToken := jwt.GenerateUserToken(userID) + + // Create a completed payment so the tip is allowed. + payID, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed") + require.NoError(t, err) + squarePayID := "sqp_test_saved_card" + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = $1 WHERE id = $2", squarePayID, payID) + require.NoError(t, err) + + // Register the payment in the mock so the tip flow works. + mockClient, ok := SquareClient.(*square.MockClient) + require.True(t, ok, "SquareClient must be a MockClient for this test") + _, err = mockClient.CreatePayment(ctx, square.CreatePaymentReq{ + Amount: 5000, + Currency: "GBP", + SourceID: "cnon:visa", + IdempotencyKey: "pay-for-saved-card-tip", + ReferenceID: bookingID, + }) + require.NoError(t, err) + + // Create a saved card for this user. + var savedCardID string + err = tx.QueryRow(ctx, ` + INSERT INTO user_saved_cards (user_id, square_card_id, brand, last_4, exp_month, exp_year, fingerprint) + VALUES ($1, 'ccof_mock_saved', 'VISA', '1111', 12, 2030, 'sqfp_mock_saved') + RETURNING id + `, userID).Scan(&savedCardID) + require.NoError(t, err) + + req := CreateTipPaymentRequest{ + Amount: 1000, + CardID: &savedCardID, + } + + 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, parsePaymentResponseBody(w, &resp)) + assert.Equal(t, "tip", resp.PaymentType) + assert.Equal(t, int64(1000), resp.Amount) + assert.Equal(t, "completed", resp.Status) + assert.NotEmpty(t, resp.CardBrand) + assert.NotEmpty(t, resp.CardLast4) +} + func TestTipPayment_TransactionFailure_SkipsSquare(t *testing.T) { t.Parallel() ctx, tx := testutils.SetupTestTx(t) diff --git a/backend/handlers/payments/refunds.go b/backend/handlers/payments/refunds.go index 1950bf4..f3781fa 100644 --- a/backend/handlers/payments/refunds.go +++ b/backend/handlers/payments/refunds.go @@ -6,6 +6,7 @@ import ( "log" "log/slog" "math" + "strconv" "time" "crussell/clock" @@ -540,10 +541,12 @@ func ProcessPendingSquareRefunds(ctx context.Context, bookingID string, reason s } refundCents := int64(math.Round(pr.Amount * 100)) + // Deterministic idempotency key derived from refund record ID and amount, + // so retries (network timeout, proxy, etc.) don't create duplicate Square refunds. refundReq := square.RefundPaymentReq{ PaymentID: *pr.SquarePaymentID, Amount: refundCents, - IdempotencyKey: pr.ID + "-square-" + clock.Now().Format("20060102150405"), + IdempotencyKey: pr.ID + "-square-" + strconv.FormatInt(refundCents, 10), Reason: reason, } sqResult, sqErr := SquareClient.RefundPayment(ctx, refundReq) diff --git a/backend/handlers/payments/validators.go b/backend/handlers/payments/validators.go index e1ddaa0..6f001b2 100644 --- a/backend/handlers/payments/validators.go +++ b/backend/handlers/payments/validators.go @@ -53,9 +53,15 @@ func ValidateRefundReason(reason string) error { return nil } -// ValidateCardInfo checks that at least one of cardID or newCardToken is provided +// ValidateCardInfo checks that exactly one of cardID or newCardToken is provided, +// non-nil, and non-empty. func ValidateCardInfo(cardID, newCardToken *string) error { - if cardID == nil && (newCardToken == nil || *newCardToken == "") { + hasCardID := cardID != nil && *cardID != "" + 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") } return nil diff --git a/backend/internal/square/square_dev.go b/backend/internal/square/square_dev.go index 1952b07..695b4d0 100644 --- a/backend/internal/square/square_dev.go +++ b/backend/internal/square/square_dev.go @@ -463,3 +463,10 @@ func isAllDigits(s string) bool { } return len(s) > 0 } + +func realBaseURL(env string) string { + if env == "production" { + return squareProductionURL + } + return squareSandboxURL +} diff --git a/backend/internal/square/square_http_client.go b/backend/internal/square/square_http_client.go index e492acd..bbed742 100644 --- a/backend/internal/square/square_http_client.go +++ b/backend/internal/square/square_http_client.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "time" ) @@ -339,8 +340,15 @@ func refundPaymentHTTP(ctx context.Context, req RefundPaymentReq) (*RefundResult func createCardOnFileHTTP(ctx context.Context, userID, cardToken string) (*CardOnFile, error) { hc := newHTTPClient() + + // Deterministic idempotency key derived from user + card (not time-based) + // so that retries with the same details don't create duplicate cards. + ikHash := fmt.Sprintf("%x", []byte(userID+"|"+cardToken)) + if len(ikHash) > 64 { + ikHash = ikHash[:64] + } body := sqCreateCardRequest{ - IdempotencyKey: fmt.Sprintf("create-card-%d", time.Now().UnixNano()), + IdempotencyKey: fmt.Sprintf("create-card-%s", ikHash), SourceID: cardToken, Card: sqCardPayload{ CustomerID: userID, @@ -358,7 +366,7 @@ func createCardOnFileHTTP(ctx context.Context, userID, cardToken string) (*CardO func getCardsOnFileHTTP(ctx context.Context, userID string) ([]CardOnFile, error) { hc := newHTTPClient() var resp sqListCardsResponse - if err := hc.doJSON(ctx, http.MethodGet, "/v2/cards?customer_id="+userID, nil, &resp); err != nil { + if err := hc.doJSON(ctx, http.MethodGet, "/v2/cards?customer_id="+url.QueryEscape(userID), nil, &resp); err != nil { return nil, err } cards := make([]CardOnFile, 0, len(resp.Cards)) @@ -416,8 +424,11 @@ func paymentFromSquare(sq *sqPayment) *PaymentResult { r.ExpYear = cd.Card.ExpYear } } - if r.CardBrand == "" { - r.CardBrand = sq.SourceType + if r.CardBrand == "" && sq.CardDetails != nil && sq.CardDetails.Card.ID != "" { + r.CardBrand = sq.CardDetails.Card.CardBrand + } + if r.CardLast4 == "" && sq.CardDetails != nil && sq.CardDetails.Card.ID != "" { + r.CardLast4 = sq.CardDetails.Card.Last4 } return r } @@ -479,10 +490,3 @@ func firstNonEmpty(vals ...string) string { } return "" } - -func realBaseURL(env string) string { - if env == "production" { - return squareProductionURL - } - return squareSandboxURL -} diff --git a/frontend/src/lib/components/account/UserBookingModal.svelte b/frontend/src/lib/components/account/UserBookingModal.svelte index 879719e..a996fa0 100644 --- a/frontend/src/lib/components/account/UserBookingModal.svelte +++ b/frontend/src/lib/components/account/UserBookingModal.svelte @@ -199,8 +199,10 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; const isTipNewCardExpiryPast = $derived( tipNewCardExpiryParts !== null && (() => { - const expiryDate = new SvelteDate(tipNewCardExpiryParts.year, tipNewCardExpiryParts.month); - return expiryDate < new SvelteDate(); + const expiryYearMonth = tipNewCardExpiryParts.year * 12 + tipNewCardExpiryParts.month; + const now = new SvelteDate(); + const currentYearMonth = now.getFullYear() * 12 + now.getMonth() + 1; + return expiryYearMonth < currentYearMonth; })() ); const hasTipNewCardInvalidMonth = $derived(/^\d{2}\/\d{2}$/.test(tipNewCardExpiry) && tipNewCardExpiryParts === null); @@ -322,8 +324,6 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; if (tipShowNewCard || tipSavedCards.length === 0) { body.new_card_token = tipNewCardNumber.replace(/\s/g, ''); - body.card_expiry = tipNewCardExpiry; - body.card_cvc = tipNewCardCVC; body.save_card = tipSaveCardFuture; } else { body.card_id = tipSelectedCardId; diff --git a/frontend/src/lib/components/booking/BookingFlow.svelte b/frontend/src/lib/components/booking/BookingFlow.svelte index 07f5467..c1a020c 100644 --- a/frontend/src/lib/components/booking/BookingFlow.svelte +++ b/frontend/src/lib/components/booking/BookingFlow.svelte @@ -136,7 +136,7 @@ const cardError = $derived( cardNumberTouched && !isValidLuhn(newCardNumber) && newCardNumber.length > 0 ? 'Invalid card number' - : cardExpiryTouched && expiryParts !== null && (() => { const d = new SvelteDate(expiryParts.year, expiryParts.month); return d < new SvelteDate(); })() + : cardExpiryTouched && expiryParts !== null && (() => { const em = expiryParts.year * 12 + expiryParts.month; const now2 = new SvelteDate(); const cm = now2.getFullYear() * 12 + now2.getMonth() + 1; return em < cm; })() ? 'This card has expired' : cardExpiryTouched && !/^\d{2}\/\d{2}$/.test(newCardExpiry) && newCardExpiry.length > 0 ? 'Enter expiry as MM/YY' diff --git a/frontend/src/lib/components/payments/UserPaymentModal.svelte b/frontend/src/lib/components/payments/UserPaymentModal.svelte index 9c59a29..525620c 100644 --- a/frontend/src/lib/components/payments/UserPaymentModal.svelte +++ b/frontend/src/lib/components/payments/UserPaymentModal.svelte @@ -79,8 +79,10 @@ const isExpiryInPast = $derived( expiryParts !== null && (() => { - const expiryDate = new SvelteDate(expiryParts.year, expiryParts.month); - return expiryDate < new SvelteDate(); + const expiryYearMonth = expiryParts.year * 12 + expiryParts.month; + const now = new SvelteDate(); + const currentYearMonth = now.getFullYear() * 12 + now.getMonth() + 1; + return expiryYearMonth < currentYearMonth; })() ); diff --git a/frontend/src/routes/account/+page.svelte b/frontend/src/routes/account/+page.svelte index 4b4f55c..65b053b 100644 --- a/frontend/src/routes/account/+page.svelte +++ b/frontend/src/routes/account/+page.svelte @@ -201,8 +201,10 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; const isNewCardExpiryInPast = $derived( newCardExpiryParts !== null && (() => { - const expiryDate = new SvelteDate(newCardExpiryParts.year, newCardExpiryParts.month); - return expiryDate < new SvelteDate(); + const expiryYearMonth = newCardExpiryParts.year * 12 + newCardExpiryParts.month; + const now = new SvelteDate(); + const currentYearMonth = now.getFullYear() * 12 + now.getMonth() + 1; + return expiryYearMonth < currentYearMonth; })() ); const isNewCardExpiryInvalidMonth = $derived( @@ -230,8 +232,10 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; const isBuyNewCardExpiryInPast = $derived( buyNewCardExpiryParts !== null && (() => { - const expiryDate = new SvelteDate(buyNewCardExpiryParts.year, buyNewCardExpiryParts.month); - return expiryDate < new SvelteDate(); + const expiryYearMonth = buyNewCardExpiryParts.year * 12 + buyNewCardExpiryParts.month; + const now = new SvelteDate(); + const currentYearMonth = now.getFullYear() * 12 + now.getMonth() + 1; + return expiryYearMonth < currentYearMonth; })() ); const isBuyNewCardExpiryInvalidMonth = $derived( diff --git a/frontend/src/routes/pay-tip/[id]/+page.svelte b/frontend/src/routes/pay-tip/[id]/+page.svelte index b2d6b47..bff064d 100644 --- a/frontend/src/routes/pay-tip/[id]/+page.svelte +++ b/frontend/src/routes/pay-tip/[id]/+page.svelte @@ -112,8 +112,10 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; const isNewCardExpiryPast = $derived( newCardExpiryParts !== null && (() => { - const expiryDate = new SvelteDate(newCardExpiryParts.year, newCardExpiryParts.month); - return expiryDate < new SvelteDate(); + const expiryYearMonth = newCardExpiryParts.year * 12 + newCardExpiryParts.month; + const now = new SvelteDate(); + const currentYearMonth = now.getFullYear() * 12 + now.getMonth() + 1; + return expiryYearMonth < currentYearMonth; })() ); const hasNewCardInvalidMonth = $derived(/^\d{2}\/\d{2}$/.test(newCardExpiry) && newCardExpiryParts === null); @@ -306,8 +308,6 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; if (showNewCardForm || savedCards.length === 0) { body.new_card_token = newCardNumber.replace(/\s/g, ''); - body.card_expiry = newCardExpiry; - body.card_cvc = newCardCVC; body.save_card = saveCardForFuture; } else { body.card_id = selectedCardId; diff --git a/frontend/src/routes/tip/+page.svelte b/frontend/src/routes/tip/+page.svelte index 6dd6379..bcefdc2 100644 --- a/frontend/src/routes/tip/+page.svelte +++ b/frontend/src/routes/tip/+page.svelte @@ -136,8 +136,12 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; const isNewCardExpiryPast = $derived( newCardExpiryParts !== null && (() => { - const expiryDate = new SvelteDate(newCardExpiryParts.year, newCardExpiryParts.month); - return expiryDate < new SvelteDate(); + // parseExpiryParts returns months 1-indexed (1=Jan, 12=Dec). + // Use year-month arithmetic to avoid Date constructor 0-index confusion. + const expiryYearMonth = newCardExpiryParts.year * 12 + newCardExpiryParts.month; + const now = new SvelteDate(); + const currentYearMonth = now.getFullYear() * 12 + now.getMonth() + 1; + return expiryYearMonth < currentYearMonth; })() ); const hasNewCardInvalidMonth = $derived(/^\d{2}\/\d{2}$/.test(newCardExpiry) && newCardExpiryParts === null); @@ -253,8 +257,6 @@ import CardBrandIcon from '$lib/components/payments/CardBrandIcon.svelte'; if (showNewCardForm || savedCards.length === 0) { body.new_card_token = newCardNumber.replace(/\s/g, ''); - body.card_expiry = newCardExpiry; - body.card_cvc = newCardCVC; body.save_card = saveCardForFuture; } else { body.card_id = selectedCardId; diff --git a/obsidian/Crussell/Future Work - Gap Backlog.md b/obsidian/Crussell/Future Work - Gap Backlog.md index 63b03d9..a2c69d7 100644 --- a/obsidian/Crussell/Future Work - Gap Backlog.md +++ b/obsidian/Crussell/Future Work - Gap Backlog.md @@ -33,8 +33,9 @@ These are things that work fine in dev (with mocks) but need real implementation | P6 | **Email/SMS notification delivery** | XL (5-7d) | Backend | `user_notification_preferences` table stores delivery preferences. 8 TODO markers reference this blocker. Notification creation works (admin_notifications table), but no delivery channel exists. No SMTP configuration, no SMS provider. 2 tests skipped as "WIP handler." | The notification queue works (reasons, priorities, acknowledging). What's missing is the delivery backend. Affects: slot eviction alerts, edit request approvals/denials, gift card codes, unpaid booking reminders, idle account warnings. | | P7 | **Production security headers** | S (1h) | Backend | HSTS and Referrer-Policy headers are commented out in `main.go:231-233` with TODO markers. They were left disabled for dev HTTP convenience. | Uncomment and configure for production. | | P8 | **Social auth stubs (Google/Microsoft/Facebook)** | L (2-3d) | Backend + Frontend | `handlers/auth/social.go` is 1 line (`package auth`). Frontend login page has 3 social buttons that show `toast.info("${provider} login coming soon")`. The `user_social_logins` table and `account_type` enum values exist from early schema design. | The schema was designed for social auth from the start (table + enum values). The OAuth flow itself was never implemented. Buttons exist as UI placeholders. | -| P9 | **Tip payments: replace placeholder card tokens** | S (1d) | Frontend | 3 tip endpoints send `card_token: 'placeholder'` — a literal string. (`UserBookingModal:190`, `tip/+page.svelte:141`, `pay-tip/[id]/+page.svelte:170`). The Square Web Payments token from the frontend card form was never wired. | The tip UI flow works (select amount, confirm). The actual card token from Square's payment form was never plumbed through. Needs to capture the real nonce/token and pass it to the backend. | +| P9 | **Tip payments: replace placeholder card tokens** | S (1d) | Frontend | ✅ COMPLETED July 2026 — `card_token: 'placeholder'` replaced with real saved card selection + CardInput with Luhn/expiry/CVC validation across all 3 tip pages. | | | P10 | **No automated database backups** | M (1d) | Infrastructure | PostgreSQL volume is persistent in Docker but no `pg_dump` cron, no point-in-time recovery. | Standard production DB setup task. | +| P11 | **Square Web Payments SDK: replace CardInput with nonce-based flow** | M (3-5d) | Frontend + Backend | Frontend still sends raw PAN, expiry, and CVC as `new_card_token` for all card entry flows (tips, booking payment, gift cards, account add card, till purchases). In production, Square's API requires a `cnon:xxx` nonce generated by the Web Payments SDK. The mock (`square_dev.go`) parses raw PANs (detecting brand from first digit), masking this failure in development. | **Action plan:** 1) Load Square Web Payments SDK (script tag in `app.html` or via `@square/web-payments-sdk` npm). 2) Replace `CardInput.svelte` (hand-rolled inputs) with Square's native card form (`payments.card()`). 3) Call `card.tokenize()` to get `cnon:xxx` nonce client-side. 4) Send only the nonce as `new_card_token`. 5) Remove `card_expiry`/`card_cvc` from request bodies (already removed from tip flows). 6) Remove `CreateCardOnFileRaw` from production paths. | --- @@ -103,11 +104,16 @@ These don't add features but reduce maintenance cost and risk. | T10 | **Replace `as any` in HolidayHours** | S (30min) | Frontend | `HolidayHours.svelte:234` — `(group.hours as any[])?.map(…)`. Hours array has known shape. | | T11 | **Replace `e: any` in button onclick** | S (30min) | Frontend | `button.svelte:101` — click handler typed as `e: any`. | | T12 | **Former name display (4 TODO sites)** | S (1d) | Frontend + Backend | 4 TODOs across GiftCards + notifications needing `previousFirstName`/`previousLastName` from backend. | -| T13 | **Fix `devProdClient` rune-arithmetic in test** | S (30min) | Backend | `square_dev_test.go:410` — `rune('0'+idx)` breaks for indices >= 10 (`:` not a digit). | +| T13 | **Fix `devProdClient` rune-arithmetic in test** | S (30min) | Backend | ✅ COMPLETED July 2026 — `rune('0'+idx)` replaced with `fmt.Sprintf("concurrent-key-%d", idx)` for proper numeric formatting beyond index 9. | | T14 | **Error tracking / monitoring (Sentry)** | M (1-2d) | Backend | `log.Printf()` only. No alerting on 5xx. 39 ALERT + 11 CRITICAL logs will never be seen. | --- +## Previously Completed Items (July 2026 backlog) + +- ~~**Tip payments: replace placeholder card tokens (P9)** — `card_token: 'placeholder'` replaced with real saved card selection + CardInput + Luhn/expiry/CVC validation across all 3 tip pages. CardBrandIcon SVGs added for all Square-supported brands.~~ +- ~~**Fix `devProdClient` rune-arithmetic in test (T13)** — `rune('0'+idx)` replaced with `fmt.Sprintf("concurrent-key-%d", idx)`.~~ + ## Previously Completed Items (June 2026 backlog) - ~~Reservation/cleanup background cron~~ — All 20 jobs migrated to centralized scheduler