From 3db8b54923bf614863c4665c47a4739ffe57cc4c Mon Sep 17 00:00:00 2001 From: Stephen Adamson Date: Fri, 31 Jul 2026 13:37:28 +0100 Subject: [PATCH] Fix silent tip/gift-card money loss on pending retry; terminal checkout wire; sentinel error; docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL — same-amount tip retry silently never charged: - CreateTipPayment idempotency check now only short-circuits when the existing record is 'completed'. A 'pending' record (previous Square call failed) is REUSED and the charge re-attempted with the same key (Square dedups safely), instead of returning the stale pending record as 200 with a success toast and no charge. - Same fix in BuyGiftCard: pending records trigger a re-attempt, not a false-success response. Unique idempotency_key constraint means the pending record must be reused, not re-inserted. - Fixes the savepoint/rollback interaction: the nested tx (savepoint) is now committed in the reuse path so the deferred rollback doesn't undo the later status UPDATE on the same connection. - Regression test: TestTipPayment_RetryPending_ReattemptsCharge verifies a pending record + same-key retry re-attempts and completes, reusing the record (count stays 1). MAJOR — terminal checkout wire contract: - device_id now sent as checkout.device_options.device_id (Square's required shape), not a top-level field which Square rejects with 400. - 'checkout pending' detection now uses typed sentinel ErrCheckoutPending with errors.Is in both handlers, matching mock and real HTTP client. MAJOR — exp_month/exp_year omitted from card creation payload when unset (now *int with omitempty) — Square would 400 on 0/0; expiry comes from the tokenized source. Docs: - README payments/infrastructure sections corrected (Web Payments SDK claim replaced with accurate P11-backlog note; dev mock parity described) - Future Work P11 updated to reflect raw-PAN rejection is now enforced in both mock and prod (new-card flows are a documented dead end) - Added plans/p11-square-web-payments-sdk.md: full implementation plan + handoff prompt for the agent picking up P11 (Web Payments SDK nonces) --- README.md | 4 +- backend/handlers/payments/giftcards.go | 102 ++++++++----- backend/handlers/payments/handlers.go | 72 ++++++--- backend/handlers/payments/payments_test.go | 56 +++++++ backend/handlers/payments/till.go | 2 +- backend/internal/square/square_dev.go | 2 +- backend/internal/square/square_http_client.go | 34 +++-- .../Crussell/Future Work - Gap Backlog.md | 2 +- .../plans/p11-square-web-payments-sdk.md | 144 ++++++++++++++++++ 9 files changed, 342 insertions(+), 76 deletions(-) create mode 100644 obsidian/Crussell/plans/p11-square-web-payments-sdk.md diff --git a/README.md b/README.md index 040ba2a..ee877cc 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Nail salon booking platform — Go 1.26.5 backend + SvelteKit 5 SPA + PostgreSQL **Booking**: Self-service (customer), walk-in (admin), call-in (admin). Slot reservations prevent double-booking (4 TTL types). **Self-blocking prevention**: `excludeUserID` parameter filters a user's own `RESERVATION` entries from time blocker overlap checks, allowing re-reservation and booking at overlapping slots. **Explicit cancellation**: `DELETE /api/bookings/reserve` releases a user reservation; `DELETE /api/admin/bookings/reserve` releases an admin walk-in/call-in reservation. **Background cleanup**: Centralised cron scheduler (`backend/internal/jobs/`) runs 21 maintenance jobs: reservation/deposit cleanup every 5min, hourly campaign transitions, daily unpaid-booking notifications, staged default hours auto-apply, GDPR anonymization, financial aggregation, and token/code cleanup. Guest accounts with GDPR-compliant anonymization (including `RESERVATION:edit_request:%` scrubbing). Service eligibility based on age + patch test validity. Overlap checks use `FOR UPDATE` row locks inside transactions. Closing-hours validation (`closing_time.go`) resolves both current and staged default hours. -**Payments**: Square Terminal (in-person) + Web Payments SDK (online). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A PostgreSQL `pg_advisory_lock` serializes payment attempts per-booking to prevent two-tab double-payment races. Gift card purchases now insert a pending payment record with VAT before calling Square — the DB transaction commits first, so Square failures leave a retryable pending record. +**Payments**: Square Terminal (in-person, via `CreateTerminalCheckout`) + online card payments (currently raw-PAN entry in dev only; production nonce integration is backlog item P11 — see `obsidian/Crussell/Future Work - Gap Backlog.md`). Cash with change calculation. Gift cards (12-digit code or account balance). Saved cards for faster checkout. Tips on completed bookings. Refunds with notice-period tiers and deposit protection (72h/24h thresholds). All payment types: deposit, full, partial, balance, tip. Payment >20% of total promotes `pending_release` bookings back to `confirmed`. Deposit paid is computed from payments on-the-fly. The first 50% of each payment is always carved out as deposit (via `buildSplitRecords`); any overflow beyond the booking total becomes a tip. A PostgreSQL `pg_advisory_lock` serializes payment attempts per-booking to prevent two-tab double-payment races. Gift card purchases insert a pending payment record with VAT before calling Square — the DB transaction commits first, so Square failures leave a retryable pending record (same-key retries reuse it). **Gift Cards**: Multi-method purchase (cash, card machine, online card, giveaway). Inventory cards for stock management. 24-month rolling expiry. Idle account cleanup (2yr/5yr thresholds). Expired balance recovery with admin audit trail. Transaction audit log. Idempotency keys for purchases. @@ -22,7 +22,7 @@ Nail salon booking platform — Go 1.26.5 backend + SvelteKit 5 SPA + PostgreSQL **Frontend**: Portfolio gallery with fuzzy tag search (relevance-sorted) and exact category filters (date-sorted), multi-format images (AVIF/WebP/JPEG/JXL with WASM client-side encoding), cursor-based pagination. MapLibre GL map on contact page. PhoneInput component with UK validation. CharCounter for long notes. -**Infrastructure**: Docker Compose (postgres, backend, sabredav, nginx). Dev mock for Square payments (`//go:build dev`). RustFS dev storage, Cloudflare R2 for prod. SabreDAV CardDAV sync for profile photos. +**Infrastructure**: Docker Compose (postgres, backend, sabredav, nginx). Dev mock for Square payments (`//go:build dev`) that mirrors production PCI-DSS behaviour (rejects raw PANs; accepts `cnon:`/`ccof:` tokens only). RustFS dev storage, Cloudflare R2 for prod. SabreDAV CardDAV sync for profile photos. **Middleware**: `JsonContentType` sets `Content-Type: application/json` globally, replacing ~80+ individual `w.Header().Set()` calls. `RespondJSON`/`RespondError` helpers standardise API response format. Progressive rate limiting (dual-window) on login/register with account lockout. diff --git a/backend/handlers/payments/giftcards.go b/backend/handlers/payments/giftcards.go index 37e9924..9ae74d2 100644 --- a/backend/handlers/payments/giftcards.go +++ b/backend/handlers/payments/giftcards.go @@ -891,16 +891,27 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { paymentService := NewPaymentService() + // Idempotency: only short-circuit when the existing record is 'completed'. + // A 'pending' record means the previous Square call failed — returning it + // as 200 would show a success without ever charging. Re-attempt below with + // the same key (Square dedups safely) and reuse the pending record. + reusePendingID := "" if req.IdempotencyKey != "" { existing, err := paymentService.CheckIdempotencyByKey(ctx, req.IdempotencyKey) if err != nil { log.Printf("Failed to check idempotency: %v", err) } if existing != nil { - if err := json.NewEncoder(w).Encode(existing); err != nil { - log.Printf("Failed to encode JSON response: %v", err) + if existing.Status == "completed" { + if err := json.NewEncoder(w).Encode(existing); err != nil { + log.Printf("Failed to encode JSON response: %v", err) + } + return + } + if existing.Status == "pending" { + reusePendingID = existing.ID + log.Printf("[PAYMENTS] Reusing pending payment %s for idempotent gift-card retry (key %s)", existing.ID, req.IdempotencyKey) } - return } } @@ -958,43 +969,62 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) { }() var buyPaymentID string - fees := paymentService.CalculateFees(req.Amount, "online") - record := PaymentRecord{ - PaymentType: "full", - PaymentMethod: "online_square", - Status: "pending", - Amount: amountPounds, - SquarePaymentID: nil, - IdempotencyKey: &req.IdempotencyKey, - Fees: float64(fees) / 100.0, - UserSavedCardID: savedCardID, - CreatedAt: clock.Now(), - UpdatedAt: clock.Now(), - CreatedBy: &userID, - } - err = tx.QueryRow(ctx, ` - INSERT INTO payments (payment_type, payment_method, status, amount, square_payment_id, idempotency_key, fees, user_saved_card_id, created_by, created_at, updated_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) - RETURNING id - `, record.PaymentType, record.PaymentMethod, record.Status, record.Amount, record.SquarePaymentID, record.IdempotencyKey, record.Fees, record.UserSavedCardID, record.CreatedBy, record.CreatedAt, record.UpdatedAt).Scan(&buyPaymentID) - if err != nil { - log.Printf("Failed to insert payment record: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } + if reusePendingID != "" { + // Reusing the pending record from a failed prior attempt — do not + // insert a duplicate (idempotency_key is UNIQUE). Proceed straight to + // the Square call, which dedups on the same key. + buyPaymentID = reusePendingID + } else { + fees := paymentService.CalculateFees(req.Amount, "online") + record := PaymentRecord{ + PaymentType: "full", + PaymentMethod: "online_square", + Status: "pending", + Amount: amountPounds, + SquarePaymentID: nil, + IdempotencyKey: &req.IdempotencyKey, + Fees: float64(fees) / 100.0, + UserSavedCardID: savedCardID, + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), + CreatedBy: &userID, + } + err = tx.QueryRow(ctx, ` + INSERT INTO payments (payment_type, payment_method, status, amount, square_payment_id, idempotency_key, fees, user_saved_card_id, created_by, created_at, updated_at) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) + RETURNING id + `, record.PaymentType, record.PaymentMethod, record.Status, record.Amount, record.SquarePaymentID, record.IdempotencyKey, record.Fees, record.UserSavedCardID, record.CreatedBy, record.CreatedAt, record.UpdatedAt).Scan(&buyPaymentID) + if err != nil { + log.Printf("Failed to insert payment record: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } - // Apply VAT to the pending payment - vatCfg, vatErr := GetVATConfig(ctx, tx) - if vatErr == nil && vatCfg.IsVATRegistered && vatCfg.VoucherType == "SPV" { - if _, vatExecErr := tx.Exec(ctx, "SELECT apply_vat_to_payment($1, $2)", buyPaymentID, vatCfg.DefaultVATRate); vatExecErr != nil { - log.Printf("Failed to apply VAT to buy gift card payment %s: %v", buyPaymentID, vatExecErr) + // Apply VAT to the pending payment + vatCfg, vatErr := GetVATConfig(ctx, tx) + if vatErr == nil && vatCfg.IsVATRegistered && vatCfg.VoucherType == "SPV" { + if _, vatExecErr := tx.Exec(ctx, "SELECT apply_vat_to_payment($1, $2)", buyPaymentID, vatCfg.DefaultVATRate); vatExecErr != nil { + log.Printf("Failed to apply VAT to buy gift card payment %s: %v", buyPaymentID, vatExecErr) + } + } + + if err := tx.Commit(ctx); err != nil { + log.Printf("Failed to commit buy transaction: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return } } - if err := tx.Commit(ctx); err != nil { - log.Printf("Failed to commit buy transaction: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return + // Commit the (nested/savepoint) transaction in the reuse path too. It is + // empty, but committing releases the savepoint so the deferred rollback + // becomes a no-op — otherwise that rollback would undo the status UPDATE + // executed later on the same connection. + if reusePendingID != "" { + if err := tx.Commit(ctx); err != nil { + log.Printf("Failed to commit buy transaction (reuse): %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } } // Step 2: DB transaction committed — safe to call Square now. diff --git a/backend/handlers/payments/handlers.go b/backend/handlers/payments/handlers.go index 45b8d7b..63637fb 100644 --- a/backend/handlers/payments/handlers.go +++ b/backend/handlers/payments/handlers.go @@ -626,7 +626,7 @@ func GetCheckoutStatus(w http.ResponseWriter, r *http.Request) { paymentResult, err := SquareClient.GetCheckout(r.Context(), checkoutID) if err != nil { - if err.Error() == "checkout pending" { + if errors.Is(err, square.ErrCheckoutPending) { if err := json.NewEncoder(w).Encode(PaymentStatusResponse{Status: "PENDING"}); err != nil { log.Printf("Failed to encode JSON response: %v", err) } @@ -1900,18 +1900,30 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { } }() - // Check idempotency inside the transaction — same pattern as CreateBookingPayment. + // Check idempotency inside the transaction. + // Only short-circuit when the existing record is 'completed'. A 'pending' + // record means the previous Square call failed — returning it as 200 would + // show a success toast without ever charging. Re-attempt the charge below + // with the same idempotency key (Square dedups safely) and reuse the + // existing record. var existingID sql.NullString var existingBookingID sql.NullString var existingPaymentType sql.NullString var existingStatus sql.NullString var existingAmount sql.NullFloat64 var existingCreatedAt sql.NullTime - if err := tx.QueryRow(r.Context(), ` + + err = tx.QueryRow(r.Context(), ` SELECT id, booking_id, payment_type, status, amount, created_at FROM payments WHERE booking_id = $1 AND idempotency_key = $2 - `, bookingID, idempotencyKey).Scan(&existingID, &existingBookingID, &existingPaymentType, &existingStatus, &existingAmount, &existingCreatedAt); err == nil { + `, bookingID, idempotencyKey).Scan(&existingID, &existingBookingID, &existingPaymentType, &existingStatus, &existingAmount, &existingCreatedAt) + + paymentID := "" + reusePendingRecord := false + switch { + case err == nil && existingStatus.String == "completed": + // Idempotent dedup — return the already-completed payment. if err := json.NewEncoder(w).Encode(PaymentResponse{ ID: existingID.String, BookingID: existingBookingID.String, @@ -1923,32 +1935,42 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) { log.Printf("Failed to encode JSON response: %v", err) } return - } else if !errors.Is(err, pgx.ErrNoRows) { + case err == nil && existingStatus.String == "pending": + // Previous Square call failed — reuse the pending record and re-attempt. + paymentID = existingID.String + reusePendingRecord = true + case err != nil && !errors.Is(err, pgx.ErrNoRows): log.Printf("Failed to check tip idempotency: %v", err) } - record := PaymentRecord{ - BookingID: bookingID, - PaymentType: "tip", - PaymentMethod: "online_square", - Status: "pending", - Amount: float64(req.Amount) / 100.0, - IdempotencyKey: &idempotencyKey, - Fees: 0, - UserSavedCardID: savedCardID, - CreatedAt: clock.Now(), - UpdatedAt: clock.Now(), - CreatedBy: &userID, + if !reusePendingRecord { + record := PaymentRecord{ + BookingID: bookingID, + PaymentType: "tip", + PaymentMethod: "online_square", + Status: "pending", + Amount: float64(req.Amount) / 100.0, + IdempotencyKey: &idempotencyKey, + Fees: 0, + UserSavedCardID: savedCardID, + CreatedAt: clock.Now(), + UpdatedAt: clock.Now(), + CreatedBy: &userID, + } + + paymentID, err = service.CreatePaymentRecordTx(r.Context(), tx, record, nil) + if err != nil { + log.Printf("Failed to create payment record: %v", err) + http.Error(w, "internal server error", http.StatusInternalServerError) + return + } + ApplyVATToBookingPayment(r.Context(), tx, paymentID) } - paymentID, err := service.CreatePaymentRecordTx(r.Context(), tx, record, nil) - if err != nil { - log.Printf("Failed to create payment record: %v", err) - http.Error(w, "internal server error", http.StatusInternalServerError) - return - } - ApplyVATToBookingPayment(r.Context(), tx, paymentID) - + // Always commit the (possibly nested/savepoint) transaction. In the reuse + // path the savepoint is empty, but committing it releases the savepoint so + // the deferred rollback becomes a no-op — otherwise that rollback would + // undo the status UPDATE executed later on the same connection. if err := tx.Commit(r.Context()); err != nil { log.Printf("Failed to commit transaction: %v", err) http.Error(w, "internal server error", http.StatusInternalServerError) diff --git a/backend/handlers/payments/payments_test.go b/backend/handlers/payments/payments_test.go index 651fc35..001db0d 100644 --- a/backend/handlers/payments/payments_test.go +++ b/backend/handlers/payments/payments_test.go @@ -1933,6 +1933,62 @@ func TestTipPayment_WithSavedCard(t *testing.T) { assert.NotEmpty(t, resp.CardLast4) } +func TestTipPayment_RetryPending_ReattemptsCharge(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_retry" + _, err = tx.Exec(ctx, "UPDATE payments SET square_payment_id = $1 WHERE id = $2", squarePayID, payID) + 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_retry', 'VISA', '1111', 12, 2030, 'sqfp_mock_retry') + RETURNING id + `, userID).Scan(&savedCardID) + require.NoError(t, err) + + // Simulate a failed prior attempt: a PENDING tip record with the same + // idempotency key the frontend will send on retry. The handler must NOT + // short-circuit on this — it must re-attempt the Square charge. + key := "tip-retry-key-123" + _, err = tx.Exec(ctx, ` + INSERT INTO payments (booking_id, payment_type, payment_method, status, amount, idempotency_key, created_at, updated_at, created_by) + VALUES ($1, 'tip', 'online_square', 'pending', 10.00, $2, NOW(), NOW(), $3) + `, bookingID, key, userID) + require.NoError(t, err) + + req := CreateTipPaymentRequest{ + Amount: 1000, + CardID: &savedCardID, + IdempotencyKey: key, + } + + 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, "completed", resp.Status, "retry of a pending record must re-attempt and complete, not return the stale pending record") + + // Exactly one payment record for this key, now completed. + var count int + var status string + err = tx.QueryRow(ctx, `SELECT COUNT(*), MAX(status) FROM payments WHERE idempotency_key = $1`, key).Scan(&count, &status) + require.NoError(t, err) + assert.Equal(t, 1, count, "must reuse the pending record, not insert a duplicate") + assert.Equal(t, "completed", status) +} + func TestTipPayment_TransactionFailure_SkipsSquare(t *testing.T) { t.Parallel() ctx, tx := testutils.SetupTestTx(t) diff --git a/backend/handlers/payments/till.go b/backend/handlers/payments/till.go index 8550a82..f751c43 100644 --- a/backend/handlers/payments/till.go +++ b/backend/handlers/payments/till.go @@ -522,7 +522,7 @@ func GetTillCheckoutStatus(w http.ResponseWriter, r *http.Request) { paymentResult, err := SquareClient.GetCheckout(r.Context(), checkoutID) if err != nil { - if err.Error() == "checkout pending" { + if errors.Is(err, square.ErrCheckoutPending) { if err := json.NewEncoder(w).Encode(PaymentStatusResponse{Status: "PENDING"}); err != nil { log.Printf("Failed to encode JSON response: %v", err) } diff --git a/backend/internal/square/square_dev.go b/backend/internal/square/square_dev.go index 67f1525..76c508f 100644 --- a/backend/internal/square/square_dev.go +++ b/backend/internal/square/square_dev.go @@ -261,7 +261,7 @@ func (m *MockClient) GetCheckout(ctx context.Context, checkoutID string) (*Payme } if checkout.Status == "PENDING" { - return nil, fmt.Errorf("checkout pending") + return nil, ErrCheckoutPending } result, ok := m.completed[checkoutID] diff --git a/backend/internal/square/square_http_client.go b/backend/internal/square/square_http_client.go index e61923e..517ab7b 100644 --- a/backend/internal/square/square_http_client.go +++ b/backend/internal/square/square_http_client.go @@ -5,6 +5,7 @@ import ( "context" "crypto/sha256" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -13,6 +14,12 @@ import ( "time" ) +// ErrCheckoutPending is returned by GetCheckout when a terminal checkout has +// not yet completed. Handlers use errors.Is(err, ErrCheckoutPending) rather +// than string comparison, so behaviour is identical across the mock and the +// real HTTP client. +var ErrCheckoutPending = errors.New("checkout pending") + // --------------------------------------------------------------------------- // Square REST API constants. // --------------------------------------------------------------------------- @@ -175,14 +182,18 @@ type sqFee struct { type sqTerminalCheckoutRequest struct { IdempotencyKey string `json:"idempotency_key"` Checkout sqTerminalCheckoutPayload `json:"checkout"` - DeviceID string `json:"device_id,omitempty"` } type sqTerminalCheckoutPayload struct { - AmountMoney sqMoney `json:"amount_money"` - ReferenceID string `json:"reference_id,omitempty"` - Note string `json:"note,omitempty"` - CustomerID string `json:"customer_id,omitempty"` + AmountMoney sqMoney `json:"amount_money"` + ReferenceID string `json:"reference_id,omitempty"` + Note string `json:"note,omitempty"` + CustomerID string `json:"customer_id,omitempty"` + DeviceOptions *sqDeviceOptions `json:"device_options,omitempty"` +} + +type sqDeviceOptions struct { + DeviceID string `json:"device_id"` } type sqTerminalCheckoutResponse struct { @@ -238,8 +249,8 @@ type sqCreateCardRequest struct { } type sqCardPayload struct { - ExpMonth int `json:"exp_month"` - ExpYear int `json:"exp_year"` + ExpMonth *int `json:"exp_month,omitempty"` + ExpYear *int `json:"exp_year,omitempty"` CardholderName string `json:"cardholder_name,omitempty"` CustomerID string `json:"customer_id,omitempty"` } @@ -295,7 +306,9 @@ func createCheckoutHTTP(ctx context.Context, req CreateCheckoutReq) (*CheckoutRe Note: req.Note, CustomerID: req.CustomerID, }, - DeviceID: req.DeviceID, + } + if req.DeviceID != "" { + body.Checkout.DeviceOptions = &sqDeviceOptions{DeviceID: req.DeviceID} } var resp sqTerminalCheckoutResponse if err := hc.doJSON(ctx, http.MethodPost, "/v2/terminals/checkouts", body, &resp); err != nil { @@ -312,6 +325,9 @@ func getCheckoutHTTP(ctx context.Context, checkoutID string) (*PaymentResult, er } tc := tcResp.Checkout if tc.Status != "COMPLETED" { + if tc.Status == "PENDING" || tc.Status == "IN_PROGRESS" { + return nil, ErrCheckoutPending + } return nil, fmt.Errorf("square: checkout %s is %s (not COMPLETED)", checkoutID, tc.Status) } if len(tc.PaymentIDs) == 0 { @@ -351,8 +367,6 @@ func createCardOnFileHTTP(ctx context.Context, userID, cardToken string) (*CardO SourceID: cardToken, Card: sqCardPayload{ CustomerID: userID, - ExpMonth: 0, - ExpYear: 0, }, } var resp sqCreateCardResponse diff --git a/obsidian/Crussell/Future Work - Gap Backlog.md b/obsidian/Crussell/Future Work - Gap Backlog.md index a2c69d7..43b106f 100644 --- a/obsidian/Crussell/Future Work - Gap Backlog.md +++ b/obsidian/Crussell/Future Work - Gap Backlog.md @@ -35,7 +35,7 @@ These are things that work fine in dev (with mocks) but need real implementation | 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 | ✅ 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. | +| 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 now mirrors production and rejects raw PANs (`CreateCardOnFileRaw` is blocked; `CreateCardOnFile` accepts only `cnon:`/`ccof:` tokens) — so these new-card flows are currently a dead end in BOTH dev and prod until this is landed or the UI is gated. | **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. | --- diff --git a/obsidian/Crussell/plans/p11-square-web-payments-sdk.md b/obsidian/Crussell/plans/p11-square-web-payments-sdk.md new file mode 100644 index 0000000..75e78da --- /dev/null +++ b/obsidian/Crussell/plans/p11-square-web-payments-sdk.md @@ -0,0 +1,144 @@ +# P11 — Square Web Payments SDK Implementation Plan + +**Status:** READY TO PICK UP (deferred from July 2026 session) +**Owner:** Agent implementing P11 (Square Web Payments SDK) +**Estimated effort:** 3-5 days +**Backlog reference:** `Future Work - Gap Backlog.md` item P11 + +--- + +## Executive Summary + +Every "enter a new card" flow in the app is currently a **dead end**. The frontend sends raw PAN, expiry, and CVC as `new_card_token` with no Square Web Payments SDK tokenization. The backend mock AND production both now reject raw PANs (PCI-DSS parity — `CreateCardOnFileRaw` is blocked, `CreateCardOnFile` accepts only `cnon:`/`ccof:` tokens). So a user entering a new card gets a guaranteed 500. **This plan makes new-card payments actually work** by integrating Square's Web Payments SDK client-side to generate `cnon:xxx` nonces. + +--- + +## Current State (verified July 2026) + +### Frontend — 6 flows send raw PAN as `new_card_token`: +1. `frontend/src/routes/tip/+page.svelte` (~line 283) — `body.new_card_token = newCardNumber.replace(/\s/g, '')` +2. `frontend/src/routes/pay-tip/[id]/+page.svelte` (~line 331) — same +3. `frontend/src/lib/components/account/UserBookingModal.svelte` (~line 353) — same (tip modal) +4. `frontend/src/lib/components/payments/UserPaymentModal.svelte` (~line 348) — booking payment +5. `frontend/src/lib/components/booking/BookingFlow.svelte` (~line 359) — deposit +6. `frontend/src/routes/account/+page.svelte` (~line 332) — Buy a Gift Card + +Plus raw PAN + CVC to add-card (`account/+page.svelte:577` → `CreatePaymentMethodFromDetails` → `CreateCardOnFileRaw`) and admin till (`GiftCardsManagement.svelte:655` → `till.go:439` → `CreateCardOnFileRaw`). + +### The reusable `CardSelection.svelte` component: +`frontend/src/lib/components/payments/CardSelection.svelte` — the standard card-selection UI (saved card list + "Use a new card" + `CardInput` with blur-based Luhn/expiry/CVC validation). **Currently applied to only 1 of 5 card UIs** (UserPaymentModal). The tip flows, account Buy Gift Card, account Add Card, and BookingFlow still have ~100 duplicated lines each. + +### `CardInput.svelte`: +`frontend/src/lib/components/payments/CardInput.svelte` — the hand-rolled card entry form (number/expiry/CVC inputs, formatNumber/formatExpiry, onfieldblur/onfieldinput callbacks). **This is what gets replaced by Square's native card form.** + +### Backend (already P11-ready): +- `backend/internal/square/square_http_client.go` — `createCardOnFileHTTP` accepts a `source_id` token and calls `POST /v2/cards`. Works with `cnon:xxx` nonces. +- `backend/handlers/payments/handlers.go` — `CreateTipPayment` accepts `new_card_token` and passes it as the source. Works with nonces. +- `CreatePaymentMethodFromDetails` (service.go:496) calls `CreateCardOnFileRaw` — **needs migration to the nonce path**. +- Till `online_square` (till.go:439) calls `CreateCardOnFileRaw` — **needs migration to the nonce path**. + +--- + +## Prerequisites + +1. **Square application credentials**: + - `SQUARE_APPLICATION_ID` (client-side, public) + - `SQUARE_LOCATION_ID` (already used server-side) + - Frontend needs the application ID in the browser context (e.g. `PUBLIC_SQUARE_APPLICATION_ID` Vite env var) +2. **Square account with Web Payments enabled** and a card processing merchant account. +3. Frontend must be HTTPS (or localhost) for the SDK to load. + +--- + +## Implementation Steps + +### Step 1 — Load the Square Web Payments SDK + +Two options (pick one): +- **npm**: `@square/web-payments-sdk` — provides `Square.payments(appId, locationId)` +- **script tag**: `` in `app.html` (sandbox) or `https://web.squarecdn.com/v1/square.js` (prod) + +Load based on `SQUARE_ENVIRONMENT` so sandbox/prod use the right URL. + +### Step 2 — Create a Square card form component + +Replace `CardInput.svelte`'s hand-rolled inputs with Square's native card form: +```js +const payments = window.Square.payments(appId, locationId); +const card = await payments.card(); +await card.attach('#square-card-container'); +// ...on submit: +const tokenResult = await card.tokenize(); +// tokenResult.token → "cnon:xxx" +``` + +**Design decision**: Either: +- (a) Embed the Square form inside `CardInput.svelte` (keep the `bind:cardNumber` etc. API surface but use Square's iframe internally — the inputs become read-only display), OR +- (b) Create a new `SquareCardInput.svelte` and swap it into `CardSelection.svelte` when a Square app ID is configured, falling back to the hand-rolled form when `SQUARE_APPLICATION_ID` is absent (keeps dev/testing working without Square). + +**Recommendation**: (b) — a `SquareCardInput.svelte` with a fallback. This keeps local dev usable when no Square app ID is configured, while production uses real tokenization. + +### Step 3 — Update the 6 payment flows to send nonces + +For each flow, `new_card_token` must become the `cnon:xxx` token from `card.tokenize()`, NOT the raw PAN. Since all 6 flows go through `CardSelection.svelte` (or the fallback path), the cleanest approach: + +1. First **extend `CardSelection.svelte` to all 5 remaining card UIs** (tip ×3, account Buy Gift Card, account Add Card, BookingFlow) — this centralises the card-selection logic so P11's change is one place, not six. +2. Then swap the card entry inside `CardSelection.svelte` to use `SquareCardInput` (Step 2b). +3. Remove `card_expiry`/`card_cvc` from all request bodies (already removed from tip flows). + +### Step 4 — Migrate the two raw-PAN backend paths + +- `CreatePaymentMethodFromDetails` (account add-card): change `CreateCardOnFileRaw` call to use the nonce path (`CreateCardOnFile` with the `cnon:xxx` token). +- Till `online_square` (`till.go:439`): same migration — `CreateCardOnFileRaw` → `CreateCardOnFile` with a nonce. The frontend `GiftCardsManagement.svelte` till flow must send the nonce instead of raw PAN. + +### Step 5 — Remove `CreateCardOnFileRaw` entirely + +Once both callers are migrated: +- Delete `CreateCardOnFileRaw` from the `SquareClient` interface (`backend/internal/square/types.go:140`) +- Delete from all 3 implementations (MockClient, ProdClient, devProdClient) +- Delete the PCI-block error stubs +- Update the `square_dev_test.go` tests that assert the raw-PAN rejection (`TestDevClient_CreateCardOnFileRaw_Rejected_ProdParity` → the method no longer exists) +- Update handler tests that assert the 500 on raw-PAN add-card + +### Step 6 — Update docs + +- `README.md` line 9: remove the "P11 — production nonce integration is backlog item P11" caveat once landed +- `Future Work - Gap Backlog.md` P11: mark completed +- `Feature Catalog.md` and `Technical Manual.md`: update "Web Payments SDK (online)" claims if they reference the old state + +--- + +## Testing Plan + +1. **Unit tests**: + - `square_http_client_test.go`: add request-shape tests via `httptest.Server` for `createCardOnFileHTTP` (verify `source_id` is the token, `idempotency_key` deterministic, no raw PAN in body) — the HTTP client currently has only 2 tests (both `paymentFromSquare`). + - Error-path tests for the HTTP client (non-2xx, malformed body). +2. **Integration tests**: + - Handler tests using `cnon:` tokens through the mock (the mock accepts `cnon:` nonces) — restore success-path coverage for `TestCreatePaymentMethod_HappyPath` etc. (currently rewritten to assert the raw-PAN 500). + - Add a test: card created with nonce → payment with saved card works. +3. **Manual/sandbox tests** (requires Square sandbox credentials): + - Each of the 6 flows: enter new card → tokenize → pay → verify charge in Square dashboard. + - Saved-card flow still works. + - Refund still works. + +--- + +## Risks / Gotchas + +- **Square iframe requires HTTPS** — localhost is exempt, but any non-local dev URL needs TLS. +- **Tokenization is one-shot** — a `cnon:` nonce is single-use. The idempotency key logic (cached per payment attempt) handles retries, but a retry must NOT re-tokenize if the first tokenize succeeded and the payment failed — the backend's idempotency dedup handles this, but the frontend should reuse the cached token on retry if the payment record is pending. +- **The hand-rolled `CardInput.svelte` Luhn/expiry validation becomes cosmetic** — Square's iframe does the real validation. Keep the display validation for UX, but don't block submission on it alone. +- **PCI-DSS scope**: with nonces, PAN never touches our server. The `CreateCardOnFileRaw` migration is essential — do NOT leave it in place. + +--- + +## Definition of Done + +- [ ] Square Web Payments SDK loads (sandbox + prod URLs, env-gated) +- [ ] `SquareCardInput` (or embedded form) tokenizes cards → `cnon:xxx` +- [ ] All 6 payment flows send nonces, not raw PANs +- [ ] `CardSelection.svelte` used by all 5 card UIs +- [ ] `CreateCardOnFileRaw` deleted from interface + all implementations +- [ ] All handler tests pass with nonce-based flows +- [ ] Sandbox smoke test: new-card payment succeeds end-to-end +- [ ] Docs updated