Block card saving for unverified accounts at the charge-handler level
Only verified accounts (account_role in 'verified_email','admin') may save cards. CreateBookingPayment, CreateTipPayment and BuyGiftCard now reject save_card=true for guests, unverified accounts and affiliates with 403 BEFORE charge-source resolution, the pending-payment insert, or any Square call — failing closed with zero side effects. Unverified users may still pay; only card persistence is blocked. The dedicated save endpoints are additionally protected by mw.RequireVerified middleware on the routes (main.go). isVerifiedRole / rejectSaveCardForUnverified mirror the existing isAdminRequest defense-in-depth pattern. Tests cover: unverified save-card 403 with no payment row, verified save-card succeeds, and unverified pay-without-save succeeds.
This commit is contained in:
@@ -983,6 +983,14 @@ func BuyGiftCard(w http.ResponseWriter, r *http.Request) {
|
|||||||
http.Error(w, "Invalid request", http.StatusBadRequest)
|
http.Error(w, "Invalid request", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Product rule (security): only verified accounts may save cards. An
|
||||||
|
// unverified/guest/affiliate user may still buy a gift card, but
|
||||||
|
// save_card=true is rejected here — before any charge source resolution.
|
||||||
|
if rejectSaveCardForUnverified(w, r, req.SaveCard) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if req.IdempotencyKey == "" {
|
if req.IdempotencyKey == "" {
|
||||||
req.IdempotencyKey = uniqueChargeKey("gc-")
|
req.IdempotencyKey = uniqueChargeKey("gc-")
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -148,6 +148,30 @@ func isAdminRequest(r *http.Request) bool {
|
|||||||
return ok && role == "admin"
|
return ok && role == "admin"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// isVerifiedRole reports whether the authenticated user may save cards
|
||||||
|
// (account_role in ('verified_email','admin')). Guests, unverified accounts,
|
||||||
|
// and affiliates may still pay but must never persist a card.
|
||||||
|
func isVerifiedRole(r *http.Request) bool {
|
||||||
|
role, ok := r.Context().Value(mw.UserRoleKey).(string)
|
||||||
|
return ok && (role == "verified_email" || role == "admin")
|
||||||
|
}
|
||||||
|
|
||||||
|
// rejectSaveCardForUnverified enforces the save-card product rule at the
|
||||||
|
// handler level for charge endpoints. When a charge request carries
|
||||||
|
// save_card=true for a non-verified user it responds 403 and returns true so
|
||||||
|
// the caller aborts BEFORE resolveChargeSource (where the card would be
|
||||||
|
// persisted), the pending payment record insert, or the Square call — failing
|
||||||
|
// closed with no side effects. A non-verified user may still pay; only card
|
||||||
|
// persistence is blocked. The dedicated save endpoints are additionally
|
||||||
|
// protected by mw.RequireVerified middleware (main.go).
|
||||||
|
func rejectSaveCardForUnverified(w http.ResponseWriter, r *http.Request, saveCard bool) bool {
|
||||||
|
if saveCard && !isVerifiedRole(r) {
|
||||||
|
http.Error(w, "Only verified accounts can save a card", http.StatusForbidden)
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
// GetDiscountPreviewHandler returns eligible discounts for a booking without applying them.
|
// GetDiscountPreviewHandler returns eligible discounts for a booking without applying them.
|
||||||
// GET /api/bookings/{id}/discount-preview
|
// GET /api/bookings/{id}/discount-preview
|
||||||
func GetDiscountPreviewHandler(w http.ResponseWriter, r *http.Request) {
|
func GetDiscountPreviewHandler(w http.ResponseWriter, r *http.Request) {
|
||||||
@@ -1194,6 +1218,13 @@ func CreateBookingPayment(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Product rule (security): only verified accounts may save cards. An
|
||||||
|
// unverified/guest/affiliate user may still pay, but save_card=true is
|
||||||
|
// rejected here — before any charge source resolution or payment record.
|
||||||
|
if rejectSaveCardForUnverified(w, r, req.SaveCard) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Resolve buyer email for Square receipt delivery (failure is non-fatal).
|
// Resolve buyer email for Square receipt delivery (failure is non-fatal).
|
||||||
var bookingBuyerEmail string
|
var bookingBuyerEmail string
|
||||||
if err := db.Conn.QueryRow(r.Context(), `SELECT email FROM users WHERE id = $1`, userID).Scan(&bookingBuyerEmail); err != nil {
|
if err := db.Conn.QueryRow(r.Context(), `SELECT email FROM users WHERE id = $1`, userID).Scan(&bookingBuyerEmail); err != nil {
|
||||||
@@ -2555,6 +2586,13 @@ func CreateTipPayment(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Product rule (security): only verified accounts may save cards. An
|
||||||
|
// unverified/guest/affiliate user may still tip, but save_card=true is
|
||||||
|
// rejected here — before any charge source resolution or payment record.
|
||||||
|
if rejectSaveCardForUnverified(w, r, req.SaveCard) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if err := ValidateAmount(req.Amount); err != nil {
|
if err := ValidateAmount(req.Amount); err != nil {
|
||||||
log.Printf("Failed to process request: %v", err)
|
log.Printf("Failed to process request: %v", err)
|
||||||
http.Error(w, "Invalid request", http.StatusBadRequest)
|
http.Error(w, "Invalid request", http.StatusBadRequest)
|
||||||
|
|||||||
@@ -0,0 +1,243 @@
|
|||||||
|
//go:build test && dev
|
||||||
|
|
||||||
|
package payments
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"crussell/clock"
|
||||||
|
"crussell/db"
|
||||||
|
"crussell/mw"
|
||||||
|
"crussell/testutils"
|
||||||
|
"crussell/testutils/fixtures"
|
||||||
|
"crussell/testutils/jwt"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi/v5"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Product rule (security): cards may only be saved by verified accounts
|
||||||
|
// (account_role in ('verified_email','admin')). Guests, unverified accounts,
|
||||||
|
// and affiliates may still pay, but must never persist a card. These tests
|
||||||
|
// cover both enforcement layers: the RequireVerified middleware on the
|
||||||
|
// dedicated payment-methods routes, and the handler-level save_card gate on
|
||||||
|
// the charge endpoints (CreateBookingPayment et al).
|
||||||
|
|
||||||
|
// serveThroughVerifiedChain invokes a handler through the same middleware
|
||||||
|
// stack the payment-methods routes are mounted under in main.go
|
||||||
|
// (RequireAuth → RequireVerified), carrying a real JWT, so the test exercises
|
||||||
|
// the actual route wiring rather than a bare handler call.
|
||||||
|
func serveThroughVerifiedChain(t *testing.T, handler http.Handler, method, path string, body interface{}, token string, ctx context.Context) *httptest.ResponseRecorder {
|
||||||
|
t.Helper()
|
||||||
|
var req *http.Request
|
||||||
|
if body != nil {
|
||||||
|
bodyBytes, err := json.Marshal(body)
|
||||||
|
require.NoError(t, err)
|
||||||
|
req = httptest.NewRequest(method, path, bytes.NewReader(bodyBytes))
|
||||||
|
req.Header.Set("Content-Type", "application/json")
|
||||||
|
} else {
|
||||||
|
req = httptest.NewRequest(method, path, nil)
|
||||||
|
}
|
||||||
|
if token != "" {
|
||||||
|
req.Header.Set("Authorization", "Bearer "+token)
|
||||||
|
}
|
||||||
|
|
||||||
|
rctx := chi.NewRouteContext()
|
||||||
|
if id, paramName := extractPaymentIDFromPath(path); id != "" {
|
||||||
|
rctx.URLParams.Add(paramName, id)
|
||||||
|
}
|
||||||
|
req = req.WithContext(context.WithValue(ctx, chi.RouteCtxKey, rctx))
|
||||||
|
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
handler.ServeHTTP(w, req)
|
||||||
|
return w
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestPaymentMethodsRoutes_RequireVerified verifies the mw.RequireVerified
|
||||||
|
// middleware on all three payment-methods routes: GET/POST/DELETE are all 403
|
||||||
|
// for an unverified user (before any handler code runs), while a verified user
|
||||||
|
// still reaches the handler.
|
||||||
|
func TestPaymentMethodsRoutes_RequireVerified(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
ctx, tx := testutils.SetupTestTx(t)
|
||||||
|
|
||||||
|
unverifiedID, err := fixtures.CreateTestUnverifiedUser(tx)
|
||||||
|
require.NoError(t, err)
|
||||||
|
unverifiedToken := jwt.GenerateUnverifiedUserToken(unverifiedID)
|
||||||
|
|
||||||
|
cardID, err := fixtures.CreateTestPaymentMethod(tx, unverifiedID, "ccof:unverified_saved", "VISA", "1111")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
verifiedID, err := fixtures.CreateTestUser(tx)
|
||||||
|
require.NoError(t, err)
|
||||||
|
verifiedToken := jwt.GenerateUserToken(verifiedID)
|
||||||
|
|
||||||
|
// POST /user/payment-methods is the primary save path — verify no card is
|
||||||
|
// persisted when an unverified user's request is rejected.
|
||||||
|
saveCardPath := "/api/user/payment-methods"
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
method string
|
||||||
|
path string
|
||||||
|
body interface{}
|
||||||
|
token string
|
||||||
|
want int
|
||||||
|
}{
|
||||||
|
{"GET unverified", http.MethodGet, "/api/user/payment-methods", nil, unverifiedToken, http.StatusForbidden},
|
||||||
|
{"POST unverified save blocked", http.MethodPost, saveCardPath, CreatePaymentMethodRequest{CardToken: "cnon:unverified-save"}, unverifiedToken, http.StatusForbidden},
|
||||||
|
{"DELETE unverified", http.MethodDelete, "/api/user/payment-methods/" + cardID, nil, unverifiedToken, http.StatusForbidden},
|
||||||
|
{"GET verified passes through", http.MethodGet, "/api/user/payment-methods", nil, verifiedToken, http.StatusOK},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
var route http.Handler
|
||||||
|
switch tc.method {
|
||||||
|
case http.MethodGet:
|
||||||
|
route = mw.RequireAuth(mw.RequireVerified(http.HandlerFunc(GetUserPaymentMethods)))
|
||||||
|
case http.MethodPost:
|
||||||
|
route = mw.RequireAuth(mw.RequireVerified(http.HandlerFunc(CreatePaymentMethod)))
|
||||||
|
case http.MethodDelete:
|
||||||
|
route = mw.RequireAuth(mw.RequireVerified(http.HandlerFunc(DeletePaymentMethod)))
|
||||||
|
}
|
||||||
|
w := serveThroughVerifiedChain(t, route, tc.method, tc.path, tc.body, tc.token, ctx)
|
||||||
|
require.Equal(t, tc.want, w.Code, "body: %s", w.Body.String())
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
var savedCount int
|
||||||
|
err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, unverifiedID).Scan(&savedCount)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, 1, savedCount, "only the fixture card exists — the rejected save attempt must not create one")
|
||||||
|
}
|
||||||
|
|
||||||
|
// setupTestDataForUserAt mirrors setupTestDataAtTime but for a caller-supplied
|
||||||
|
// user (so an unverified or otherwise non-default-role user owns the booking).
|
||||||
|
func setupTestDataForUserAt(t *testing.T, ctx context.Context, q db.Querier, userID string, startTime time.Time) (string, string) {
|
||||||
|
t.Helper()
|
||||||
|
serviceID, err := fixtures.CreateTestService(q)
|
||||||
|
require.NoError(t, err)
|
||||||
|
bookingID, err := fixtures.CreateTestBookingAtTime(q, userID, serviceID, startTime)
|
||||||
|
require.NoError(t, err)
|
||||||
|
_, err = q.Exec(ctx, "UPDATE bookings SET status = 'in_progress' WHERE id = $1", bookingID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
return bookingID, serviceID
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCreateBookingPayment_SaveCard_UnverifiedForbidden verifies the
|
||||||
|
// handler-level save-card gate: an unverified user sending save_card=true on a
|
||||||
|
// booking payment gets 403 with NO side effects — no payment record and no
|
||||||
|
// saved card, and no charge reaching Square.
|
||||||
|
func TestCreateBookingPayment_SaveCard_UnverifiedForbidden(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
ctx, tx := testutils.SetupTestTx(t)
|
||||||
|
|
||||||
|
unverifiedID, err := fixtures.CreateTestUnverifiedUser(tx)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
bookingID, _ := setupTestDataForUserAt(t, ctx, tx, unverifiedID, time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
|
||||||
|
|
||||||
|
unverifiedToken := jwt.GenerateUnverifiedUserToken(unverifiedID)
|
||||||
|
|
||||||
|
cardToken := "cnon:unverified-save-attempt"
|
||||||
|
req := CreateBookingPaymentRequest{
|
||||||
|
Amount: 5000,
|
||||||
|
PaymentType: "full",
|
||||||
|
NewCardToken: &cardToken,
|
||||||
|
SaveCard: true,
|
||||||
|
IdempotencyKey: "unverified-save-card-key-" + bookingID,
|
||||||
|
}
|
||||||
|
|
||||||
|
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, unverifiedToken, ctx)
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusForbidden, w.Code, "body: %s", w.Body.String())
|
||||||
|
|
||||||
|
var paymentCount int
|
||||||
|
err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1`, bookingID).Scan(&paymentCount)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Zero(t, paymentCount, "no payment record may be created for the rejected save-card request")
|
||||||
|
|
||||||
|
var cardCount int
|
||||||
|
err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, unverifiedID).Scan(&cardCount)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Zero(t, cardCount, "no card may be persisted for the rejected save-card request")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCreateBookingPayment_SaveCard_VerifiedSucceeds verifies the gate does
|
||||||
|
// not disturb verified users: the same save_card=true request from a
|
||||||
|
// verified_email account charges AND persists the card, exactly as before.
|
||||||
|
func TestCreateBookingPayment_SaveCard_VerifiedSucceeds(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
ctx, tx := testutils.SetupTestTx(t)
|
||||||
|
|
||||||
|
userID, bookingID, _ := setupTestData(t, ctx, tx)
|
||||||
|
|
||||||
|
userToken := jwt.GenerateUserToken(userID)
|
||||||
|
|
||||||
|
cardToken := "cnon:verified-save-card"
|
||||||
|
req := CreateBookingPaymentRequest{
|
||||||
|
Amount: 2500,
|
||||||
|
PaymentType: "deposit",
|
||||||
|
NewCardToken: &cardToken,
|
||||||
|
SaveCard: true,
|
||||||
|
IdempotencyKey: "verified-save-card-key-" + bookingID,
|
||||||
|
}
|
||||||
|
|
||||||
|
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, w.Code, "body: %s", w.Body.String())
|
||||||
|
|
||||||
|
var resp PaymentResponse
|
||||||
|
require.NoError(t, parsePaymentResponseBody(w, &resp))
|
||||||
|
require.Equal(t, "completed", resp.Status)
|
||||||
|
|
||||||
|
var cardCount int
|
||||||
|
err := tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, userID).Scan(&cardCount)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, 1, cardCount, "verified user's card must still be persisted")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCreateBookingPayment_NoSaveCard_UnverifiedSucceeds verifies the gate
|
||||||
|
// does not block payment for non-verified users: an unverified user paying
|
||||||
|
// WITHOUT save_card still charges successfully and saves nothing.
|
||||||
|
func TestCreateBookingPayment_NoSaveCard_UnverifiedSucceeds(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
ctx, tx := testutils.SetupTestTx(t)
|
||||||
|
|
||||||
|
unverifiedID, err := fixtures.CreateTestUnverifiedUser(tx)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
bookingID, _ := setupTestDataForUserAt(t, ctx, tx, unverifiedID, clock.Now().Add(-1*time.Hour))
|
||||||
|
|
||||||
|
unverifiedToken := jwt.GenerateUnverifiedUserToken(unverifiedID)
|
||||||
|
|
||||||
|
cardToken := "cnon:unverified-one-off"
|
||||||
|
req := CreateBookingPaymentRequest{
|
||||||
|
Amount: 5000,
|
||||||
|
PaymentType: "full",
|
||||||
|
NewCardToken: &cardToken,
|
||||||
|
SaveCard: false,
|
||||||
|
IdempotencyKey: "unverified-one-off-key-" + bookingID,
|
||||||
|
}
|
||||||
|
|
||||||
|
w := makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", req, unverifiedToken, ctx)
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, w.Code, "payment must not be blocked for unverified users paying without save_card: %s", w.Body.String())
|
||||||
|
|
||||||
|
var paymentCount int
|
||||||
|
err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM payments WHERE booking_id = $1`, bookingID).Scan(&paymentCount)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, 1, paymentCount, "the unverified user's one-off payment must be recorded")
|
||||||
|
|
||||||
|
var cardCount int
|
||||||
|
err = tx.QueryRow(ctx, `SELECT COUNT(*) FROM user_saved_cards WHERE user_id = $1`, unverifiedID).Scan(&cardCount)
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Zero(t, cardCount, "no card may be saved when save_card is false")
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user