From 8b753e4f4bbb4d8dfce6f50383b33e56ccc2011f Mon Sep 17 00:00:00 2001 From: marcus06 Date: Sat, 6 Jun 2026 21:06:25 +0200 Subject: [PATCH] feat(sca): add requires_action orde status for 3ds in flight sate (#36) * feat(sca): add requires_action orde status for 3ds in flight sate * fix(sca): fix stuff the review commented on --- api/handlers/cart.go | 31 +++-- api/handlers/cart_test.go | 141 +++++++++++++++++++ api/handlers/webhook.go | 2 +- api/handlers/webhook_test.go | 224 +++++++++++++++++++++++++++++++ pkg/models/order.go | 15 ++- pkg/models/order_display.go | 4 + pkg/models/order_display_test.go | 1 + 7 files changed, 398 insertions(+), 20 deletions(-) create mode 100644 api/handlers/webhook_test.go diff --git a/api/handlers/cart.go b/api/handlers/cart.go index 122d8ed..c9065ca 100644 --- a/api/handlers/cart.go +++ b/api/handlers/cart.go @@ -539,9 +539,10 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) { }) } -// respondRequiresAction handles a PaymentIntent that needs 3DS auth -// The order stays pending. the webhook flips it to paid once the customer -// completes the bank-hosted challenge, the tui polls /api/v1/orders/{id}/status +// respondRequiresAction handles a PaymentIntent that needs 3DS auth. The order +// transitions to requires_action (see caveat #20 in sca-psd2-compliance.md); +// the payment_intent.succeeded webhook flips it to paid once the customer +// completes the bank-hosted challenge. The TUI polls /api/v1/orders/{id}/status. func (h *CartHandler) respondRequiresAction(w http.ResponseWriter, order *models.Order, paymentMethodID string, pi *stripe.PaymentIntent) { db := database.GetDB() @@ -568,13 +569,15 @@ func (h *CartHandler) respondRequiresAction(w http.ResponseWriter, order *models return } - // Persist the paymentIntent id AND reset status to pending. The - // payment_intent.payment_failed webhook from the initial off-session decline - // often arrives before this response and flips the order to failed — - // override it back so the 3DS-in-flight order shows the correct state. + // Persist the paymentIntent id AND set status to requires_action so list + // UIs can distinguish "charging" from "awaiting customer 3DS". The + // payment_intent.payment_failed webhook from the initial off-session + // decline often arrives before this response and flips the order to + // failed - override it back so the 3DS-in-flight order shows the + // correct state if err := db.Model(order).Updates(map[string]any{ "stripe_payment_id": pi.ID, - "status": models.OrderStatusPending, + "status": models.OrderStatusRequiresAction, }).Error; err != nil { utils.RespondError(w, http.StatusInternalServerError, "DATABASE_ERROR", "failed to save payment id", nil) return @@ -658,10 +661,14 @@ func (h *CartHandler) RetryAuth(w http.ResponseWriter, r *http.Request) { return } - // Only pending (initial attempt mid-flight) and failed (webhook flipped it - // after a non-recoverable error) are retryable. paid, refundedn shipped, - // delivered, cancelled are terminal - refuse - if order.Status != models.OrderStatusPending && order.Status != models.OrderStatusFailed { + // Retryable source states: + // pending - initial attempt mid-flight (pre-PI confirm page) + // requires_action - customer abandoned or failed the 3DS challenge + // failed - webhook flipped after a non-recoverable error + // terminal states (paid, shipped, delivered, cancelled, refunded) refuse. + if order.Status != models.OrderStatusPending && + order.Status != models.OrderStatusRequiresAction && + order.Status != models.OrderStatusFailed { utils.RespondError(w, http.StatusConflict, "INVALID_STATE", "order is already finalized", nil) return } diff --git a/api/handlers/cart_test.go b/api/handlers/cart_test.go index ec6b61e..52d5795 100644 --- a/api/handlers/cart_test.go +++ b/api/handlers/cart_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/go-chi/chi/v5" + "github.com/stripe/stripe-go/v78" "terminalShop/api/middleware" "terminalShop/pkg/audit" @@ -919,6 +920,12 @@ func TestRetryAuth(t *testing.T) { seedPayment: "pi_test_refunded", seedEvents: 1, wantStatus: http.StatusConflict, wantErrCode: "INVALID_STATE", }, + { + name: "requires_action honors retry cap", + ownedByUser: true, seedStatus: models.OrderStatusRequiresAction, + seedPayment: "pi_test_req_action_cap", seedEvents: maxAuthIssuances, + wantStatus: http.StatusTooManyRequests, wantErrCode: "RETRY_LIMIT", + }, { name: "retry cap reached returns 429", ownedByUser: true, seedStatus: models.OrderStatusFailed, @@ -1022,3 +1029,137 @@ func TestRetryAuth_OrderNotFound(t *testing.T) { t.Fatalf("want 404, got %d body=%s", w.Code, w.Body.String()) } } + +func TestRespondRequiresAction_SetStatusRequiresAction(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + card := models.Card{ + UserID: user.ID, StripePaymentID: "pm_test_req_action", + Last4: "4242", Brand: "Visa", ExpMonth: 12, ExpYear: 2030, + } + if err := db.Create(&card).Error; err != nil { + t.Fatalf("seed card: %v", err) + } + + order := models.Order{ + UserID: user.ID, CardID: card.ID, + Status: models.OrderStatusPending, + Subtotal: 500, Total: 500, + ShippingName: "Test", ShippingStreet: "1 Main St", + ShippingCity: "PDX", ShippingState: "OR", + ShippingZip: "97201", ShippingCountry: "US", + } + if err := db.Create(&order).Error; err != nil { + t.Fatalf("seed order: %v", err) + } + + pi := &stripe.PaymentIntent{ + ID: "pi_test_action_set", + NextAction: &stripe.PaymentIntentNextAction{ + RedirectToURL: &stripe.PaymentIntentNextActionRedirectToURL{ + URL: "https://hooks.stripe.com/redirect/test/test_3ds", + }, + }, + } + + handler := NewCartHandler("", "http://localhost", 0) + w := httptest.NewRecorder() + handler.respondRequiresAction(w, &order, card.StripePaymentID, pi) + + if w.Code != http.StatusAccepted { + t.Fatalf("want 202, got %d body=%s", w.Code, w.Body.String()) + } + + var resp struct { + Data struct { + OrderID uint `json:"order_id"` + PaymentIntentID string `json:"payment_intent_id"` + Status string `json:"status"` + RedirectURL string `json:"redirect_url"` + } `json:"data"` + } + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v", err) + } + if resp.Data.Status != "requires_action" { + t.Fatalf("response status: %q, got %q", "requires_action", resp.Data.Status) + } + if resp.Data.RedirectURL == "" { + t.Fatalf("expected redirect url, got empty") + } + if resp.Data.PaymentIntentID != pi.ID { + t.Fatalf("payment intent id: want %q, got %q", pi.ID, resp.Data.PaymentIntentID) + } + + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload order: %v", err) + } + if reloaded.Status != models.OrderStatusRequiresAction { + t.Fatalf("order.Status: want %q, got %q", models.OrderStatusRequiresAction, reloaded.Status) + } + if reloaded.StripePaymentID != pi.ID { + t.Fatalf("order.StripePaymentID: want %q, got %q", pi.ID, reloaded.StripePaymentID) + } +} + +// TestRespondRequiresAction_OverridesFailedFromWebhookRace simulates the race +// described in sca-psd2-compliance.md note #3 / caveat #4: the +// payment_intent.payment_failed webhook arrives BEFORE respondRequiresAction +// returns and flips the row to OrderStatusFailed. The helper must override +// that back to OrderStatusRequiresAction so the in-flight 3DS state surfaces +// correctly to the TUI. +func TestRespondRequiresAction_OverridesFailedFromWebhookRace(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + card := models.Card{ + UserID: user.ID, StripePaymentID: "pm_test_race", + Last4: "4242", Brand: "Visa", ExpMonth: 12, ExpYear: 2030, + } + if err := db.Create(&card).Error; err != nil { + t.Fatalf("seed card: %v", err) + } + + order := models.Order{ + UserID: user.ID, CardID: card.ID, + Status: models.OrderStatusFailed, + Subtotal: 500, Total: 500, + ShippingName: "Test", ShippingStreet: "1 Main St", + ShippingCity: "PDX", ShippingState: "OR", + ShippingZip: "97201", ShippingCountry: "US", + } + if err := db.Create(&order).Error; err != nil { + t.Fatalf("seed order: %v", err) + } + + pi := &stripe.PaymentIntent{ + ID: "pi_test_race", + NextAction: &stripe.PaymentIntentNextAction{ + RedirectToURL: &stripe.PaymentIntentNextActionRedirectToURL{ + URL: "https://hooks.stripe.com/redirect/test_race", + }, + }, + } + + handler := NewCartHandler("", "http://localhost", 0) + w := httptest.NewRecorder() + handler.respondRequiresAction(w, &order, card.StripePaymentID, pi) + + if w.Code != http.StatusAccepted { + t.Fatalf("want 202, got %d body=%s", w.Code, w.Body.String()) + } + + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload order: %v", err) + } + if reloaded.Status != models.OrderStatusRequiresAction { + t.Fatalf("override failed: want %q, got %q", models.OrderStatusRequiresAction, reloaded.Status) + } +} diff --git a/api/handlers/webhook.go b/api/handlers/webhook.go index 2644f84..fdcd569 100644 --- a/api/handlers/webhook.go +++ b/api/handlers/webhook.go @@ -184,7 +184,7 @@ func (h *WebhookHandler) handlePaymentIntentFailed(ctx context.Context, event st } db := database.GetDB().WithContext(ctx) - if err := db.Model(&models.Order{}).Where("id = ? AND status = ?", orderIDStr, models.OrderStatusPending). + if err := db.Model(&models.Order{}).Where("id = ? AND status IN (?, ?)", orderIDStr, models.OrderStatusPending, models.OrderStatusRequiresAction). Update("status", models.OrderStatusFailed).Error; err != nil { webhookLog().Error("failed to mark order failed", "order_id", orderIDStr, "error", err) } diff --git a/api/handlers/webhook_test.go b/api/handlers/webhook_test.go new file mode 100644 index 0000000..988202a --- /dev/null +++ b/api/handlers/webhook_test.go @@ -0,0 +1,224 @@ +package handlers + +import ( + "context" + "encoding/json" + "fmt" + "os" + "testing" + + "github.com/stripe/stripe-go/v78" + + "terminalShop/pkg/database" + "terminalShop/pkg/models" +) + +// buildPaymentFailedEvent constructs a stripe.Event whose Data.Raw decodes to +// a PaymentIntent referencing the given order via metadata. errCode is set on +// last_payment_error to drive the authentication_required short-circuit +// branch in handlePaymentIntentFailed (pass "" to omit the error block). +func buildPaymentFailedEvent(t *testing.T, piID string, orderID uint, errCode string) stripe.Event { + t.Helper() + pi := map[string]any{ + "id": piID, + "metadata": map[string]string{"order_id": fmt.Sprintf("%d", orderID)}, + } + if errCode != "" { + pi["last_payment_error"] = map[string]any{"code": errCode} + } + raw, err := json.Marshal(pi) + if err != nil { + t.Fatalf("marshal pi: %v", err) + } + return stripe.Event{ + Type: "payment_intent.payment_failed", + Data: &stripe.EventData{Raw: raw}, + } +} + +func seedOrderForWebhook(t *testing.T, userID uint, status models.OrderStatus, piID string) models.Order { + t.Helper() + db := database.GetDB() + card := models.Card{ + UserID: userID, + StripePaymentID: fmt.Sprintf("pm_test_%s", piID), + Last4: "4242", Brand: "Visa", ExpMonth: 12, ExpYear: 2030, + } + if err := db.Create(&card).Error; err != nil { + t.Fatalf("seed card: %v", err) + } + order := models.Order{ + UserID: userID, CardID: card.ID, + StripePaymentID: piID, + Status: status, + Subtotal: 500, Total: 500, + ShippingName: "Test", ShippingStreet: "1 Main St", + ShippingCity: "PDX", ShippingState: "OR", + ShippingZip: "97201", ShippingCountry: "US", + } + if err := db.Create(&order).Error; err != nil { + t.Fatalf("seed order: %v", err) + } + return order +} + +// TestHandlePaymentIntentFailed_RequiresActionFlipsToFailed verifies the +// widened predicate (IN (pending, requires_action)) catches a row in the new +// requires_action state. Without this case, a card_declined webhook arriving +// after respondRequiresAction wrote requires_action would leave the row +// stuck forever. +func TestHandlePaymentIntentFailed_RequiresActionFlipsToFailed(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + order := seedOrderForWebhook(t, user.ID, models.OrderStatusRequiresAction, "pi_test_req_action_failed") + + h := NewWebhookHandler("", "", "") + evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined") + h.handlePaymentIntentFailed(context.Background(), evt) + + db := database.GetDB() + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload order: %v", err) + } + if reloaded.Status != models.OrderStatusFailed { + t.Fatalf("status: want %q, got %q", models.OrderStatusFailed, reloaded.Status) + } +} + +// TestHandlePaymentIntentFailed_RequiresActionAuthRequiredShortCircuits +// verifies the existing short-circuit (note #3 in sca-psd2-compliance.md) +// still fires when the row is in requires_action and the error code is +// authentication_required. Without the short-circuit, the recovery flow +// would race the webhook and surface a momentarily-failed state to the TUI. +func TestHandlePaymentIntentFailed_RequiresActionAuthRequiredShortCircuits(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + order := seedOrderForWebhook(t, user.ID, models.OrderStatusRequiresAction, "pi_test_req_action_authreq") + + h := NewWebhookHandler("", "", "") + evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "authentication_required") + h.handlePaymentIntentFailed(context.Background(), evt) + + db := database.GetDB() + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload order: %v", err) + } + if reloaded.Status != models.OrderStatusRequiresAction { + t.Fatalf("auth_required must not flip requires_action; got %q", reloaded.Status) + } +} + +// TestHandlePaymentIntentFailed_PendingStillFlips guards the pre-existing +// pending -> failed transition against a regression where the widened +// predicate accidentally dropped pending from the source set. +func TestHandlePaymentIntentFailed_PendingStillFlips(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + order := seedOrderForWebhook(t, user.ID, models.OrderStatusPending, "pi_test_pending_failed") + + h := NewWebhookHandler("", "", "") + evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined") + h.handlePaymentIntentFailed(context.Background(), evt) + + db := database.GetDB() + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload order: %v", err) + } + if reloaded.Status != models.OrderStatusFailed { + t.Fatalf("status: want %q, got %q", models.OrderStatusFailed, reloaded.Status) + } +} + +// TestHandlePaymentIntentFailed_PaidNotTouched locks the upper boundary of +// the predicate: a paid row must NEVER be flipped to failed by a stale +// payment_failed webhook. Without this guard, a future maintainer could +// widen the IN clause to include paid (e.g. for some refund-retry flow) +// and silently break idempotency. +func TestHandlePaymentIntentFailed_PaidNotTouched(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + order := seedOrderForWebhook(t, user.ID, models.OrderStatusPaid, "pi_test_paid_untouched") + + h := NewWebhookHandler("", "", "") + evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined") + h.handlePaymentIntentFailed(context.Background(), evt) + + db := database.GetDB() + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload order: %v", err) + } + if reloaded.Status != models.OrderStatusPaid { + t.Fatalf("paid order was modified by payment_failed webhook: got %q", reloaded.Status) + } +} + +// TestHandlePaymentIntentFailed_MissingMetadataNoOp covers the no-order +// short-circuit so the test file exercises every early-return branch in +// handlePaymentIntentFailed touched by the predicate work. +func TestHandlePaymentIntentFailed_MissingMetadataNoOp(t *testing.T) { + testDB, _ := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + pi := map[string]any{"id": "pi_test_no_meta"} + raw, err := json.Marshal(pi) + if err != nil { + t.Fatalf("marshal pi: %v", err) + } + evt := stripe.Event{ + Type: "payment_intent.payment_failed", + Data: &stripe.EventData{Raw: raw}, + } + + h := NewWebhookHandler("", "", "") + h.handlePaymentIntentFailed(context.Background(), evt) // must not panic +} + +func TestHandlePaymentIntentSucceeded_RequiresActionFlipsToPaid(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + order := seedOrderForWebhook(t, user.ID, models.OrderStatusRequiresAction, "pi_test_req_action_paid") + + pi := map[string]any{ + "id": order.StripePaymentID, + "amount": order.Total, + "metadata": map[string]string{"order_id": fmt.Sprintf("%d", order.ID)}, + } + raw, err := json.Marshal(pi) + if err != nil { + t.Fatalf("marshal pi: %v", err) + } + evt := stripe.Event{ + Type: "payment_intent.succeeded", + Data: &stripe.EventData{Raw: raw}, + } + + h := NewWebhookHandler("", "", "") + h.handlePaymentIntentSucceeded(context.Background(), evt) + + db := database.GetDB() + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload order: %v", err) + } + if reloaded.Status != models.OrderStatusPaid { + t.Fatalf("status: want %q, got %q", models.OrderStatusPaid, reloaded.Status) + } + if reloaded.StripePaymentID != order.StripePaymentID { + t.Fatalf("stripe_payment_id: want %q, got %q", order.StripePaymentID, reloaded.StripePaymentID) + } +} diff --git a/pkg/models/order.go b/pkg/models/order.go index 3273c64..5a47701 100644 --- a/pkg/models/order.go +++ b/pkg/models/order.go @@ -10,13 +10,14 @@ import ( type OrderStatus string const ( - OrderStatusPending OrderStatus = "pending" - OrderStatusPaid OrderStatus = "paid" - OrderStatusShipped OrderStatus = "shipped" - OrderStatusDelivered OrderStatus = "delivered" - OrderStatusCancelled OrderStatus = "cancelled" - OrderStatusRefunded OrderStatus = "refunded" - OrderStatusFailed OrderStatus = "failed" + OrderStatusPending OrderStatus = "pending" + OrderStatusRequiresAction OrderStatus = "requires_action" + OrderStatusPaid OrderStatus = "paid" + OrderStatusShipped OrderStatus = "shipped" + OrderStatusDelivered OrderStatus = "delivered" + OrderStatusCancelled OrderStatus = "cancelled" + OrderStatusRefunded OrderStatus = "refunded" + OrderStatusFailed OrderStatus = "failed" ) type TrackingStatus string diff --git a/pkg/models/order_display.go b/pkg/models/order_display.go index b7ddb5e..c13de00 100644 --- a/pkg/models/order_display.go +++ b/pkg/models/order_display.go @@ -35,6 +35,8 @@ func (order *Order) DisplayState() string { switch order.Status { case OrderStatusPending: return "Awaiting payment" + case OrderStatusRequiresAction: + return "Awaiting authentication" case OrderStatusPaid: if order.TrackingNumber == "" { return "Paid * awaiting label" @@ -77,6 +79,8 @@ func (order *Order) DisplayKind() DisplayKind { switch order.Status { case OrderStatusPending: return DisplayKindNeutral + case OrderStatusRequiresAction: + return DisplayKindNeutral case OrderStatusPaid: return DisplayKindAccent case OrderStatusShipped: diff --git a/pkg/models/order_display_test.go b/pkg/models/order_display_test.go index 6321292..3d218a6 100644 --- a/pkg/models/order_display_test.go +++ b/pkg/models/order_display_test.go @@ -14,6 +14,7 @@ func TestOrderDisplayState(t *testing.T) { kind DisplayKind }{ {"pending", Order{Status: OrderStatusPending}, "Awaiting payment", DisplayKindNeutral}, + {"requires_action", Order{Status: OrderStatusRequiresAction}, "Awaiting authentication", DisplayKindNeutral}, {"paid no tracking", Order{Status: OrderStatusPaid}, "Paid * awaiting label", DisplayKindAccent}, {"paid with tracking, no event", Order{Status: OrderStatusPaid, TrackingNumber: "12999"}, "Paid", DisplayKindAccent}, {"Paid pre-transit", Order{Status: OrderStatusPaid, TrackingNumber: "12999", TrackingStatus: TrackingStatusPreTransit}, "Paid * ready to ship", DisplayKindAccent},