3130b39c77a5d73fdab212842f9b6e133811f17c
322
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 popertots
|
4e398a7a2b |
fix: round-2 loop-B adversarial (503c326 baseline) — B1 webhook race, APPROVED refund semantics, notification cap single-source, 2FA cooldown/StateFor hardening, register bcrypt semaphore
Round 2 Loop B red-team (money/security/dup-mod adversarial) findings on the full payments overhaul: MONEY: - HIGH: webhook COMPLETED promotion now resolves the B1 parent row (mirrors the re-poll resolveB1ParentFailed + till-sale clawback) — the sweep no longer re-replays an expired key into stacked unauthorized charges - HIGH: A6 deposit-with-discount clamp — chargeAmount capped to max(0, remaining-discount) for ALL discount cases; overflow guard compares against the discounted remaining - MED-HIGH: APPROVED refunds treated as NON-terminal at the webhook (event-driven, may still fail); payments call sites aligned; FAILED can now demote an APPROVED-then-failed row - MED: B1 refund transport-error fails the row + CRITICAL immediately (no 3-charge stacking) - MED: till_sales capped-fail surfaces the outstanding funding (gift_card_transactions trace) for manual reversal - MED: guest-bookings cash/gift-card terminal charges now audited (NULL target); audit reordered post-commit; cancellation refunds audited - MED: A6 no-discount skip-path returns campaign_fully_redeemed 400 (no success-shaped no-op); skip-path writes a marker row for idempotency SECURITY: - HIGH: notification cap centralized in adminnotify (MaxUnacknowledgedCriticalLogs) + applied at ALL insert sites (webhooks x2, jwt refresh_token_reuse, account erasure, sweep, twofa) with suppressed-insert logging; per-issue bucket for reissue alerts - MED-HIGH: twofa.StateFor saturated state made IMMUTABLE (LastMintAt writes are no-ops; no cross-user throttling); eviction never drops in-window count>0 records - MED: /register now uses the shared bcrypt semaphore (authBcryptSlots, 20) — botnet CPU burn bounded - MED: NAT collateral reduced (429-reject only at top progressive tier; lower tiers sleep) - MED: ClearMintCooldownForUser exposed for fresh-charge success; reissue cooldown-skip raises a capped alert - LOW: audit coverage gaps (reschedule fee forgiveness, gift-card transfer, clawback) closed DUP/MOD: - Frontend deposit-percent literals -> POLICY constants (10 sites); LOYALTY_DISCOUNT_RATE single-sourced; generateUUID adopted; admin PaymentModal overflow-tip confirm path added; £500 gift-card cap named Verified: 26/26 dev + 24/24 prod (CI condition), both vet tags, frontend tests+build, env-docs 42/42. |
||
|
popertots
|
3866cc5963 |
fix: round-2 loop-A fresh review (503c326 baseline) — B1 replay cap, A6 discount record, 2FA reissue+cooldown, notification flood, lockout saturation, VAT/refund-status consolidation
Round 2 Loop A fresh money/security/dup-mod review. 23 findings fixed: MONEY: - CRITICAL: B1 duplicate auto-refund gains an attempt cap (b1_attempts col, cap 3) — a rejected auto-refund no longer re-replays the expired key every sweep run (which minted a stacking unauthorized charge each time); FAILED-webhook demotion respects the cap; never re-replay a key whose B1 refund failed - HIGH: A6 deposit_covered_by_discount skip path now APPLIES the eligible campaign discount rows immediately (capped) instead of skipping with no discount recorded — no more promised-discount-not-recorded overcharge - MEDIUM: 2FA code burned by the SAVE gate is re-issued on failed new-card+save_card charges (re-issue guard now covers req.SaveCard) - LOW: GetBookingPaymentSummary excludes tip rows from paidAmount (remaining now matches the authoritative tip-excluded balance) SECURITY: - MEDIUM: unacknowledged CRITICAL admin-notification flood capped (global cap on critical_payment_log + refresh_token_reuse rows) - MEDIUM: 2FA reissue no longer bypasses the mint cooldown (Check no longer clears LastMintAt on gate-verify; cleared on terminal charge success) - MEDIUM: twofa.StateFor map-saturation returns a shared permanently-locked state instead of a fresh 5-guess budget per request - MEDIUM: ProgressiveRateLimit rejects 429 past maxProgressiveSleepDelayMs instead of sleeping unboundedly; login bcrypt concurrency semaphore added - LOW: loginInProgress 409->429; webhook key-set/URL-unset startup check; email-verification per-user attempt counter DUP/MOD: - formatCurrency single source (frontend format.ts, 7 files consolidated); SquareRefundStatusToLocal single source (errors.go, all sites); admin audit-log helper dedup; SCA retry model unified (proactive on all 6 surfaces); buyDailyTotal/daily-cap mirror via backend; lock TTL from backend; generateUUID at all card-form sites; magic numbers named (defaultPostgresHost, epsilon, fee constants); admin CASH + gift-card terminal charges now audited; DAV_SKIP_INIT documented in manuals Verified: 26/26 dev + 24/24 prod (GO_TESTING=1, the CI condition), both vet tags, frontend tests+build, env-docs 42/42. |
||
|
popertots
|
36887167c6 |
fix: loop-A fresh review (503c326 baseline) — overflow-guard bypass, discounted-deposit retry, GDPR audit scrub, till cap, sweep rescue, 2FA reissue + SCA retry, consolidation round
Loop A fresh money/security/dup-mod review of the whole payments overhaul. 28 consolidated findings fixed:
MONEY:
- HIGH-1: B12 overflow guard now uses the discounted obligation — a pre-start deposit can never mint an unintended tip; the discount is never truncated to £0 when the customer pays the discounted deposit
- HIGH-2: discounted-deposit pending-reuse retry compares pendingStoredAmountPence vs chargeAmount (the actual Square amount), not req.Amount — no more permanent amount_mismatch 400 on lost-response retries
- MEDIUM-3: sweep rescue now carves overflow as a tip record + runs completion side-effects (was booking overflow as service revenue, skipping completion)
- MEDIUM-4 (shared w/ security): admin_audit_log.admin_id made nullable + anonymize_user/delete_guest_user NULL it + scrub details.card_last4 — 2fa_fallback_charge PII no longer survives account deletion
- MEDIUM-5: till gift-card payment now passes the £5,000/day admin cap (giftcard_limits)
- LOW-6: expired gift-card balance surfaced as expired/zero in GetUserGiftCardBalance
SECURITY:
- 2FA single-use consume made atomic at verify time for all 5 saved-card gates (fresh charges consume; pending-reuse retries don't); deferred consumption removed
- reissueTwoFACodeAfterFailedCharge routed through the fail-closed issuance gate (pepper check, cooldown) + fresh-only semantics (only when a code was actually consumed)
- family-alive cache invalidated on the stale-family cleanup DELETE (no 30s warm window after expiry)
- frontend 503-retry no longer reuses a consumed 2FA code — aligns with backend re-issue
DUP/MOD:
- reissue helper single-sourced (5 call sites), squareRefundStatusToLocal (10 inline switches), writeChargeSnapshot (7 sites, immutability guard on gift-card/till), postChargeRecheck (3+1 sites), scanIdempotencySlot (2), applyVATToChargeRecord (3 patterns), user_saved_cards upsert (2), BuyGiftCard pending INSERT via service
- till completed-dedup now re-validates paymentHasLiveRefund (aligns with booking/tip/gift-card)
- frontend 402 idempotency-key regeneration added to PaymentModal (aligns with other CIT surfaces)
- PAYMENT_METHOD_SAVED_CARD constant standardised ('saved_card' everywhere)
- admin audit coverage added for AdminRefundBooking + gift-card buy/top-up
- audit-helper cross-package dedup (user/twofa.go now calls payments' exported insert)
Verified: 26/26 dev + 24/24 prod packages, both vet tags, frontend tests + build, gitleaks clean.
|
||
|
popertots
|
b7122be3a0 |
fix: SCA review round + gitea pipeline green — GDPR audit scrub, backend test gaps, frontend SCA/Square-API, docs parity
7 review agents (pipeline run, self-review, codebase-context, frontend-placement, backend testing-gaps, Square-API, docs-parity) audited the SCA-primary work. ALL findings fixed, including every pre-existing red CI job: GDPR (HIGH): - anonymize_user() now scrubs admin_audit_log.target_user_id (mirrors delete_guest_user) so 2fa_fallback_charge rows (customer id + card_last4 PII) no longer survive registered-user account deletion; gdpr test added BACKEND TEST GAPS (all 10): - delivery-unavailable 503 branch: prod-tag predicate test + dev-variant marker - twoFactorFallbackEnabled alias/case/default matrix tests + exported wrapper - insertTwoFAFallbackAudit details-JSON shape + audit-row assertions for all 6 gate sites (booking/tip/gift-card/payment-method/terminal/till, both actors) - CreateTerminalPayment.VerificationToken: passthrough, too-long 400, 2FA-skip, token-less fallback + SCA-required (new terminal_sca_test.go) - isVerificationRequiredError at all 5 charge sites (402 + code:verification_required) - customer_initiated handler-level assertions (MIT false admin / CIT true customer) - Mock: ApprovePendingVerification, ChallengeResult auto/deny, _deny token suffix, parseVerifyToken unit tests FRONTEND SCA + Square-API (CRITICAL): - tokenizeSavedCardWithVerification reads result.token (the verified token) not result.verificationResult (deprecated verifyBuyer shape — saved-card SCA could never succeed in production before); parseTokenizeVerificationResult pure fn extracted + pinned in square.test.ts; 'verified' with no token proceeds tokenless - HIGH: saved-card idempotency key regenerated after a definitive 402 (fresh token under the same key = IDEMPOTENCY_KEY_REUSED dead-loop); kept on 503/cancelled - challenge-cancelled copy no longer promises a 2FA fallback the UI doesn't show; 'waiting for approval in your banking app' state on CIT surfaces - sca-unavailable demotion resets per attempt; card selection disabled mid-challenge; genuine saved-card declines no longer relabeled 'requires verification'; modal-close guard during processing; retry affordance standardized PIPELINE (every red job now green): - prod-tag build break fixed (shared square stub + test_helpers_test.go, prod-safe) - govulncheck: x/image 0.45.0 bumped (x/text resolved); go mod tidy clean - race: TestDeleteAccount_InvalidatesSquareCustomerCache made deterministic - DAV_ADMIN_PASSWORD placeholder in .env.example (compose config passes) - frontend: prettier 28 files, eslint, a11y 38 errors, knip (currentZIndex), deps in-range, audit vulns (nanoid/postcss) — all fixed; 67 vitest cases DOCS PARITY (6 DRIFTs + 5 GAPs): payments doc Ch4/Ch14/Appendix A, Technical Manual 2FA + counter-reset + payment sections, README test counts + SNAPSHOT_ENC_KEY, Feature Catalog, .env.example REQUIRE_2FA — SCA-primary/2FA-backup posture verified against code everywhere Verified: 26/26 dev + 24/24 prod packages, both vet tags, golangci-lint/staticcheck/ gosec 0 on both tags, gitleaks clean, 2,464 backend + 67 frontend tests. |
||
|
popertots
|
c4c65d9dd8 |
feat: proactive saved-card SCA — challenge runs BEFORE the first charge, never a naked ccof attempt
Square's card.tokenize(verificationDetails, squareCardId) determines the SCA requirement UP FRONT and returns a fresh verification_token (or an explicit outcome), so the customer-initiated saved-card flow now runs it before the first charge attempt instead of the reactive 'attempt naked ccof -> 402 verification_required -> challenge + retry' round-trip. - UserPaymentModal/TipPayment/BookingFlow/account gift-card buy: call runSavedCardSCAProactively before charging; 'verified' carries the token on attempt #1; 'sca-unavailable' demotes to the 2FA gate (the only tokenless path); 'challenge-cancelled'/'sca-failed' never charge and keep the pending row retryable with the same cached idempotency key - The reactive re-challenge hook is removed; a defensive verification-required 402 (stale/consumed token) surfaces VERIFICATION_REQUIRED_MESSAGE and lets the user retry - Admin PaymentModal + till saved-card charges remain MERCHANT-INITIATED (customer_initiated=false, SCA-exempt, no liability shift) — unchanged - square.ts comments updated (saved-card charges now carry a token proactively; SAVED_CARD_VERIFICATION_MESSAGE is the defensive path) - Tests: 98 frontend tests (proactive decision coverage); build clean |
||
|
popertots
|
5dae0bba08 |
feat: Square 3DS2 SCA primary authorisation for saved-card charges; 2FA demoted to audited backup
SCA is now the PRIMARY authorisation for saved-card (ccof) charges (PSR 2017 /
chargeback liability shift); the homegrown 2FA becomes a BACKUP used only when
SCA is unavailable (e.g. a bank without in-app approval), with a strict audit
trail. The 'approve in your banking app' UX comes from Square buyer
verification. Email/SMS remains the intended 2FA delivery channel; the [2FA]
stdout-log relay (TWO_FACTOR_ALLOW_LOG_DELIVERY=true) is the explicit-insecure
pre-email/SMS stopgap.
BACKEND:
- CreateTerminalPaymentRequest gains VerificationToken (forwarded to Square in
the admin saved-card branch; validated like the other charge handlers)
- Structured SCA-required error surfacing: isVerificationRequiredError +
writeVerificationRequiredResponse (HTTP 402 with {code:'verification_required'})
at all 5 charge error sites — the frontend keys on it to trigger the challenge
- requireTwoFactorForCardAccess reworked: SCA token present => 2FA skipped
(SCA primary); no token => 2FA fallback requires delivery channel + consume +
insertTwoFAFallbackAudit (admin_audit_log reason 2fa_fallback_charge,
{sca_performed:false,...}); TWO_FACTOR_FALLBACK env flag (default true) gates
the fallback; false => SCA-only posture
- MIT vs CIT: admin till saved-card + admin booking saved-card charges now flag
customer_initiated=false (merchant-initiated, no SCA, no liability shift);
customer-initiated online flows keep true
FRONTEND:
- square_card_id threaded through SavedCard/SelectableCard + admin lists
- isVerificationRequiredSignal + shouldFallbackTo2FA helpers (402 + code / text
fallback); VERIFICATION_REQUIRED_MESSAGE
- tokenizeSavedCardWithVerification (Square SDK tokenize(details, squareCardId))
with verified/challenge-cancelled/sca-unavailable/sca-failed outcomes
- Per-surface SCA retry with the SAME idempotency key + fresh verification_token
(booking/tip/till/gift-card/admin); 'waiting for approval in your banking
app' state on admin surfaces; 2FA backup-only UX in the shared composable
MOCK PARITY:
- SimulateSavedCardVerificationRequired toggle (default off) + grandfathering
- Challenge state (ApprovePendingVerification/DenyPendingVerification,
ChallengeResult config, token-encoded _ok|_deny outcome)
- One-time-use verify_mock_ token ledger + amount/source binding
- MockCardForm saved-card verification simulation + mock Approve button
- Tests: saved-card SCA gate, one-time-use, denied, amount-mismatch,
grandfathered; frontend helper tests
DOCS: payments-doc SCA appendix, Technical Manual 2FA section, README,
Overview, Feature Catalog updated to SCA-primary + 2FA-backup; env-var
documented (42/42).
26/26 backend packages; 95/95 frontend tests + build; env-docs 42/42.
|
||
|
popertots
|
1543160f6a |
fix: loop-B full-scope adversarial findings — tip-excluded detail endpoints, £0-charge guard, 24h window, 2FA single-use everywhere
Loop B full-scope red-team (money/security/dup-mod) findings: - CRITICAL: booking detail handlers (GetBookingHandler/GetAdminBookingHandler) now exclude payment_type='tip' from amount_paid — a tip before the final balance no longer undercharges the booking (bookings.go x3 sites) - HIGH: A6 deposit clamp adds a zero-guard — when the eligible discount covers the entire deposit, the flow returns deposit_covered_by_discount instead of charging £0 at Square (real Square rejects £0; the mock accepted it); square_dev CreatePayment + CreateRefund now reject Amount <= 0 (mock/prod parity) - HIGH: replayLegitimateRetryWindow restored to 22h (== stalePendingKeyedAge) so sweep-produced duplicate charges are still auto-refunded, not rescued-and-hidden - HIGH: 2FA single-use consume-at-gate applied to ALL saved-card charge gates (booking 2263, admin saved-card 960, tip 4483, till 967, gift-card purchase 1482) with re-issue-on-failed-charge on each; pending-reuse retries keep their code - MEDIUM: 2FA re-issue now fires only when the gate actually consumed a code (fresh saved-card path) — new-card failures no longer silently burn a standing code - MEDIUM: pre_start tip-exclusion consistent across admin lists + detail handlers (bookings.go) - MEDIUM: remaining-balance counts pending refunds (service.go) — capacity consistent with GetBookingPaymentInfo - Mock CreatePayment/CreateRefund reject £0 amounts (INVALID_REQUEST_ERROR) for dev/prod parity 26/26 backend packages; 80/80 frontend tests + build; env-docs 41/41. |
||
|
popertots
|
9a182db932 |
fix: full-scope review — tip-inclusive amount_due, sweep deposit-strand, A6 clamp cap, B13 clawback, 2FA single-use, mint audit, account-deletion re-auth, refresh dedup
Full-scope Loop A restart review (18 findings across money/security/dup-mod): MONEY: - HIGH: amount_paid/amount_due CTEs now exclude payment_type='tip' (bookings.go x6, today.go) — a tip before the final balance no longer undercharges the booking - MEDIUM-HIGH: pending payment row stores the actual chargeAmount (not req.Amount) so the sweep replay amount-match rescues deposit-with-discount rows instead of auto-refunding them; refundSweepDuplicateCharge refunds the replayed payment's actual amount - MEDIUM: A6 deposit clamp-up now caps at the discounted obligation (remainingPence - eligibleDiscountPence) — no more silent overcharge when a campaign discount >= deposit - MEDIUM: B13 campaign-loss balance credits are clawed back on cancellation (clawbackB13CampaignCredit in ProcessCancellationRefundTx) - LOW: replayLegitimateRetryWindow extended 22h->24h so a legitimate same-key retry in the retry-eligible window is rescued, not auto-refunded SECURITY: - 2FA single-use strengthened (consume-at-gate for fresh charges, re-issue on failure) - Admin 2FA mint now writes admin_audit_log + logs code reuse - Account deletion requires current password (and 2FA when enforced) — stolen token can no longer destroy the account - Multi-tab refresh-token replay deduped via cross-tab lock (no false family-kill alerts) - family-alive cache invalidated on password change / GDPR erasure - Login lockout keyed per user+IP with a capped ceiling FRONTEND/DUP-MOD: - OverflowTipConfirm shared component (UserPaymentModal + BookingFlow); overflow computation aligned (deposit-discount-aware) - PaymentModal admin 2FA gate now method-conditioned (no over-reveal on cash/giftcard) - requestTwoFactorCode shared helper (requestNewTwoFactorCode + adminRequestNewTwoFactorCode) - BookingFlow deposit display aligned to the discounted amount; formatCurrency used consistently 26/26 backend packages; 80/80 frontend tests + build; env-docs 41/41. |
||
|
popertots
|
b46927336b |
fix: dup/mod secondary round — till 2FA gate parity, mint-cooldown single source, notification parity, pence comments
Loop B dup/mod attack findings:
- TillPurchases admin 2FA gate now mirrors PaymentModal and the backend: gateActive = twoFactorEnforced && customerTwoFactorEnabled && paymentMethod='saved_card'; the customer's setup flag is fetched from GET /api/admin/users/{id} on selection. A 2FA-disabled customer in an enforced env no longer hits a dead-end blocked input — the charge 403 surfaces the actionable message via the existing self-heal.
- Extracted twoFAMintThrottled helper shared by SetupTwoFAHandler and ensurePendingTwoFACode — mint-cooldown rule can no longer drift between setup and disable-flow paths
- Notification-helper drift documented: sweep copy states the intentional booking+user scoping vs the canonical webhook copy (cross-referenced); auth refresh_token_reuse insert verified to carry the same NOT EXISTS acknowledged_at IS NULL guard; no import cycle (webhooks→payments one-way)
- Pence convention: 'rounded to the cent' corrected to 'pence' (handlers.go:2726)
26/26 backend packages; 72/72 frontend tests + build; env-docs 41/41.
|
||
|
popertots
|
7c424b28b8 |
fix: loop-B adversarial findings — tip-type double-charge, tip-refund capacity, loyalty stamp farming, gate ordering, auth amplification, admin audit log
Loop B restart (money/security/dup-mod adversarial) fixes: - CRITICAL: CreateTerminalPayment rejects payment_type='tip' (mirrors CreateBookingPayment) — a tip-typed admin charge no longer records the FULL amount as a tip and double-collects (all is-paid computations exclude tip rows) - HIGH: tip refunds can no longer re-open booking capacity — refunded_total subqueries filter payment_type <> 'tip' (service.go) and RefundPayment rejects tip rows - MEDIUM: loyalty-stamp farming closed — stamp award once-per-booking via loyalty_stamp_awarded_at column (init-script.sql) + existing same-day guard - MEDIUM: CreateTipPayment/CreateBookingPayment 2FA gates moved AFTER the idempotency completed-dedup (code consumed only on new money paths; terminal path already correct) — lost-response retries return the completed payment instead of 400 - MEDIUM: replayRescueLowerBoundSkew widened to 5m (DB-clock-skew stranded originals now rescued) - MEDIUM-1: verifyFamilyAlive DB amplification reduced via 30s bounded family-alive cache; admin route group rate-limited - MEDIUM-3: admin saved-card charges now write admin_audit_log (handlers.go helper + till); [2FA] log line decoupled from user identity - LOW-1: logout scoped to the presented token's family (no cross-session kill) - LOW-2: refresh-reuse grace widened for same-IP replays - LOW-4: squareEnvironmentMismatch enforced for empty env - LOW-5: uuid.ts hard-fails on Math.random fallback (crypto.randomUUID) - Cash/giftcard tip-enabled overflow mirrors the card-terminal carve 26/26 backend packages; 72/72 frontend tests + build; env-docs 41/41. |
||
|
popertots
|
03d85c6d13 |
fix: admin-scoped 2FA mint targets the CUSTOMER — user authentication for saved cards, never the admin
The till and admin payment modal 'Request a new code' buttons previously called
the session-scoped POST /api/user/2fa/code, which mints a code for the ADMIN's
session — a code that can never satisfy the card-owner gate and is delivered to
the admin's log line, not the customer.
- New POST /api/admin/users/{id}/2fa/code (AdminSendVerificationCodeHandler,
RequireAdmin + per-user limiter): mints/reuses a code for the TARGET user
(the card owner/customer), keyed to the CUSTOMER's userID so the [2FA]
delivery log carries the customer's ID — the customer, never the admin, is
the authentication subject for their card
- Shared useTwoFactorCodeForSavedCard composable gains an optional mint()
option; admin surfaces (PaymentModal, TillPurchases) pass the customer-scoped
mint, customer surfaces keep the session default
- Frontend: adminRequestNewTwoFactorCode(userID) in square.ts; PaymentModal
mints for booking.user_id, TillPurchases for selectedCustomer.id
- Tests: admin mint keys the code to the customer's userID (log line contains
customer ID, NOT the admin ID) + pending hash persisted for the customer;
unknown target user 404s
Backend 26/26 packages; frontend 72/72 + build clean.
|
||
|
popertots
|
6d00c3004f |
fix: convert admin PaymentModal to shared useTwoFactorCodeForSavedCard composable (verification round FAIL)
The verification of 62adccd found the 2FA composable conversion was incomplete: PaymentModal.svelte (admin Take Payment on the today page — a live saved-card charge surface) still re-implemented the 2FA gate inline while the composable's own doc listed it as one of the six surfaces. This completes the refactor: - Removed inline twoFactorCode/reveal2FACodeInput/show2FACodeInput/ missing2FACode/requesting2FACode/handleRequestNew2FACode state (74 -> 21 net lines) and the now-unused requestNewTwoFactorCode import - Composable call mirrors the TillPurchases admin reference: enabled() => true (admin supplies the CUSTOMER's code), gateActive() => twoFactorEnforced && customerTwoFactorEnabled (byte-identical semantics) - Rewired request body, success handler, 403 self-heal, focus effect, and the TwoFactorCodeInput/request-button/Pay-button bindings to the composable - Zero inline gate patterns remain in the payments components dir Frontend 72/72 tests + build + eslint clean; backend 26/26 packages. |
||
|
popertots
|
a6a4683b74 |
fix: review round — B1 clock-skew tolerance + re-poll escalation, refresh-token access-token revocation, shared 2FA composable, per-package-DB test alignment
Three fresh reviews (money/security/dup-mod) cross-validated findings: - MEDIUM: B1 'new charge' discrimination adds a lower-bound tolerance (replayRescueLowerBoundSkew) so a retained-key replay of the ORIGINAL charge (DB clock ahead of Square) is never auto-refunded; ambiguous margins leave PENDING + CRITICAL - MEDIUM: B1 re-poll escalates after stalePendingB1RefundAge (48h) — FAILED/REJECTED refunds go terminal (fail parent, claw back till-sale funding, CRITICAL notification); no more unbounded re-polling / stranded parents without webhooks - DRIFT-REAL: processManualPaymentGroup now checks PENDING/FAILED/REJECTED on the synchronous refund response (mirrors processChargeGroup/manual handler) — no more premature 'completed' - HIGH: refresh-token family kill now also invalidates the attacker's freshly-minted ACCESS token — access tokens carry a family_id claim and VerifyToken rejects tokens whose family was deleted (GenerateTokenForFamily + family-alive check); 30s grace window for concurrent two-tab refresh (no false theft alert) - LOW: 2FA mint endpoint returns remaining_seconds; in-memory 2FA counters documented; 90-day refresh expiry single-sourced (RefreshTokenLifetime + make_interval) - Dup/mod: NEW shared useTwoFactorCodeForSavedCard Svelte composable replaces 6 surface copies of the 2FA gate logic (Request-a-new-code added to BookingFlow + TillPurchases); account page adopts generateUUID - Test architecture: removed t.Parallel() from 8 global-SquareClient-swapping tests per Testing Architecture doc line 89 (B1 flaky-test lesson) — fixes within-package race - SQL alias pence rename (total_cents/paid_cents -> total_pence/paid_pence) 26/26 backend packages; 72/72 frontend tests + build; env-docs 41/41. |
||
|
popertots
|
a8bf24ee23 |
fix: dup/modularisation findings — account gift-card 2FA gate, single-source 2FA predicate, login store delegation
Restart-loop dup/mod review findings: - Account page gift-card buy flow now passes the 2FA verification-code gate end-to-end: TwoFactorCodeInput + Request-a-new-code wired for saved-card/save-card charges, verification_code in the /api/user/giftcards/buy body, 403/429 gate-failure self-heal, buy button gated on missing code (backend BuyGiftCard gate at giftcards.go:1471 already required it — the frontend never sent it) - Removed dead requires2FACodeForSavedCard export from square.ts (zero consumers; all surfaces use authStore.savedCardChargeRequires2FACode) + its test; auth store getter documented as THE single source of truth - Login page now delegates token persistence to authStore.setToken instead of direct localStorage writes (drift-risk closed; setToken persists both tokens identically so the full-reload init still works) - Pence comment corrected (GBP minor unit) - account/+page.svelte:84+6; square.ts -16; square.test.ts -26; auth.svelte.ts comment; login/+page.svelte delegation Frontend 72/72 tests + build clean; backend builds. |
||
|
popertots
|
4d5d2cd381 |
fix: restart-loop-A findings — pending sweep refunds, tip carve on discounts, TOCTOU redemption, single-use 2FA code + mint endpoint, refresh-token family revocation, admin 2FA code UX
Restart of Loop A (fresh review -> fix -> verify) findings from commit 5e967fa: - B1: sweep auto-refund treats Square PENDING refunds as NON-terminal (row stays pending, no gift-card clawback, refunds row inserted for payments AND till_sales, re-polls the deterministic sweepdup- key); Square-less pre-pass exempts square_refund_id IS NOT NULL rows - M4: terminal tip carve accounts for pending campaign discounts (headroom = total - pending - paid) so explicit tips aren't absorbed as service revenue; no-tip case stays a single record - max_redemptions TOCTOU closed with atomic conditional UPDATE ... RETURNING; exhausted-at-apply surfaces campaign_fully_redeemed - 2FA: verification code is single-use on the saved-card gate (VerifyForUser consume=true, interactive flows unaffected); new POST /api/user/2fa/code mints a fresh code for enabled users (RequireAuth + RequireNonGuest + mint cooldown + per-user limiter) - Refresh tokens: family_id + used_at columns; reuse of an already-rotated token revokes the ENTIRE family and inserts a refresh_token_reuse admin alert; rotation mints descendants in the same family - Frontend: 2FA code input + Request-a-new-code on all saved-card surfaces; admin modal keys code input to customer 2FA + 403 self-heal; tip-display note for pending discounts; 76 frontend tests - Verified: all 26 backend packages pass, frontend build+tests green, env-docs 41/41 |
||
|
popertots
|
fe88f2084d |
fix: review-loop B — adversarial findings (sweep auto-refund, admin clamp, 2FA real challenge, opaque refresh tokens, gated client IP, GBP pence)
Loop B aggressive adversarial round (3 attack agents) + fix + secondary + verification:
- CRITICAL: sweep replay auto-refunds provably-created-later duplicate charges (gated on parseable CreatedAt); 22h legitimate-retry window == 22h sweep cutoff (no dead zone)
- HIGH: admin Take Payment clamps to remaining obligation (cash/giftcard/saved-card/terminal); no unintended tip from overflow; campaign credit against remaining
- HIGH: /api/services/eligible-for/{id} requires auth + owner-or-admin (DOB/age + patch-test health-data leak closed)
- HIGH: opaque refresh-token rotation (login/refresh return {token, jti, refreshToken}; refresh REQUIRES opaque token; single-use rotation; logout revokes; access token rejected at refresh)
- HIGH: saved-card charges require a REAL 2FA verification code (B6/B10) — backend gate on all 8 charge paths + shared TwoFactorCodeInput frontend component on all 7 surfaces; 2FA gate is no longer setup-flag-only
- MEDIUM: ungated CF-Connecting-IP in reserve/admin_reserve gated via exported mw.ClientIP; 2FA limiter keyed on userID alone (no header-rotation bypass); ChangePassword actually revokes JTI + refresh tokens; 2FA setup mint cooldown + persistent failed-attempt counter; campaign redemption race surfaces campaign_fully_redeemed
- Terminal saved-card VAT applied (was under-collected); age-guard reconcile failures notify; isWeakJWTSecret entropy gate; gift-card redeem per-card counter + per-user limiter; webhook signature key startup validation
- NEW internal/twofa package (single source of truth breaking the payments<->user import cycle); consolidation of duplicate 2FA hash/verify
- Frontend: refresh-token storage + rotation, TwoFactorCodeInput component, amountPaidPence in admin modal, B5/B6/B10 contract wiring; 70 frontend tests
- Tests: loop_b_fixes_test.go, internal/twofa tests, updated auth/services/profile/twofa/mw tests
All 26 backend packages pass (incl. internal/twofa); frontend 70/70 + build clean; env-docs 41/41.
|
||
|
popertots
|
faceb9809c |
fix: review-loop A — discount credit on admin payments, campaign over-credit cap, sweep replay window, dedup refund revalidation, duplication/modularisation, GBP pence naming
Round-A fresh review (6 agents) + fix + secondary cross-cutting + verification rounds: - F1: campaign discounts reduce the charged amount (deposit credit + admin PaymentModal discounted total); capDiscountToRemainingObligation prevents over-credit at completion in all four campaign blocks - F2: sweep replay rescue distinguishes legitimate same-key retries (21h window) from expired-key new charges; ccof blind-fails leave pending + CRITICAL instead of clawing back - F3: post-start online overflow carved as a tip record (mirrors terminal split builder) - A1: single-source Square decline-code classification (till delegates to square.IsDefinitivePaymentError) - A2/A5: refund attempt-cap literals consolidated; refund-failure counter capped + reset on terminal resolutions + admin notifications - A3/A9: idempotency helpers adopted across derivations; IsExplicitDevOrMockEnv relocated + all gates unified (incl. health-check) - A7: 2FA user+IP limiter + TRUST_PROXY_HEADERS startup warning; SNAPSHOT_ENC_KEY startup validation; TWO_FACTOR_PEPPER docs corrected - A8: snapshot encryption on all 6 write sites + marker-aware reuse paths; MPV->SPV effective voucher type (single VAT point) - A10/A11/A12/A16: gift-card slot scan advances past failed; amount-aware refund reconciliation; completed-booking refund re-check; PaymentWasRefunded on SquareClient interface - Dedup refund revalidation on tip/terminal/gift-card paths; sweep acknowledged_at IS NULL parity; refund-notification single source (exported payments.InsertRefundFailedNotifications) - Duplication/modularisation round: shared frontend helpers (sanitizeDecimalInput, campaignDiscountCents, twoFactorBlocksSavedCards getter, generateUUID), single-source MaxIdempotencyKeyLength, notification-helper consolidation, snapshot-guard comments - Cross-cutting GBP rename: Cents->Pence across backend + frontend + tests (26 identifiers, 16 files) - Tests: 11 behavior-change tests updated to new invariants; coverage for fixed functions; frontend vitest 55 tests; docs corrected (test counts, 2FA delivery, pre-launch checklist, resolution status) - gitleaks: allowlist backend/internal/square test fixtures (mock idempotency keys) All 25 backend packages pass; frontend 55/55 + build clean; env-docs 41/41. |
||
|
popertots
|
6d82535780 |
fix: adversarial review round — replay-rescue double-charge, discount credit, 2FA/per-IP limits, snapshot encryption, refund reconciliation, VAT, frontend parity, tests+docs
Addresses the adversarial fresh-eyes audit (findings A1-A20) plus review-round fixes: - CRITICAL A1: replay-by-key rescue cross-checks replayed CreatedAt; ccof blind-fail leaves pending with CRITICAL + notification instead of clawing back - A2/A3/A4: till idempotency key restored to unconditional hash; tip rejected in CreateBookingPayment; campaign discount now reduces the charged amount (deposit credit) - A5: admin notifications on blind-fail, manual-refund re-arm, cap-stranded charge-group, webhook FAILED/REJECTED refunds - A6/A10: BuyGiftCard idempotency user-scoped; gift-card slot scan advances past failed rows - A7/A14/A15: 2FA user+IP limiter, SNAPSHOT_ENC_KEY startup validation, accurate pepper/log-delivery docs - A8/A9: snapshot encryption on all write+reuse sites; MPV->SPV effective voucher type (single VAT point) - A11/A12/A13/A16: amount-aware refund reconciliation; completed-booking refund re-validation; till retry dedup; PaymentWasRefunded on SquareClient interface - A17/A18/A19/A20: CI runs npm test; confirm_overflow_tip frontend dialog; unknown-event admin notification; mock token redaction - M7 ConfirmOverflowTip, M9 snapshot encryption, C1 discount ordering regression test - Frontend vitest framework (41 tests), backend coverage for fixed functions, docs corrected (2,269 tests, SUPPORT_EMAIL tokens, resolution status) All 25 backend packages pass; frontend 41/41; build + env-docs green. |
||
|
popertots
|
78e6d00dc5 |
fix: payments review rounds — money-safety, GDPR, security, gift-card cancel, modal stacking
Money-safety: - Deterministic till idempotency fallback (Square-charging only); cash/on_the_house keep unique keys; £250 till gift-card cap; 45-char key validation - Gift-card admin caps £250/tx + £5,000/day; user buy £500/day; BuyGiftCard allowlist unchanged - CancelGiftCard: CCR 2013 14-day right with partial-spend refund of the unspent balance (spend verified via payments.gift_card_id); atomic vs redeem/transfer; refunds stay pending until reversal commits; admin cancel surface (AdminCancelGiftCard) - Sweep: cancelled-booking charges failed+notified instead of silently completed; source-override replay uses live square_source_id; legacy square-less refund sweep; snapshot refresh on pending reuse - Refund lock consolidation; recordTerminalPaymentTx shared recorder; structured Square error codes; terminal checkout CustomerID GDPR / security: - Notes retained as de-identified medical/safety record at erasure (single field treated as health data; rest of record wiped, no re-identification map) + comments updated per UK GDPR/Art 9/Equality Act 2010 - square_request_snapshot PII scrubbed on all erasure paths; delete_guest_user FK unlinks; verification codes + dispute reasons handled; idle/stale-guest erasure deletes Square cards/customers + CardDAV/R2 - Durable square-erasure outbox job (retry-square-erasures); 2FA dev/prod build split, pepper fail-closed, no prod code-in-log; prod 2FA delivery fail-loud without a channel - Webhook unknown-type family split (non-money acked, money retried); untracked dispute notifications; rate-limit CF/X-Real-IP trust gating; nginx CSP nonce + api_limit Frontend: - Dynamic z-index stack (ui/dialog/zindex.ts) claimed in open order via data-state observer; re-claims on every reopen; removes stale !z-* overrides — nested modals (booking→user→booking) always paint newest-on-top (browser-verified 3-level + reopen) - Mobile: iOS zoom fixes, bottom-sheet dialogs, 44px touch targets, inputmode decimal, dvh - Gift-card buy/cancel UI, admin £250 + daily limits, cancellation/privacy/terms policy accuracy S3: - Connect() creates buckets before probing; in-memory fallback only on genuine unreachability; health reports degraded; stale S3_PUBLIC_URL documented (host-specific) Tests/docs: - 2263 test functions; all 22 backend packages green; round8/9/10 regression suites; NextEditWindowTime removes wall-clock flake; docs reconciled (notes retention, gift-card partial-use, modal T15 future work) |
||
|
popertots
|
9111258461 |
fix: 2FA disable now requires the verification code (enforced mode)
The disable flow was broken in enforced (production) mode: the account page
posted {code:''} to /api/user/2fa/disable, but the backend mints + validates a
code when 2FA is enforced, so the empty code always failed with 400 and a user
could never disable 2FA through the UI. In unenforced (dev/mock) mode the
backend short-circuits and no code is needed — which is why the user only saw
the confirmation dialog and no code prompt.
Backend:
- New POST /api/user/2fa/disable/code (SendDisableCodeHandler): mints +
delivers a fresh disable-flow code via the existing ensurePendingTwoFACode
machinery (per-user 1-min mint cooldown, 429 when throttled, 5-attempt
lockout preserved on the disable call itself). This is the disable-flow
equivalent of /api/user/2fa/setup. Runs unconditionally (no dev
short-circuit) so the step is exercisable in dev too. Route mounted in
main.go beside the other 2FA routes.
- Tests: mints fresh code, reuses valid pending code (hash unchanged),
mint-throttled 429 (after the pending code is dropped, as a lockout does),
unauthorized 401, unenforced still mints.
Frontend (account page):
- The disable confirmation now branches on twoFactorRequired: enforced →
POST /api/user/2fa/disable/code to mint, then a 6-digit code-entry input +
'Confirm Disable' button that posts the code to /api/user/2fa/disable;
unenforced (dev) → unchanged direct disable. Code entry mirrors the enable
flow's input styling; mint-throttle 429 / wrong-code 400 / lockout 429 all
surface as toasts with the entry kept open for retry.
Verification: go test -tags test,dev -count=1 -parallel 8 ./... (all 20
packages ok, 0 failures incl. 5 new 2FA tests), go build ./... and -tags dev,
go vet clean, svelte-check 0 errors 0 warnings, env-docs gate OK.
|
||
|
popertots
|
6691cd5657 |
fix: rate-limit per-IP keying + dev no-op, admin 2FA recovery, 2FA account UI, admin modals, tip totals
Rate limiting (backend):
- RateLimit/ProgressiveRateLimit now derive the per-client key from
CF-Connecting-IP, then chi's GetClientIP (the X-Real-IP value nginx sets at
main.go:323), then RemoteAddr. Previously only CF-Connecting-IP/RemoteAddr
were used, so behind the Docker nginx every client shared ONE bucket per
limiter — 10 logins/min site-wide blocked all users (the reported
'Error: Rate limit exceeded' after seeding was the login 10/min bucket
tripped by the seed's 11 logins, all keyed 127.0.0.1 in dev).
- Real implementation is now //go:build !dev || test; new
mw/ratelimit_dev.go (//go:build dev && !test) is a no-op passthrough, so
'go run -tags dev' (the dev harness) never rate-limits dev/seeding traffic,
while production and tests (-tags test,dev) keep the real limiter. The docs
(Technical Manual) already claimed dev no-op behaviour — the code now
matches. NewProgressiveRateLimiter is provided in the no-op build because
tag-free ratelimit_shared.go:104 initializes the global at package init.
Admin 2FA management (backend):
- users.two_factor_last_used_at TIMESTAMPTZ column (init-script, fresh-DB).
- AdminUserDetail now returns twoFactorEnabled/twoFactorMethod/
twoFactorLastUsedAt.
- New POST /api/admin/users/{id}/2fa/remove (admin-only): clears all 5 2FA
columns + drops the user's in-memory attempt/lockout state — an admin
recovery path when a user loses 2FA access.
- two_factor_last_used_at updated on every successful 2FA verification.
Account page (/account):
- 2FA section moved under the Notifications heading, visible to all roles;
Email/SMS toggles (Notifications styling) acting as a radio group with
'none' state; Apply button only when the selection differs from saved;
unselecting shows a payment-rules warning dialog; the dev-comment
'2FA is optional right now (REQUIRE_2FA is off)' and the 'Dev code:' debug
line are removed.
- Cards tab hidden from admin role.
Admin modals:
- User Details modal: new 'Two-Factor Authentication' section above Patch
Tests showing Enabled/Disabled, method, last-used timestamp, and a Remove
2FA button with a confirmation dialog (POST to the admin endpoint, refetch
on success).
- Booking Details modal: the customer's name now links to their User Details
modal (optional openUserModal prop threaded through admin/+page and
today/+page; other call sites unaffected).
Take Payment + /today:
- PaymentModal shows pre-tip (netTotal) and post-tip (totalWithTip) totals
with a tip-amount delta row only when a tip is selected; zero-tip flow
unchanged.
- The /today Payment button is hidden unless the booking is in_progress or
completed, matching the backend gate (was shown for confirmed/pending
bookings, producing the 'Booking must be in_progress or completed' error).
Verification: go test -tags test,dev -count=1 -parallel 8 ./... (20/20 ok
incl. new admin 2FA tests + mw tests), go build ./... and -tags dev both
compile, go vet clean, svelte-check 0 errors 0 warnings, env-docs gate OK,
docker compose config valid.
|
||
|
popertots
|
5cc5a7f6d2 |
fix: review round 7 — fresh-eyes audit fixes (6 agents) + full test suites for every backend change
Fresh-eyes review round with 6 independent agents (money-safety, concurrency, Square wire parity, security, frontend flow, testing-gaps). Every finding was independently verified against the code before fixing. All backend changes now carry full test suites (10+ new tests, each verified to FAIL without its guard). All 20 packages green, race detector clean. Money-safety: - Gift-card purchase refunds no longer create money: manual refunds of a no-booking (gift-card purchase) payment are rejected with a clear message in the direct handler AND never re-issued by the sweep-resume path (processManualPaymentGroup skips them; reconcile-then-fail, no re-issue). - BuyGiftCard no-client-key fallback: derived deterministically under the advisory lock (pending-row reuse fixes lost-response double-charge; completed-row sequence advance preserves distinct-purchase collapse fix). - Terminal completion is never unrecorded: activeTerminalCheckoutID now calls recordUntrackedTerminalPayment when a provisional (tmp-) checkout is found COMPLETED at Square (previously only marked the row COMPLETED — a lost poll left the payment invisible and unrefundable). - Sweep: provisional tmp- checkout rows are resolved against Square first (COMPLETED → record; live → keep guard; NOT_FOUND/CANCELED → fail; ambiguous → leave pending) instead of blind-failing a possibly-live checkout. recordUntrackedTerminalPayment re-checks the booking status (FOR UPDATE) and refuses to record on a cancelled booking, inserting a critical_payment_log admin notification instead. Till-sale post-charge UPDATE now requires status='pending' (no resurrection of a clawed-back sale). Frontend (Svelte 5): - UserPaymentModal keeps CardSelection mounted through processing (bind:this ref + Square iframe survive the loyalty/tokenize awaits) — new-card payments work again. - BookingFlow clears the cached nonce/verification pair on any failure (retry re-tokenizes fresh; idempotency key retained for dedup); 409 'already paid' refetches the booking and reconciles depositPaid so the confirmation gate opens; Back button disabled during processing. - Synchronous double-submit guards on buyGiftCard/redeemGiftCard/submitTip. Square wire parity (mock vs real): - processing_fee sign unified (negated at paymentFromSquare; mock agrees). - SimulateSourceUsed (SOURCE_USED, 400) matches real CreateCard. - GetCardsOnFile excludes disabled cards (matches ListCards). - ForcePaymentStatus toggle + tests prove the charge path can't be status-blind. - CreateCheckout rejects empty device_id (env fallback SQUARE_TERMINAL_DEVICE_ID); completed terminal checkout's payment resolvable by id. Security: - 2FA attempt-map data race fixed: lastAt is atomic.Int64 (nanos) — eviction scan reads race-free; concurrent verify+evict tests under -race. - Backend refuses to start on weak/placeholder JWT_SECRET_KEY (<32 chars or known public placeholders) with openssl rand -hex 32 guidance. - Dockerfile no longer COPYs .env (secrets injected via compose env_file). - SabreDAV requires DAV_ADMIN_PASSWORD (no admin/admin default); compose fails at config time when missing. Testing gaps closed (each verified to FAIL without its guard): - refunded-dedup 409 (CreateBookingPayment), keyed sweep past-retention blind-fail, reconcile status-switch (CANCELED/FAILED/APPROVED/PENDING/unknown in both by-key and by-id paths), resolveChargeSource Square-failure branches, structured 500 / CARD_DECLINED / cancelled-context E2E (row stays pending), deriveBookingPaymentIdempotencyKey >45-char truncation, webhook findPaymentByDisputeID fallback, clawbackOneTillSale non-gift-card branch, dispute.evidence / terminal.checkout dispatch. Infra: - local-dev-2.sh fails loudly on port-5432 squatters / docker compose failures (previously died silently under ERR_EXIT with hidden output). - Test harness defaults SQUARE_TERMINAL_DEVICE_ID; money_safety_fixes_test.go gained the missing build tag. Verification: go test -tags test,dev -count=1 -parallel 8 ./... (20/20 ok), -race clean on 2FA + payments money paths, go build ./... + -tags dev, go vet clean, svelte-check 0 errors, env-docs gate OK (36 vars), docker compose config valid. |
||
|
popertots
|
67cf5b9a45 |
fix: review round 6 — P0 deposit charge, idempotency rotation, dev-safety guard, 2FA/webhook hardening
Sixth fresh-eyes review pass (5 agents: goal, QA, code-quality, security, context-mining). QA FAILED the deposit-required new-card flow; the P0 root cause was backend + frontend, now fixed. All 20 packages green. P0 money-safety: - Deposit-required bookings now actually charge the deposit on new-card payment. Two-part fix: (1) CreateBookingHandler re-reads the trigger-maintained total_amount/total_duration_minutes from the DB after the booking_services insert (the INSERT..RETURNING row predates the recalc trigger, so TotalAmount serialized as 0 and DepositPaid computed TRUE on an unpaid booking — the frontend gate trusted deposit_paid:true, never charged, and confirmed the booking with zero payment rows); (2) BookingFlow.svelte gates the confirmation view on depositPaid and guards against re-creating a booking on retry. Regression test TestBookings_Create_DepositPaidFalseOnUnpaidBooking. Payments (idempotency + money): - deriveBookingPaymentIdempotencyKey: no-client-key fallback now advances a sequence for repeatable types (partial) and rotates past refunded completed rows, so refund-then-repay and equal-amount partials diverge onto distinct keys; an un-refunded completed row keeps its key (double-charge protection holds). Dedup hits on refunded rows now 409, never stale success. - chargeFailureStatus default is 503 (ambiguous), never 402; table test. - Flaky TestBookingPayment_FullPayment_SplitsIntoDepositAndBalance fixed (ORDER BY payment_type). - resolveChargeSource: orphaned card-on-file disabled via DeleteCardOnFile when SaveCardForUser fails (best-effort, redacted log); retry path preserved. Square client: - Dev builds HARD-FAIL (panic) on SQUARE_ENVIRONMENT=production without SQUARE_ALLOW_REAL_API=1; sandbox routes with a loud banner. - Mock fault-injection FailAfterCommit (commit-then-5xx) exercises the exact lost-response same-key retry; SimulateCardTokenUsed; 45-char idempotency-key cap parity; SquareEnvironment/SquareLocationID shared env helpers used by the sweep (env contract no longer comment-only). - listRefunds truncation now errors (money-sensitive reconcile retries instead of over-refunding); getCardsOnFile truncation loudly logged. Webhooks + 2FA: - square-environment header checked fail-closed (403) when configured env is production/sandbox; dispatch DB work bounded by 30s timeout contexts. - 2FA codes HMAC-SHA256 pepper'd (TWO_FACTOR_PEPPER) with legacy-hash migration + upgrade-on-verify; disable-flow mint cooldown (1/min, 429) caps the brute-force loop; in-lockout records never LRU-evicted. Repo hygiene: - env-docs CI gate green again (FRONTEND_ORIGIN + SQUARE_ALLOW_REAL_API + TWO_FACTOR_PEPPER documented; Vite DEV built-in allowlisted). - Dead square_deposits schema dropped; obsidian/README/legal-page drift fixed (consumeradvice.scot signposting, CORS allowlist, p11 R3/P13, T1). - 2FA disable residual documented; P6 email/SMS delivery and P12 sandbox smoke test remain the pre-go-live gates. Verification: go test -tags test,dev -count=1 -parallel 8 ./... (20/20 ok), go build ./... + -tags dev, go vet clean, svelte-check 0 errors, env-docs gate OK, live deposit-required flow re-verified end-to-end (deposit £11 charged, square_payment_id recorded). |
||
|
popertots
|
e9b0f0f2a7 |
fix: review-loop hardening — identical-body replay, 2FA gates, webhook at-least-once, GDPR scrub
Follow-up to the comprehensive payment-system review. Fixes the issues the review found in the initial integration, plus the rough edges it introduced. Money-safety: - Replay-by-key now replays the FULL original request verbatim from a stored square_request_snapshot, so a retained idempotency key returns the original payment instead of IDEMPOTENCY_KEY_REUSED (previously the row sat pending forever). IDEMPOTENCY_KEY_REUSED remains ambiguous (never proof of no charge). - Dev mock mirrors real Square for unknown-key replays: ccof: saved-card sources are charged and rescued; spent cnon: nonces surface ErrReplayKeyNotRetained. (Fixes dev/prod parity divergence.) - Webhook dedup row committed AFTER dispatch (at-least-once); FAILED till sales claw back gift-card funding; event-type strings match Square's real catalog. - Expired-gift-card cancellation refunds set creditFailed (never a phantom 'completed' refund); cancellation refunds lock all payment rows ascending. - Sweep never rescue-completes a gift-card purchase without delivering the card. - Tip no-client-key fallback is a deterministic count-based key under the booking advisory lock (retry-safe, distinct tips don't collapse). - M-cap subtracts completed refunds, clamped to [0, total]. 2FA (PSD2 SCA stand-in) for online saved-card payments: - Full feature: status/setup/verify/disable endpoints, gating helper wired into all 7 saved-card charge paths (incl. BuyGiftCard + admin saved-card), account admin-tab settings UI, frontend gating across all payment surfaces. - Enforcement is FAIL-CLOSED: on unless REQUIRE_2FA=false or an explicit mock/dev SQUARE_ENVIRONMENT; startup warning when off in a non-dev env. - Verify is brute-force hardened (5-attempt lockout, timing-safe compare); plaintext codes only logged when enforcement is off (dev). - GDPR: anonymize_user also scrubs 2FA columns and staff notes. Infra/docs: - nginx: /api/ response cache removed (cross-user disclosure); port 80 redirects to HTTPS (localhost/RFC1918 exempt, end-anchored regexes); HSTS; separate webhook rate-limit zone. - Schema: users 2FA columns; payments/till_sales square_source_id + square_request_snapshot. - Legal docs: gift-card cooling-off, international-transfers section, tips policy; Gap Backlog P3 webhooks marked done; stale counts/wording corrected. - Flaky test race fixed (t.Parallel + global mock mutation); suite 26/26 packages green, 2,142 tests, svelte-check clean. |
||
|
popertots
|
4b28e93710 |
fix: tip double-count, fully-paid auto-completion, discount-refund hardening
Tip double-count (root cause of £33.75 vs £28.75 display):
- Remove mock's fixed +500p auto-tip when AllowTipping is true (square_dev.go) —
real Square only enables a terminal prompt, it never adds a tip to the amount
- Set AllowTipping=false in CreateTerminalPayment: the frontend already embeds
the tip in the amount, so the terminal must not prompt for a second tip
- M4 tip split now derives the tip as charged amount minus remaining booking
value ('after 100% is tips'), not from Square's TipAmount field
- Success screens divide paymentResult.amount by 100 (pence -> pounds) in both
PaymentModal and UserPaymentModal
Fully-paid bookings auto-complete:
- Extract ApplyBookingCompletionSideEffects into payments package (shared by
admin progress endpoint and payment paths; avoids circular import)
- Add bookingIsFullyPaid + completeFullyPaidBooking: when completed non-tip
payments reach 100% of the booking total, an active booking transitions to
'completed' so it leaves the admin Current Appointment view
- Wired into CreateBookingPayment (inside tx) and GetCheckoutStatus (terminal,
after commit); completion side-effects (loyalty, campaigns, deposits_required)
fire identically to the manual progress endpoint
- Add /admin/bookings/{id}/refund route (AdminRefundBooking)
Discount-refund hardening:
- RefundPayment explicitly rejects discount/on_the_house payments (was relying
on the incidental NULL-square_payment_id guard)
- Hide the Refund button for discount/on_the_house payments in EditBookingModal
- Cancel-refund estimate in BookingModal also excludes on_the_house
- Cancellation refund loop + GetBookingPaymentInfo + GetBookingRefundableAmountCents
exclude payment_type='tip' from refundable totals
Tip flow (start-time guard) fixes tests:
- Tip tests updated to use past-dated bookings (tips now require booking started)
Tests:
- m4_tip_refund_redesign_test.go (tip split, refund exclusion, admin refund cap)
- m5_fully_paid_completion_test.go (online + terminal full-payment completion,
partial stays active, tip excluded, cancelled stays cancelled)
- Full suite passes with -race (25 packages)
|
||
|
popertots
|
5e3dc9b428 |
fix: comprehensive payment system hardening (4 review passes)
CRITICAL fixes: - C1: JWT exp claim now validated via jwtauth.VerifyToken (was Decode) - C2: OverrideAmount validated post-substitution (prevents negative money minting) - C3: Terminal gift-card payments store gift_card_id; refund credits user balance - C4: Refund dedup returns stored amount, not req.Amount (prevents admin mislead) - C5: Booking recheck uses FOR UPDATE (prevents TOCTOU with cancellation) - C6: processChargeGroup idempotency key stable (charge-only, prevents double-refund) MAJOR fixes: - M2: Gift-card refund UPDATE checks RowsAffected; 0 rows -> failed - M3: ProcessCancellationRefund returns commit error (was swallowed) - M5: Dispute webhook handling (created + state.updated + disputes table) MEDIUM fixes: - ME1: CORS restricted to FRONTEND_ORIGIN env var (was reflect-any) - ME2: anonymize_user() scrubs users.notes, bookings.notes, name_history, refresh_tokens - ME3: Webhook handlers now mutate state (payment.updated, refund.updated) Frontend fixes: - Same-key retry on 503 (ambiguous failure) wired to all 8 payment flows - CHARGE_AND_STORE intent for save-card flows (SCA compliance) - Nonce staleness check verified across all flows Additional fixes from adversarial re-review: - F1: Till-sale completed dedup echoes stored amount (C4-class) - F2: Cash/giftcard terminal path uses FOR UPDATE (C5-class) - F3: Square-success UPDATE checks RowsAffected (till sales) - F4: Dispute reason truncated to 192 chars (prevents INSERT failure) - F5: Booking-user lookup failure marks refund failed (prevents silent money loss) - F6: Saved-card/tip rechecks wrapped in transaction (C5 residual) Tests: - 15 adversarial attack tests (negative override, zero override, terminal gift card, refund dedup, TOCTOU, deleted gift card, advisory lock, overcharge, zero/negative/huge amount, raw PAN, missing auth, gift card balance, concurrent refunds) - 14 webhook state tests (dispute created/state, payment/refund updated) - 3 CORS tests, 3 GDPR tests, 1 HTTP timeout test - Full suite passes with -race (25 packages, 0 failures) 25 files changed, +1532/-275 lines |
||
|
popertots
|
ea771ffaf5 |
Till: saved-card payment surface with customer picker and valid-card gating
Staff may charge a customer's saved card at the till but cannot add or save
one. A customer picker (reusing the admin user-search pattern, excluding
admin/guest/affiliate) loads the customer's cards via
GET /admin/users/{id}/payment-methods; the 'Saved card' payment option is
hidden outright when the customer has no currently-valid cards, computed
client-side with the Square convention (valid through the end of
exp_month/exp_year). The saved-card charge sends payment_method saved_card
plus user_id/user_saved_card_id with no card_token or verification_token, and
the per-line idempotency keys also key on the selected card so switching cards
yields fresh keys. No Square Dashboard hint, no new-card form, no save
checkbox — the till can never persist a card.
|
||
|
popertots
|
c7d7169cd2 |
Disable svelte/prefer-svelte-reactivity (SvelteDate purge rationale)
The rule pushes SvelteDate (svelte/reactivity), which this codebase deliberately removed in 55b2c8c because it caused real bugs. All Date usage is local wall-clock computation via parseWallClockDate helpers, not reactive $state Date mutations, and the rule has no options to allowlist Date — so disable it outright in the svelte block with a rationale comment. Restores a clean eslint gate for the pre-commit hook and CI. |
||
|
popertots
|
05bb142cfd |
Frontend: verified-only save-card gating + shared nonce-staleness helper
canSaveCardsForRole(role) in square.ts is the single source of truth for the
save-card product rule (verified_email, admin — never affiliate). All four
predicate sites (account page, UserBookingModal, BookingFlow, TipPayment) were
wrong before, excluding admin and including affiliate. The worst gap was
BookingFlow passing canSaveCards={authStore.isAuthenticated} to the Pay-Early
modal, which let unverified users save cards — it now passes the derived value.
isNonceStale() + NONCE_STALENESS_MS replace the 240s staleness check duplicated
five times, keeping the amount-bound re-tokenization semantics identical.
|
||
|
popertots
|
197d4c4b9b |
Gift-card rolling expiry, SvelteDate→Date purge, strict DST tests, UTC scan-location + settings legal floor
Gift-card rolling expiry (setting-driven, was dead config): - GetGiftCardExpiryMonths(): single source of truth (business_settings gift_card_expiry_months, fallback 24) shared by payment handlers and the CleanupExpiredGiftCards job (was hardcoded 24). - expiry_date now maintained on ALL 9 gift-card write sites (buy, topup, transfer, redeem, terminal payment, refund credit, till) so the refund-time guard at refunds.go actually fires. Schema default 12->24 + migration note; test-DB seed aligned. Stale "expiry_date IS NULL" test rewritten; new expired-card-rejected regression test. Frontend SvelteDate purge (docs' stated convention, wide): - All 180+ raw `new SvelteDate(...)` uses across routes/components replaced with parseWallClockDate (backend UTC ISO) or new Date (wall-clock constructors). SvelteDate imports removed. timeSlots.ts getDayWithOrdinal fixed. Zero SvelteDate references remain; svelte-check clean. Strict timezone/DST testing + QA fixes: - 8 new hermetic boundary tests: clock.DST transitions (both 2026 folds), closing-hours GMT vs BST, booking date-window midnight, refund-tier elapsed-time independence, deposit-window UTC-instant, scheduling LondonDateString midnight, today AT TIME ZONE window + UTC round-trip. - today.go summary date labels fixed to London wall-clock (were showing the previous UTC day during BST) + regression test. - pgx ScanLocation fixed to UTC via AfterConnect (was host-local -> JSON offsets depended on deployment TZ, contradicting the documented UTC invariant) + regression test. Registered as a new *Type to avoid a data race on the shared type map (caught by -race). Admin Business Settings (setting now functional => legal floor): - gift_card_expiry_months validation floor raised 1 -> 12 months (CMA/ Consumer Rights Act 2015 unfair-contract-term guidance) in endpoint + UI, with rolling-expiry semantics shown in both display and edit form. - 3 new expiry validation tests; 2 pre-existing message assertions updated. Full suite 25/25 + race clean via run-tests.sh lockfile; svelte-check 0 errors/warnings; production build succeeds. |
||
|
popertots
|
7f1c649f1e |
Apply second-round review fixes: idempotency-key length caps, stable-sentinel card keys, test-isolation, naming
Money-safety idempotency hardening (I1, wide): - validate:"max=45" on CreateTerminalPayment/BookingPayment/Refund/Tip/ BuyGiftCard idempotency keys (all feed Square's 45-char /v2/payments, /v2/refunds, /v2/cards caps); BuyGiftCard corrected from a wrongly-loose max=64. Till keeps max=64 (its key also feeds the 64-char terminal-checkout endpoint). - Explicit 45-char guard in RefundPayment: the one handler that decodes RefundRequest without running the struct validator, so the tag alone was inert; a longer key would 400 at Square and be misclassified as a definitive refund decline. - New TestIdempotencyKey_OverLength_RejectedAcrossPaymentHandlers covers all six endpoints (terminal saved-card, booking, tip, gift-card, till, refund). Stable-sentinel card identity in idempotency keys (C1, wide): - BookingFlow deposit key now uses the 'new-card' sentinel instead of embedding the cnon: nonce (matches UserPaymentModal/account). A re-tokenize after a spent nonce no longer regenerates the key, closing a lost-response double-charge window. - TipPayment + UserBookingModal tip keys now include card identity (selectedCardId || 'new-card'); previously keyed on amount only, so a same-amount tip on a DIFFERENT card reused the key and deduped a distinct charge. Resets cleared in every success/close path. Test isolation (R1): TestRefund_PendingResume_NewKeyAfterModalReopen no longer t.Parallel — it swaps the package-level SquareClient mid-test and a concurrent parallel test could observe the swapped instance. Naming/quality (M1/M2/M4): resolveChargeSource local renamed savedRowID (was shadowing the cardID *string parameter); BuyGiftCard fallback prefix "till-" -> "gc-"; saved-card terminal response key "checkout_id" -> "payment_id" (it holds a DB payment row, not a Square checkout) with matching frontend fallback. README maintenance-job count corrected 24 -> 25. Full suite 25/25 + race clean via run-tests.sh lockfile; svelte-check 0 errors/warnings; production build succeeds. |
||
|
popertots
|
a8d54f1e2a |
Fix review findings: aggregated-refund/saved-card/legacy-refund idempotency keys, structured Square error classification, CSP for Square SDK
Money-safety idempotency fixes (external review bugs 1-3): - processChargeGroup: aggregated refund key now hashes the sorted pending-row set (chargeID-square-agg-<sha256 suffix>) so a changed group can never mark a new row completed against an old smaller refund; >45-char chargeIDs use a hashed prefix instead of verbatim truncation (which would collide charges on Square's global key dedup). Same-set crash-retry keeps Square's dedup. - CreateTerminalPayment saved_card: two-tier idempotency key — client-supplied per-attempt UUID preferred (distinct identical charges no longer collapse), deterministic booking+type+amount+card fallback for no-key retry safety. PaymentModal sends a per-charge UUID cleared after success. - ensureRefundKey: legacy NULL-key manual refunds persist a generated key to the row BEFORE the Square call (race-safe AND idempotency_key IS NULL guard), so a lost-response retry reuses the key and never double-refunds. Wired into resumeManualPendingRefund and the sweep's manual-retry loop. Classification + money-safety hardening: - till.go/sweep.go: structured square.ErrorCode/IsNotFound are authoritative when present; message-substring matching only for non-structured errors (dev mock, client-side status errors). Fixes fragile string-matching driving sweep retries and gift-card clawbacks. - SaveCardForUser: ON CONFLICT (user_id, square_card_id) DO NOTHING + re-select (was a latent UNIQUE-violation 500 on save-card retry). - CreateBookingPayment: partial payments re-validated against remaining balance inside the advisory lock (closes concurrent-overpayment race). - InvalidateSquareCustomerCache on GDPR erasure paths (account.go, time-blockers.go stale-guest anonymization). - GetUserGiftCardBalanceAdmin: in-handler admin check (defense-in-depth). - getCheckoutHTTP: warn on multi-payment checkouts instead of dropping payments[1:]. - Cash/giftcard terminal branch: removed dead idempotency SELECT, "tip-" -> "till-" prefix. - UserPaymentModal: removed vestigial polling state; proper interval cleanup. - account/+page.svelte: gift-card redeem dialog links /terms. - nginx CSP: allow *.squarecdn.com and js.squareup.com so the Square Web Payments SDK + card iframe can tokenize behind the proxy. Tests: +8 regression tests covering changed-set refund keys, legacy NULL-key single-refund, saved-card client-key dedup/no-dedup, concurrent partials, and cache invalidation. Full suite + race detector clean via run-tests.sh lockfile. |
||
|
popertots
|
726ac8cb65 |
Harden money-safety re-review findings: nonce-independent idempotency keys, reconciliation-required logging, terminal-state classification
Re-review (2 Oracle + security + QA + librarian + context-miner) surfaced fixes, all applied: (1) UserPaymentModal and the account-page gift-card buy now key the cached idempotency key on a stable 'new-card' sentinel instead of the cnon: nonce, so clearing the nonce on a failed charge no longer regenerates the key — a lost-response retry now dedups at Square instead of double-charging (the tip flows already keyed on amount only). (2) The create-with-redeem clawback now logs CRITICAL when the guarded balance reversal is blocked (previously silent), and its transaction DELETE is scoped to this sale instead of deleting every transaction on the card. (3) reconcileStalePaymentAtSquare now treats APPROVED/PENDING as non-terminal (leave pending) instead of definitively failed, matching Square's documented state machine. (4) GetTillCheckoutStatus returns 404 for a sale already swept to failed instead of reporting a live state. (5) The critical-payment scan job skips candidates whose booking was hard-deleted, so one orphan can no longer silence all critical alerts. |
||
|
popertots
|
965da86b64 |
Re-tokenize fresh after a definitive card charge failure in all nonce flows
A cnon: nonce and its SCA verification token are consumed by a definitive charge failure (e.g. declined card) and can never succeed again, but TipPayment, UserBookingModal, UserPaymentModal and the account-page Buy-a-Gift-Card cached them and resubmitted the dead nonce on every retry — a non-retryable failure loop. The nonce/verification-token/amount/timestamp cache is now cleared in each error branch so retries re-tokenize fresh, while the idempotency key is kept for network-timeout dedup. |
||
|
popertots
|
439fc16402 |
Fix sticky Square SDK rejection and clean up script element on failure
A load that resolved the script tag but failed to expose window.Square (or timed out) permanently cached a rejected promise, bricking card entry until reload. sdkPromise now resets and the injected script element is removed on every failure path so later calls retry fresh. |
||
|
popertots
|
e5c6458ec7 |
Fix frontend payment flows: BookingFlow fetch loop, shared TipPayment, card icons, terms route
Fix the P0 infinite refetch in BookingFlow (payment-methods fetched once via a guard flag, was looping on empty saved-card arrays and DoS-ing the rate limiter). Extract the shared TipPayment component so tip and pay-tip routes no longer drift; reconcile formatTimeRange override_duration_minutes and subtotal/tipsPaid. CardBrandIcon gains the correct Square enum keys (DISCOVER_DINERS, CHINA_UNIONPAY). PaymentModal reads card_last4. Login links resolve to the new /terms and /privacy-policy routes. Add frontend/.env.example. |
||
|
popertots
|
54a5b1024e |
Implement full Square payment review fixes + frontend polish
Implement every finding from the deep payment review (P0-P2, minors, nitpicks), then close the post-implementation re-review items, then align card-form typography and roll out the Square trust badge. Backend - Square API alignment: - tip_settings.allow_tipping nested under device_options (was top-level: terminal tips were silently lost in prod) - CreateCardOnFile now accepts customerID and sends card.customer_id; saved-card (ccof:) charges forward square_customer_id as CustomerID - New SquareClient methods GetPayment, CreateCustomer, CancelCheckout - SCA verification_token accepted + forwarded in all charge paths - ExpMonth/ExpYear -> *int; URL-path id validation; CancelCheckout NOT_FOUND-only no-op (dropped unverified NOOP); exported ErrorCode/ ErrorDetail helpers; mock rejects raw PANs, RList locks, redacts emails, ForceRefundPending hook Backend - money safety: - sweepManualPendingSquareRefunds reconciles rows WITH square_refund_id instead of stranding them forever - SweepStalePendingPayments reconciles at Square before failing (tri-state: leave pending on transport error, rescue completed, fail definitively) - GetCheckoutStatus cancellation-recheck; terminal CANCELED resolution; SweepStaleTerminalCheckouts covers terminal_checkouts table - till gift-card clawback on definitive failure incl. retry path + INSUFFICIENT_FUNDS/ADDRESS_VERIFICATION_FAILURE/TRANSACTION_LIMIT - cross-user saved-card collision fixed (UNIQUE(user_id,square_card_id)) - customer provisioning (lazy, save-only); one-off/guest mint no customer - discount preview/apply unified in discounts.go (global-milestone visible in preview, N+1 eliminated, redemption counter preserved on failures) - webhook event_id dedup; refund loop dedup; stale comment fixes - test-isolation t.Cleanup on committed sweep tests Frontend: - SCA tokenizeWithVerification across all charge flows (amount as major-units decimal), 5-min token-expiry re-tokenize, verification_token in request bodies - PaymentModal synchronous double-click + zero/negative-amount guards - till online-card UI wired to /api/admin/till/sale - policyPopover generalised; new /privacy-policy route; consent checkbox copy + Square privacy link - Square card iframe styled to app typography (Inter 14px, oklch tokens); mock form md:text-sm parity - 'Secure payment powered by Square' badge on all 8 card-payment flows Schema/docs: terminal_checkouts + square_customer_id + per-user card constraint in init-script.sql; README migrations; P14 plan + backlog + Technical Manual updated. Includes 39 modified/new test files; full backend suite (25 pkgs), -race on payments+square, and frontend build are green. |
||
|
popertots
|
52e2bfff55 |
Add dev-only frontend Square mock mode for as-if-live walkthroughs
VITE_SQUARE_ENVIRONMENT=mock renders a plain HTML card form (MockCardForm) instead of the Square Web Payments SDK iframe, minting the same cnon: tokens the backend dev mock accepts — all 8 payment flows run end-to-end locally with zero credentials. - isSquareMock() gated on import.meta.env.DEV: structurally impossible in a production build even if the env var is mis-set - MockCardForm: Luhn/brand/expiry/CVC validation, Amex 15-digit + 4-digit CVC, error states, disabled propagation — mirrors the real form's onReady contract so CardSelection.isCardValid and submit guards behave identically - tokenize() maps typed card -> deterministic cnon: token matching backend detectCardInfo (4242->test-card, 4111->visa, 5555->mastercard, 3782->amex) - lazy-loaded via dynamic import: mock code ships in its own chunk, referenced only from the mock branch, never statically imported into the main bundle - docs: .env.example (mock pairing with SQUARE_ENVIRONMENT=mock), P11 plan (mock opt-in + canonical-last4 caveat), Feature Catalog (2.1, 2.5) - prettier formatting fixes in 10 unrelated files (line wrapping only) |
||
|
popertots
|
7439fa86c1 |
Fix payment review round 3: saved-card idempotency, stale-pending sweep, webhook fail-closed
R1/R4: saved_card branch in CreateTerminalPayment now mirrors CreateTipPayment - advisory lock (crussell:payment:<bookingID>) serializes concurrent double-clicks - deterministic key bookingID-sc-type-amount-cardID (<=45 chars) so a lost-response retry derives the same key and dedups instead of double-charging - idempotency switch inside the lock: completed -> dedup, pending -> reuse with pence amount-guard, failed -> clean 409 - success response includes card_brand/card_last4 (frontend already reads them) R2: add 'failed' case to all four retry switches (tip, booking, gift card, till) - a swept/definitively-rejected record returns 409 instead of 500-ing on the idempotency_key UNIQUE constraint R3: extend SweepStalePendingPayments to till_sales card rows - sweeps pending till_sales (online_square/in_person_card) past Square's ~24h key retention, closing the double-charge window for till sales - swept rows logged with the same CRITICAL manual-reconciliation marker as the refund sweep Webhook fail-closed: reject 503 when SQUARE_WEBHOOK_SIGNATURE_KEY unset, 403 on bad signature (was: skip verification in dev) Refund status resolution: refunds now resolve by Square status (COMPLETED/PENDING/FAILED/REJECTED) instead of assuming completed; real error codes (REFUND_AMOUNT_INVALID, PAYMENT_NOT_REFUNDABLE, REFUND_ALREADY_PENDING) added to the definitive/processed classification HTTP client: CreateCard key truncated to <=45 chars, device_options always sent (env SQUARE_TERMINAL_DEVICE_ID fallback), processing_fee reads amount_money, ListCards cursor loop, refund keys hashed to <=45 chars Other fixes: payment/till/gift-card advisory-lock + FOR UPDATE asymmetries, GetPaymentByID NULL scans, loyalty redemption lock, card upsert on conflict, mock ccof: prefix parity, IsValidSquareCheckoutID for real Square IDs, isAdminRequest defense-in-depth on all 6 admin payment handlers, webhook signature docs, M8/L5 debug markers removed Docs: README/FC/TM/Overview updated (22 jobs, 20 CRITICAL sites, 23-section GDPR export, sweep jobs, webhook fail-closed); P11 plan marks remaining items (sandbox smoke test, M-8 customer_id, saved-card key dedup trade-off) as deferred with rationale; gap backlog pruned of completed items |
||
|
popertots
|
9ff591fa4e |
Fix payment review round 2: refund idempotency, pending-resume safety, terminal-completion lock
Refund idempotency (P2):
- RefundRequest gains an optional client idempotency_key: two DISTINCT equal
partial refunds of one payment no longer collide on the amount-derived key
(the second was silently swallowed as a dedup)
- Extract resumeManualPendingRefund: resumes a pending refund with the row's
OWN stored key, so Square's key dedup returns the original refund if the
prior attempt completed — never issues a second
- (payment, amount) pending fallback: when the exact-key lookup misses (admin
reopened the modal, new UUID), resume the matching pending row instead of
creating a second pending row the sweep would double-process
- 409 in-flight guard: if a pending refund exists for the payment but no
same-amount row matches, reject a different-amount refund (money state at
Square is unknown — no new refund is safe until it resolves)
- Frontend (EditBookingModal): UUID per refund attempt, reused on retry,
mirroring the tip flow
Terminal completion (P3):
- GetCheckoutStatus serializes on pg_advisory_lock('crussell:terminal:' ||
SquarePayID) on a pinned connection — concurrent polls of the same checkout
can no longer both pass the dedup SELECT and race the UNIQUE constraint
Card-on-file / doc-only:
- Document why CreateCardOnFile is NOT rolled back on payment failure
(deterministic sha256 retry returns the same card; deletion breaks it)
- Document HasCompletedPayment's deliberate 'tip' exclusion
Regression tests:
- TestRefund_TwoEqualPartialRefunds_ClientKeyDisambiguates
- TestRefund_PendingResume_NewKeyAfterModalReopen (proves stored-key resume)
- TestRefund_PendingResume_DifferentAmountRejected (409 + no second row)
- TestRefund_GuardCountsPendingRefunds updated: 400 -> 409 (in-flight guard
fires first — strictly safer, blocks before any Square attempt)
- TestGetCheckoutStatus_ConcurrentPolls_SingleRecord (real two-goroutine race)
|
||
|
popertots
|
53ca89603d |
Fix payment review round: till integrity, HTTP client tests, concurrency tests, card-selection consolidation
Addresses the payment review (all 10 blocking + 2 minor findings): Till money-integrity (CreateTillSale): - Add pg_advisory_lock on the idempotency key (concurrent same-key double-funding race) - Guard amount on pending-reuse retry (mirrors tip/gift-card guards) - Explicitly complete the row for cash/on_the_house pending-reuse - Reject method-switch on a live card-machine checkout (double-charge guard) - 3 regression tests (amount-mismatch, cash-completes-row, method-switch) BookingFlow: - Fetch saved cards at the deposit step (was dead code) - Charge the server-computed deposit_amount, not the client estimate HTTP client tests (was untested): doJSON error parsing, refund sentinel classification, payment/refund/card wire shapes, checkout polling states, list-refunds pagination + 20-page guard, sha256 card idempotency key Concurrency regression tests: real two-goroutine races for BuyGiftCard, tip, and booking-payment locks asserting exactly-one record each Frontend: - Fix CRIT-1: zero-saved-card users blocked (all flows now handle it) - Consolidate tip/deposit/Buy-Gift-Card card UI onto CardSelection - Explicit save-card consent checkbox (was silent/inconsistent) - Fix stale saved-card field names in BookingFlow (last4 -> last_4) - Unique instance ids (crypto.randomUUID) in CardSelection/SquareCardInput - UserPaymentModal: keep card form mounted on error + Try Again button Health/docs: /api/health reports square state (mock/ok, was not_implemented), close P1 backlog, correct stale webhook and env-var claims |
||
|
popertots
|
64d4b65083 |
Implement P11: Square Web Payments SDK new-card tokenization
Re-enable new-card entry across all 8 flows via Square Web Payments SDK cnon: nonces (backend was already P11-ready): - Add square.ts SDK loader (env-gated on VITE_SQUARE_APPLICATION_ID/LOCATION_ID, sandbox vs prod URL auto-derived from app-ID prefix) + SquareCardInput.svelte (tokenize() via bind:this, onReady state, CardEntryUnavailable fallback) - CardSelection.svelte: replace newCardDisabled gate with new-card toggle + SquareCardInput; expose tokenize() for parent flows - Wire new-card mode into tip x3, booking payment (UserPaymentModal), deposit (BookingFlow incl. guest), Buy a Gift Card + Add a Card (account), and admin till online_square (GiftCardsManagement create/topup) - Retry-safe: each flow caches the one-shot nonce and reuses it on retry so the backend idempotency key dedups instead of re-tokenizing - Docs: README, Gap Backlog P11, Feature Catalog, Technical Manual, P11 plan |
||
|
popertots
|
ae8735ba2f |
Close refund system and gate raw-PAN card entry
Refund system (Round 3 fixes + follow-up + alignment): - Serialize cancellation refunds against the manual handler via per-payment advisory locks taken before the prior-refunds read (pg_advisory_xact_lock, ascending, same crussell:refund: key space) - Aggregate pending cancellation refunds into ONE Square refund per charge (stable charge-level -square-agg key); atomic group UPDATE keeps crash-retry amounts identical for Square key-dedup - Persist paymentID-square-amount idempotency keys on cancellation refunds; scheduler reads the stored key (legacy fallback for old rows) - Add sweep-pending-square-refunds cron (*/5, concurrency 1) with refund_attempts cap; sweep retries stale manual pending refunds with each row's own stored idempotency key - Reconcile at Square (GET /v2/refunds ListPaymentRefunds) before every terminal failed transition: tri-state result leaves rows pending on reconcile error instead of false-failing; PAYMENT_ALREADY_REFUNDED resolves to completed - Move over-refund guard inside the lock, counting completed + pending (excluding failed); ErrRefundDeclined distinguishes definitive vs ambiguous outcomes - forgiveFees now executes a real full refund (forceFullRefund override) with admin_forgiven_fees reason threaded to Square - Surface failed card refunds in the admin notification centre (refund_failed enum, RETURNING-id pre-pass inserts, NOT EXISTS dedup) - Dedup double-cancel refund inserts via ON CONFLICT (idempotency_key) DO NOTHING without consuming refundRemaining Frontend: - Remove all raw-PAN card entry: zero card_number/card_cvc/new_card_token in request bodies; gate new-card entry behind CardEntryUnavailable notice + newCardDisabled prop across all 8 flows - Delete hand-rolled CardInput.svelte; keep CardSelection saved-card UI and CardEntryUnavailable fallback - Update cancellation-policy page to in-person cash pickup wording Tests: - Rewrite the two amount-blind dedup tests to assert real money movement (single call, aggregated amount, shared refund ID) - Add coverage: manual refund vs cancellation serialization (concurrent goroutines), reconcile error vs no-match branches, stale manual retry, forgive-fees real refund row + reason, double-cancel dedup, mock refund key dedup, ListPaymentRefunds filtering - Fix time-dependent booking flakes with fixtures.NextWorkingDayAt - 25/25 packages pass; -race clean on payments/square/db/jobs/bookings |
||
|
popertots
|
54f6bf3c1a |
Fix P0/P1 review findings: truncation, raw-PAN API edge, refund lock, till pending-retry, idempotency keys
P0 — float truncation: applied math.Round to all remaining int64(x*100) sites (till penceAmount, refund over-refund guard, GetAlreadyRefundedAmount, payment summary conversions). A £1.14 till sale previously charged 113p. P0 — raw PAN stopped at the API edge: - Deleted CardNumber/CardExpMonth/CardExpYear/CardCVC from TillSaleRequest and CardNumber/Expiry/CVC from CreatePaymentMethodRequest. Both now accept card_token (Square nonce) and return 400 when absent. PAN+CVV no longer transit the application server (PCI-DSS SAQ-A scope). - Deleted CreateCardOnFileRaw from the SquareClient interface and all implementations (MockClient, ProdClient, devProdClient). - Added idempotency_key column to refunds table (UNIQUE). P0 — RefundPayment hardened: advisory lock on payment ID (prevents two concurrent refunds passing the over-refund guard), pending-refund-record- then-Square pattern (scheduler reprocesses on failure), same-key dedup. P1 — till sale pending-retry now re-attempts the Square charge instead of returning the stale 'pending' status (gift card was already funded in the committed tx — silent money loss otherwise). Sale row reused, not duplicated. P1 — idempotency key caching in frontend: BuyGiftCard and UserPaymentModal/BookingFlow now cache the key per amount+card, regenerated on change and cleared on success — matches the tip-flow pattern so a lost-response retry dedups instead of double-charging. P1 — CreateTerminalPayment cash/giftcard INSERTs now persist idempotency_key. Key is unique per payment (booking+type+amount would wrongly dedup two legitimate identical payments, e.g. two £50 cash receipts). P1 — gift-card codes no longer logged (spendable credential; value+recipient only). Tests: till pending-retry re-attempt, refund same-key dedup, mock CreatePayment idempotency dedup, CreatePaymentMethod nonce happy path + raw-PAN rejection, till online_square card_token required/valid. |
||
|
popertots
|
73dd2c2dea |
Fix regression: UserPaymentModal 'Use a new card' overridden by auto-select effect
Move the saved-card auto-select effect INTO CardSelection, where it owns both
cards and showNewCardForm. The effect is guarded by !showNewCardForm so the
'Use a new card' click (selectedCardId = '') is not immediately re-set to the
default card — previously the parent's unguarded effect (moved from the OLD
showNewCardForm guard during the CardSelection refactor) silently charged the
saved default card instead of the newly entered card.
CardSelection mounts fresh each time the modal opens (conditional {#if}
mounting in UserBookingModal and BookingFlow), so the auto-select fires once
on load, exactly like the tip flows' one-shot load-time selection.
|
||
|
popertots
|
5a373a3b0b |
Unify card selection UI via reusable CardSelection component; fix Buy a Gift Card new-card bug
Create CardSelection.svelte reusable component encapsulating the standard saved-card list + 'Use a new card' + CardInput pattern with blur-based validation (Luhn, expiry, CVC) — identical to the tip flows and account page. Refactor UserPaymentModal (Make a Payment submodal) to use CardSelection: - Removed its bespoke 'Use a different card' expand/collapse UI and inline validation derivations (parseExpiryParts, isValidLuhn, touched state) - Bound selectedCardId + new card fields to the component - payButtonDisabled now driven by component's onValidityChange callback - Removed now-unused CardInput import, SvelteDate import, formatCardExpiry Fix account 'Buy a Gift Card' bug: 'Use a new card' click did nothing because the auto-select effect immediately re-set buySelectedCard back to the default card. Added buyShowNewCard flag so the effect only auto-selects on initial load; reset after successful new-card purchase so the next purchase re-defaults. |
||
|
popertots
|
28f0ddb328 |
Fix tip amount-change false dedup, wire BuyerEmail everywhere, clear ESLint errors
Money-moving fixes:
- Tip idempotency key regenerates when the tip amount changes after a failed
attempt (all 3 tip flows). Cached key still reused on same-amount retry
(dedup intact) and cleared on success/modal reset. Prevents silent
under-charge when a user retries at a different amount.
- Till replay path returns actual till_sales.status (may be 'pending') instead
of hardcoded 'completed' — no more misreported successful charge.
- BuyerEmail wired for CreateBookingPayment, gift card purchases, and till
sales (saved_card + online_square), matching the tip flow. Email lookup
errors logged, non-fatal.
- Till buyer-email errors now logged (was silently swallowed).
- on_the_house till top-up uses cached getIdempotencyKey() for retry-safe dedup
(was fresh crypto.randomUUID()).
Test/validation fixes:
- Add TestPaymentFromSquare_* unit tests (else-branch + nil card details),
build tag relaxed to 'test' so they run in the standard dev suite.
- Add TestValidateCardInfo table test (7 cases: both/either/neither/empty).
- Add TestCreateTillSale_TwoIdenticalCreateSales_BothSucceed regression test.
- Remove dead mock pre-registration in TestTipPayment_WithSavedCard.
- Correct misleading till regression-test comment.
ESLint cleanup (12 errors -> 0):
- Remove unused loadingCards in tip + pay-tip pages (dead assignments in
loadSavedCards).
- Scoped eslint-disable for {@html} in CardBrandIcon (hardcoded brand SVGs).
- Remove dead confirmSaveDefaultHours + unused rescheduleVersion prop in
WeeklySchedule (and its parent pass-through).
- Replace new Date() with SvelteDate in WeeklySchedule + BusinessHours.
- Fix each-block key in BusinessHours skeleton loader.
- Use void expression for reactivity-tracker reads in effects.
|
||
|
popertots
|
2459ddc919 |
Fix review findings: expiry bug (all 8 files), idempotency keys, card_expiry/card_cvc removal, URL encoding, BuyerEmail logging, ValidateCardInfo, saved-card test, future work doc
Backend: - Fix refund idempotency key: clock.Now() → deterministic (pr.ID + amount) - Fix ValidateCardInfo: enforce mutual exclusivity, handle empty strings symmetrically - Fix paymentFromSquare brand fallback (remove dead SourceType fallback) - Fix URL encoding: PathEscape → QueryEscape for customer_id query param - Fix BuyerEmail: log warning on DB error instead of silent discard - Fix idempotency key in createCardOnFileHTTP: time.Now() → deterministic hex hash - Add BuyerEmail to CreateTipPayment Square request - Move realBaseURL from shared file to square_dev.go (only used in dev) - Add TestTipPayment_WithSavedCard test (card_id path coverage) - Fix AMEX brand in mock (AMEX → AMERICAN_EXPRESS, fix test) Frontend: - Fix off-by-month expiry bug in ALL 8 files using year-month arithmetic (parseExpiryParts returns 1-indexed, SvelteDate expects 0-indexed) Files: tip/+page, pay-tip/[id], UserBookingModal, UserPaymentModal, BookingFlow, account/+page (add card + buy gift card sections) - Remove card_expiry/card_cvc from tip request bodies (backend has no fields) Docs: - Mark P9 (placeholder tokens) as completed, add P11 (Square Web Payments SDK) - Mark T13 (rune arithmetic) as completed |
||
|
popertots
|
4abcb324c9 |
Square payment integration: real HTTP client, tip flow rewrite, card UI/validation overhaul
Backend: - Create square_http_client.go: real Square REST API client (Payments, Terminal Checkouts, Refunds, Cards, Locations) with proper JSON types, auth, error handling - Update ProdClient in square.go to delegate to shared HTTP functions - Wire devProdClient in square_dev.go to also make real HTTP calls for sandbox/prod env - Rewrite CreateTipPayment handler: accept card_id OR new_card_token (+save_card), advisory lock, idempotency check, max amount validation - Add ValidateCardInfo, bump ValidateAmount max to £10,000 - Fix mock CreateCardOnFile to detect brand/last4 from raw card numbers - Fix mock RefundPayment to index by SquarePayID and accept unknown payment IDs - Remove dead types (ProcessingFee, sqAddress), add Deadline parity - Fix AMEX brand inconsistency (AMEX -> AMERICAN_EXPRESS) - Pre-existing fix: remove unused context import in giftcards.go Frontend: - CardInput.svelte: add onfieldblur/onfieldinput callbacks for blur-based validation - CardBrandIcon.svelte: brand SVGs for VISA, MC, AMEX, Discover, Diners, JCB, Square Gift Card, UnionPay, Interac, EFTPOS - tip/+page, pay-tip/[id], UserBookingModal tip: saved card list + CardInput + Luhn/expiry/CVC validation + blur-based errors + no-saved-cards edge case - UserPaymentModal, BookingFlow: card validation parity (blur-based, all-valid check) - account page: replace text brand badges with CardBrandIcon - Fix handleCustomTip bug (state mutations outside if block) - Remove dead pageState variable - Add tip modal scroll (max-h-[90vh] overflow-y-auto) - Submit button disabled on !isCardValid Tests: - 30 square package tests (+new: CreateCardOnFile raw number path, detectCardInfo variants) - 5 tip handler tests (HappyPath, NoPriorPayment, WrongOwner, MultipleTips, TxFailure) - All +-race clean, refund tests fixed |
||
|
popertots
|
f6caaab8a3 |
feat: redesign homepage, add global footer, fix carousel font
Homepage: restored original v0 design, replaced lorem ipsum with real service descriptions, added BusinessHours (Opening Hours) section, alternating bg-gray-50 section backgrounds. Layout: added global sticky footer (hidden on /admin/schedule, /account, and ?format=pdf pages), wrapped content in min-h-screen flex layout. PortfolioCarousel: set heading to Playfair Display. |