Block guest-role tokens from online payment routes (RequireNonGuest middleware)
Guests (account_role='guest') have no login flow and never receive a JWT normally, so this is defense-in-depth: any token whose role claim is 'guest' (forged/minted guest tokens or future changes) is refused 403 before the money handlers run. The check reads role from context ONLY — real guests are seeded with account_type='email', so account_type is never the discriminator. A missing role passes through (RequireAuth guarantees presence; same trust model as isVerifiedRole). Wired onto all user-facing money routes: booking payment, apply-redemption, payment-lock POST/DELETE, tip, gift-card redeem and gift-card buy. Admin and till routes (RequireAdmin) are untouched — admin can never be guest. The payment-methods routes gain RequireVerified (verified_email, admin) alongside, so only verified accounts can manage saved cards. Tests: 6 middleware tests (reject guest, allow verified/unverified/admin/ affiliate/missing-role) + 4 integration tests (guest 403 on booking payment with zero side effects, tip, gift-card buy; verified user still pays 200).
This commit is contained in:
@@ -0,0 +1,150 @@
|
||||
//go:build test && dev
|
||||
|
||||
package payments
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"crussell/db"
|
||||
"crussell/mw"
|
||||
"crussell/testutils"
|
||||
"crussell/testutils/fixtures"
|
||||
"crussell/testutils/jwt"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// withNonGuest wraps a payment handler in the RequireNonGuest middleware, the
|
||||
// same chain main.go mounts for user-facing money routes (after RequireAuth).
|
||||
func withNonGuest(handler http.HandlerFunc) http.HandlerFunc {
|
||||
return mw.RequireNonGuest(handler).ServeHTTP
|
||||
}
|
||||
|
||||
// createGuestBooking sets up a guest user with an in_progress booking so a
|
||||
// charge would otherwise succeed if the guest guard did not block it.
|
||||
func createGuestBooking(t *testing.T, ctx context.Context, q db.Querier) (string, string) {
|
||||
guestID, err := fixtures.CreateTestGuestUser(q)
|
||||
require.NoError(t, err)
|
||||
|
||||
serviceID, err := fixtures.CreateTestService(q)
|
||||
require.NoError(t, err)
|
||||
|
||||
bookingID, err := fixtures.CreateTestBookingAtTime(q, guestID, serviceID, time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC))
|
||||
require.NoError(t, err)
|
||||
|
||||
_, err = q.Exec(ctx, "UPDATE bookings SET status = 'in_progress' WHERE id = $1", bookingID)
|
||||
require.NoError(t, err)
|
||||
|
||||
return guestID, bookingID
|
||||
}
|
||||
|
||||
func TestGuestBlocked_CreateBookingPayment(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx, tx := testutils.SetupTestTx(t)
|
||||
|
||||
guestID, bookingID := createGuestBooking(t, ctx, tx)
|
||||
guestToken := jwt.GenerateTestToken(guestID, "guest")
|
||||
|
||||
cardToken := "cnon:test-card-nonce"
|
||||
req := CreateBookingPaymentRequest{
|
||||
Amount: 2500,
|
||||
PaymentType: "deposit",
|
||||
NewCardToken: &cardToken,
|
||||
IdempotencyKey: "guest-block-payment-key",
|
||||
}
|
||||
|
||||
handler := withNonGuest(CreateBookingPayment)
|
||||
w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/payment", req, guestToken, ctx)
|
||||
|
||||
if w.Code != http.StatusForbidden {
|
||||
t.Errorf("expected 403 for guest payment, got %d. body: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
var count int
|
||||
if err := tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1", bookingID).Scan(&count); err != nil {
|
||||
t.Fatalf("failed to query payments: %v", err)
|
||||
}
|
||||
if count != 0 {
|
||||
t.Errorf("expected 0 payment rows for blocked guest, got %d", count)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGuestBlocked_CreateTipPayment(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx, tx := testutils.SetupTestTx(t)
|
||||
|
||||
guestID, bookingID := createGuestBooking(t, ctx, tx)
|
||||
guestToken := jwt.GenerateTestToken(guestID, "guest")
|
||||
|
||||
cardToken := "cnon:tip-card"
|
||||
req := CreateTipPaymentRequest{
|
||||
Amount: 500,
|
||||
NewCardToken: &cardToken,
|
||||
}
|
||||
|
||||
handler := withNonGuest(CreateTipPayment)
|
||||
w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/tip", req, guestToken, ctx)
|
||||
|
||||
if w.Code != http.StatusForbidden {
|
||||
t.Errorf("expected 403 for guest tip, got %d. body: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestGuestBlocked_BuyGiftCard(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx, tx := testutils.SetupTestTx(t)
|
||||
|
||||
guestID, err := fixtures.CreateTestGuestUser(tx)
|
||||
require.NoError(t, err)
|
||||
guestToken := jwt.GenerateTestToken(guestID, "guest")
|
||||
|
||||
newToken := "cnon:test-card"
|
||||
req := BuyGiftCardRequest{
|
||||
Amount: 1000,
|
||||
RecipientType: "self",
|
||||
NewCardToken: &newToken,
|
||||
IdempotencyKey: "guest-block-gc-key",
|
||||
}
|
||||
|
||||
handler := withNonGuest(BuyGiftCard)
|
||||
w := makePaymentRequest(handler, "POST", "/api/user/giftcards/buy", req, guestToken, ctx)
|
||||
|
||||
if w.Code != http.StatusForbidden {
|
||||
t.Errorf("expected 403 for guest gift card purchase, got %d. body: %s", w.Code, w.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestGuestBlocked_VerifiedUserStillPays(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx, tx := testutils.SetupTestTx(t)
|
||||
|
||||
userID, bookingID, _ := setupTestData(t, ctx, tx)
|
||||
userToken := jwt.GenerateUserToken(userID)
|
||||
|
||||
cardToken := "cnon:test-card-nonce"
|
||||
req := CreateBookingPaymentRequest{
|
||||
Amount: 2500,
|
||||
PaymentType: "deposit",
|
||||
NewCardToken: &cardToken,
|
||||
SaveCard: true,
|
||||
IdempotencyKey: "guest-block-verified-key",
|
||||
}
|
||||
|
||||
handler := withNonGuest(CreateBookingPayment)
|
||||
w := makePaymentRequest(handler, "POST", "/api/bookings/"+bookingID+"/payment", req, userToken, ctx)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for verified user through RequireNonGuest, got %d. body: %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
var count int
|
||||
if err := tx.QueryRow(ctx, "SELECT COUNT(*) FROM payments WHERE booking_id = $1", bookingID).Scan(&count); err != nil {
|
||||
t.Fatalf("failed to query payments: %v", err)
|
||||
}
|
||||
if count != 1 {
|
||||
t.Errorf("expected 1 payment for verified user, got %d", count)
|
||||
}
|
||||
}
|
||||
+16
-10
@@ -385,22 +385,28 @@ func main() {
|
||||
r.Delete("/bookings/reserve", bookings.CancelReservationHandler)
|
||||
|
||||
// User payment routes
|
||||
r.Post("/bookings/{id}/payment", payments.CreateBookingPayment)
|
||||
r.Post("/bookings/{id}/apply-redemption", payments.ApplyLoyaltyRedemption)
|
||||
r.Post("/bookings/{id}/payment-lock", payments.AcquirePaymentLock)
|
||||
r.Delete("/bookings/{id}/payment-lock", payments.ReleasePaymentLock)
|
||||
r.Get("/user/payment-methods", payments.GetUserPaymentMethods)
|
||||
r.Post("/user/payment-methods", payments.CreatePaymentMethod)
|
||||
r.Delete("/user/payment-methods/{id}", payments.DeletePaymentMethod)
|
||||
r.Post("/bookings/{id}/tip", payments.CreateTipPayment)
|
||||
// Product rule (security): guests (account_role='guest') must not
|
||||
// pay online — RequireNonGuest 403s any token with a guest role
|
||||
// claim before the money handlers run.
|
||||
r.With(mw.RequireNonGuest).Post("/bookings/{id}/payment", payments.CreateBookingPayment)
|
||||
r.With(mw.RequireNonGuest).Post("/bookings/{id}/apply-redemption", payments.ApplyLoyaltyRedemption)
|
||||
r.With(mw.RequireNonGuest).Post("/bookings/{id}/payment-lock", payments.AcquirePaymentLock)
|
||||
r.With(mw.RequireNonGuest).Delete("/bookings/{id}/payment-lock", payments.ReleasePaymentLock)
|
||||
// Product rule (security): cards may only be saved by verified
|
||||
// accounts — RequireAuth (group mw above) runs first and injects
|
||||
// userID/role into ctx, then RequireVerified 403s the rest.
|
||||
r.With(mw.RequireVerified).Get("/user/payment-methods", payments.GetUserPaymentMethods)
|
||||
r.With(mw.RequireVerified).Post("/user/payment-methods", payments.CreatePaymentMethod)
|
||||
r.With(mw.RequireVerified).Delete("/user/payment-methods/{id}", payments.DeletePaymentMethod)
|
||||
r.With(mw.RequireNonGuest).Post("/bookings/{id}/tip", payments.CreateTipPayment)
|
||||
r.Get("/bookings/{id}/payment-summary", payments.GetBookingPaymentSummary)
|
||||
r.With(mw.RateLimit(10, time.Minute), limitBody(defaultBodyLimit)).
|
||||
Get("/bookings/{id}/discount-preview", payments.GetDiscountPreviewHandler)
|
||||
|
||||
// User gift card routes
|
||||
r.Post("/user/giftcards/redeem", payments.RedeemGiftCard)
|
||||
r.With(mw.RequireNonGuest).Post("/user/giftcards/redeem", payments.RedeemGiftCard)
|
||||
r.Get("/user/giftcards/balance", payments.GetGiftCardBalance)
|
||||
r.Post("/user/giftcards/buy", payments.BuyGiftCard)
|
||||
r.With(mw.RequireNonGuest).Post("/user/giftcards/buy", payments.BuyGiftCard)
|
||||
})
|
||||
|
||||
r.With(mw.RequireAuth, mw.RequireVerified, limitBody(uploadBodyLimit)).Post("/user/profile-picture", user.UploadProfilePictureHandler)
|
||||
|
||||
@@ -95,6 +95,25 @@ func RequireVerified(next http.Handler) http.Handler {
|
||||
return RequireRole("verified_email", "admin")(next)
|
||||
}
|
||||
|
||||
// RequireNonGuest middleware - blocks guest-role tokens from money routes.
|
||||
// Guests are created with account_role='guest' (account_type is NOT checked —
|
||||
// real guests are seeded with account_type='email'), and they have no login
|
||||
// flow, so a legitimate guest never holds a JWT. This is a defense-in-depth
|
||||
// guard: any token whose role claim is 'guest' (forged/minted guest tokens or
|
||||
// future changes) is refused. A missing role is treated as not-guest and passes
|
||||
// through — RequireAuth guarantees the role is present when this runs, matching
|
||||
// the trust model of the in-handler isVerifiedRole checks.
|
||||
func RequireNonGuest(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
role, ok := r.Context().Value(UserRoleKey).(string)
|
||||
if ok && role == "guest" {
|
||||
RespondJSON(w, http.StatusForbidden, map[string]string{"error": "forbidden"})
|
||||
return
|
||||
}
|
||||
next.ServeHTTP(w, r)
|
||||
})
|
||||
}
|
||||
|
||||
// RequireAdmin middleware - only allows admin and validates the user ID is present
|
||||
// in context, so handlers don't need to re-check. The user ID is always set by
|
||||
// RequireAuth before this runs, but this adds a defensive safety net.
|
||||
|
||||
@@ -307,6 +307,81 @@ func TestRequireVerified_Unverified(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// RequireNonGuest (blocks account_role='guest' tokens from money routes)
|
||||
// =============================================================================
|
||||
|
||||
func TestRequireNonGuest_AllowsVerifiedEmail(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
ctx := context.WithValue(req.Context(), UserRoleKey, "verified_email")
|
||||
req = req.WithContext(ctx)
|
||||
w := httptest.NewRecorder()
|
||||
RequireNonGuest(testHandler()).ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for verified_email, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequireNonGuest_AllowsUnverifiedEmail(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
ctx := context.WithValue(req.Context(), UserRoleKey, "unverified_email")
|
||||
req = req.WithContext(ctx)
|
||||
w := httptest.NewRecorder()
|
||||
RequireNonGuest(testHandler()).ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for unverified_email, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequireNonGuest_AllowsAdmin(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
ctx := context.WithValue(req.Context(), UserRoleKey, "admin")
|
||||
req = req.WithContext(ctx)
|
||||
w := httptest.NewRecorder()
|
||||
RequireNonGuest(testHandler()).ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for admin, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequireNonGuest_AllowsAffiliate(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
ctx := context.WithValue(req.Context(), UserRoleKey, "affiliate")
|
||||
req = req.WithContext(ctx)
|
||||
w := httptest.NewRecorder()
|
||||
RequireNonGuest(testHandler()).ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 for affiliate, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequireNonGuest_RejectsGuest(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
ctx := context.WithValue(req.Context(), UserRoleKey, "guest")
|
||||
req = req.WithContext(ctx)
|
||||
w := httptest.NewRecorder()
|
||||
RequireNonGuest(testHandler()).ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusForbidden {
|
||||
t.Errorf("expected 403 for guest, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRequireNonGuest_NoRoleInContext(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "/", nil)
|
||||
// No UserRoleKey in context — treated as not-guest, passes through
|
||||
w := httptest.NewRecorder()
|
||||
RequireNonGuest(testHandler()).ServeHTTP(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Errorf("expected 200 when no role in context, got %d", w.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// RequireAdmin (RequireRole("admin") + UserIDKey guard)
|
||||
// =============================================================================
|
||||
|
||||
Reference in New Issue
Block a user