From 3420c8dce0da224c9ed564f90a465ad0fdbaa93b Mon Sep 17 00:00:00 2001 From: marcus06 Date: Wed, 3 Jun 2026 23:42:27 +0200 Subject: [PATCH] feat(sca): add retry-auth endpoint for failed 3ds challenges (#31) * feat(sca): add retry-auth endpoint for failed 3ds challenges * fix(sca): retry-auth unexpected-state branch out of card-decline just fixes up in some of the things the review of the last commit noticed --- api/handlers/cart.go | 152 ++++++++++++++++++++++++++++++++++++++ api/handlers/cart_test.go | 147 ++++++++++++++++++++++++++++++++++++ api/routes/routes.go | 1 + pkg/api/client.go | 52 +++++++++++++ pkg/audit/audit.go | 8 +- 5 files changed, 358 insertions(+), 2 deletions(-) diff --git a/api/handlers/cart.go b/api/handlers/cart.go index f922906..d76ca0f 100644 --- a/api/handlers/cart.go +++ b/api/handlers/cart.go @@ -5,8 +5,10 @@ import ( "encoding/json" "fmt" "net/http" + "strconv" "time" + "github.com/go-chi/chi/v5" "github.com/stripe/stripe-go/v78" "github.com/stripe/stripe-go/v78/paymentintent" "github.com/stripe/stripe-go/v78/paymentmethod" @@ -28,6 +30,12 @@ type CartHandler struct { maxOrderCents int } +// maxAuthIssuances caps how many times a single ordes's PaymentIntent can be +// pushed into requires_action; one initial challende plus up to three retries +// before we tell the customer to use a different card. Counted via existing +// audit rows (audit.EventOrderRequiresAction) so the cap stays migration-free. +const maxAuthIssuances = 4 + // NewCartHandler creates a new cart handler with the Stripe secret key func NewCartHandler(stripeSecretKey, appURL string, maxOrderCents int) *CartHandler { return &CartHandler{stripeKey: stripeSecretKey, appURL: appURL, maxOrderCents: maxOrderCents} @@ -622,3 +630,147 @@ func appURLForHandler(r *http.Request) string { } return fmt.Sprintf("%s://%s", scheme, host) } + +// RetryAuth re-Confirms an in-flight PaymentIntent that the customer failed +// or abandoned during the 3DS challenge, recovering the existing order rather +// than starting a new checkout. Bound to POST /api/v1/orders/{id}/retry-auth. +// +// Stripe's PI state machine bounces back to requires_payment_method after a +// failed or abandoned 3DS, so paymentintent.Confirm(pi.id, …) mints a fresh +// next_action.redirect_to_url — the same shape respondRequiresAction already +// handles. No idempotency key is set: Confirm mutates an existing PI and +// Stripe collapses by id (see sca-psd2-compliance.md caveat #5). +func (h *CartHandler) RetryAuth(w http.ResponseWriter, r *http.Request) { + db := database.GetDB().WithContext(r.Context()) + userID := middleware.UserIDFromContext(r.Context()) + + idStr := chi.URLParam(r, "id") + id64, err := strconv.ParseUint(idStr, 10, 32) + if err != nil { + utils.RespondError(w, http.StatusBadRequest, "INVALID_ID", "invalid order id", nil) + return + } + orderID := uint(id64) + + var order models.Order + if err := db.Where("id = ? AND user_id = ?", orderID, userID).First(&order).Error; err != nil { + utils.RespondError(w, http.StatusNotFound, "ORDER_NOT_FOUND", "order not found", nil) + 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 { + utils.RespondError(w, http.StatusConflict, "INVALID_STATE", "order is already finalized", nil) + return + } + if order.StripePaymentID == "" { + utils.RespondError(w, http.StatusConflict, "INVALID_STATE", "order has no payment intent to retry", nil) + return + } + + // Cap is best-effort: two concurrent retries at issuances=cap-1 will both + // pass this check and both Confirm before either writes a new event row. + // Per-user RateLimitByUser(10/min) on the route bounds the worst case. + // SQLite-WAL has no SELECT ... FOR UPDATE, so a stricter guard would need + // a serializing INSERT - not worth the cimplexity at current traffic. + var issuances int64 + if err := db.Model(&models.OrderEvent{}). + Where("order_id = ? AND type = ?", order.ID, audit.EventOrderRequiresAction). + Count(&issuances).Error; err != nil { + utils.RespondError(w, http.StatusInternalServerError, "DATABASE_ERROR", "failed to count auth attempts", nil) + return + } + if issuances >= maxAuthIssuances { + middleware.RecordCartConversion("retry_max_attempts") + utils.RespondError(w, http.StatusTooManyRequests, "RETRY_LIMIT", "too many authentication attempts, please use a different card", nil) + return + } + + // scoped card lookup - match the initial-flow policy in ConvertCart so a + // card the user deleted (or that the storage-TTL reconciler removed) + // cannot be silently re-charged through retry. The ownership clause is + // defensive: order.CardID is enforced at SetCard time today but binding + // the retry path to that invariant explicitly costs nothing. + var card models.Card + if err := db.Where("id = ? AND user_id = ?", order.CardID, userID).First(&card).Error; err != nil { + utils.RespondError(w, http.StatusConflict, "CARD_NO_LONGER_AVAILABLE", "saved card is no longer available, add it again to retry", nil) + return + } + if card.IsStorageExpired(time.Now()) { + if err := expireStoredCard(db, &card, h.stripeKey); err != nil { + utils.RespondError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to expire card", nil) + return + } + respondCardStorageExpired(w) + return + } + + stripe.Key = h.stripeKey + + stripeStart := time.Now() + confirmed, cerr := paymentintent.Confirm(order.StripePaymentID, &stripe.PaymentIntentConfirmParams{ + PaymentMethod: stripe.String(card.StripePaymentID), + OffSession: stripe.Bool(false), + ReturnURL: stripe.String(h.appURL + "/post-3ds"), + }) + stripeDur := time.Since(stripeStart).Seconds() + if cerr != nil { + middleware.ObserveStripeRequest("payment_intent_confirm", "error", stripeDur) + if stripeErr, ok := cerr.(*stripe.Error); ok && stripeErr.Type == stripe.ErrorTypeCard { + db.Model(&order).Update("status", models.OrderStatusFailed) + audit.OrderFailed(userID, order.ID, order.Total, stripeErr.Msg) + middleware.RecordCartConversion("retry_card_declined") + utils.RespondError(w, http.StatusPaymentRequired, mapStripeCardErrCode(stripeErr.Code), stripeErr.Msg, nil) + return + } + if stripeErr, ok := cerr.(*stripe.Error); ok && stripeErr.Code == "payment_intent_unexpected_state" { + actual, gerr := paymentintent.Get(order.StripePaymentID, nil) + if gerr != nil { + middleware.RecordCartConversion("retry_stripe_error") + utils.RespondError(w, http.StatusBadGateway, "PAYMENT_FAILED", "payment failed", nil) + return + } + switch actual.Status { + case stripe.PaymentIntentStatusSucceeded: + middleware.RecordCartConversion("retry_race_already_done") + utils.RespondSuccess(w, http.StatusAccepted, map[string]any{ + "order_id": order.ID, + "status": "succeeded", + }) + return + case stripe.PaymentIntentStatusCanceled: + db.Model(&order).Update("status", models.OrderStatusFailed) + audit.OrderFailed(userID, order.ID, order.Total, "payment intent canceled") + middleware.RecordCartConversion("retry_pi_canceled") + utils.RespondError(w, http.StatusConflict, "INVALID_STATE", "payment intent is canceled, please start a new order", nil) + return + default: + middleware.RecordCartConversion("retry_pi_unexpected_state") + utils.RespondError(w, http.StatusBadGateway, "PAYMENT_FAILED", "payment intent in unexpected state", nil) + return + } + } + + middleware.RecordCartConversion("retry_stripe_error") + utils.RespondError(w, http.StatusBadGateway, "PAYMENT_FAILED", "payment failed", nil) + return + } + middleware.ObserveStripeRequest("payment_intent_confirm", "success", stripeDur) + + // frictionless 3DS on retry, stripe approved without a new challenge. + // the payment_intent.succeeded webhook owns the order.status=paid + // transition (matches the initial-flow contract); tell the TUI to keep + // polling /orders/{id}/status until it lands + if confirmed.Status == stripe.PaymentIntentStatusSucceeded { + middleware.RecordCartConversion("retry_frictionless") + utils.RespondSuccess(w, http.StatusAccepted, map[string]any{ + "order_id": order.ID, + "status": "succeeded", + }) + return + } + + h.respondRequiresAction(w, &order, card.StripePaymentID, confirmed) +} diff --git a/api/handlers/cart_test.go b/api/handlers/cart_test.go index 58020d8..c520c02 100644 --- a/api/handlers/cart_test.go +++ b/api/handlers/cart_test.go @@ -3,6 +3,7 @@ package handlers import ( "bytes" "encoding/json" + "fmt" "net/http" "net/http/httptest" "os" @@ -12,6 +13,7 @@ import ( "github.com/go-chi/chi/v5" "terminalShop/api/middleware" + "terminalShop/pkg/audit" "terminalShop/pkg/database" "terminalShop/pkg/models" ) @@ -867,3 +869,148 @@ func TestConvertCart_ExpiredReturns410(t *testing.T) { t.Fatalf("want CARD_STORAGE_EXPIRED, got %q", resp.Error.Code) } } + +// TestRetryAuth exercises the pre-Stripe validation branches of +// CartHandler.RetryAuth. The happy paths (Confirm → requires_action / +// succeeded) hit the live Stripe API and aren't covered here — see +// sca-psd2-compliance.md "Test plan" for manual coverage with the +// 4000 0027 6000 3184 test card. Caveat #14 in that doc still applies: +// NewCartHandler takes 3 args and the seeded user must have a non-empty +// StripeCustomerID so the Convert/Retry paths stay hermetic. +func TestRetryAuth(t *testing.T) { + cases := []struct { + name string + ownedByUser bool + seedStatus models.OrderStatus + seedPayment string + seedEvents int + wantStatus int + wantErrCode string + }{ + { + name: "wrong user returns not found", + ownedByUser: false, seedStatus: models.OrderStatusFailed, + seedPayment: "pi_test_wrong_user", seedEvents: 1, + wantStatus: http.StatusNotFound, wantErrCode: "ORDER_NOT_FOUND", + }, + { + name: "missing payment intent rejects", + ownedByUser: true, seedStatus: models.OrderStatusPending, + seedPayment: "", seedEvents: 0, + wantStatus: http.StatusConflict, wantErrCode: "INVALID_STATE", + }, + { + name: "paid order is not retryable", + ownedByUser: true, seedStatus: models.OrderStatusPaid, + seedPayment: "pi_test_paid", seedEvents: 1, + wantStatus: http.StatusConflict, wantErrCode: "INVALID_STATE", + }, + { + name: "refunded order is not retryable", + ownedByUser: true, seedStatus: models.OrderStatusRefunded, + seedPayment: "pi_test_refunded", seedEvents: 1, + wantStatus: http.StatusConflict, wantErrCode: "INVALID_STATE", + }, + { + name: "retry cap reached returns 429", + ownedByUser: true, seedStatus: models.OrderStatusFailed, + seedPayment: "pi_test_capped", seedEvents: maxAuthIssuances, + wantStatus: http.StatusTooManyRequests, wantErrCode: "RETRY_LIMIT", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + user.StripeCustomerID = "cus_test_retry" + if err := db.Save(&user).Error; err != nil { + t.Fatalf("save user: %v", err) + } + + ownerID := user.ID + if !tc.ownedByUser { + other := models.User{ + SSHKeyFingerprint: "SHA256:retryother", + SSHPublicKey: "ssh-ed25519 AAAA retryother", + } + if err := db.Create(&other).Error; err != nil { + t.Fatalf("seed other user: %v", err) + } + ownerID = other.ID + } + + card := models.Card{ + UserID: ownerID, StripePaymentID: "pm_test_retry", + 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: ownerID, + CardID: card.ID, + StripePaymentID: tc.seedPayment, + Status: tc.seedStatus, + 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) + } + + for i := 0; i < tc.seedEvents; i++ { + evt := models.OrderEvent{ + OrderID: order.ID, + Type: audit.EventOrderRequiresAction, + Actor: fmt.Sprintf("user:%d", ownerID), + Payload: `{}`, + } + if err := db.Create(&evt).Error; err != nil { + t.Fatalf("seed event: %v", err) + } + } + + handler := NewCartHandler("", "http://localhost", 0) + req := orderRequest("POST", + fmt.Sprintf("/api/v1/orders/%d/retry-auth", order.ID), nil, user.ID, fmt.Sprintf("%d", order.ID)) + w := httptest.NewRecorder() + handler.RetryAuth(w, req) + + if w.Code != tc.wantStatus { + t.Fatalf("status: want %d, got %d body=%s", tc.wantStatus, w.Code, w.Body.String()) + } + + var resp struct { + Error struct { + Code string `json:"code"` + } `json:"error"` + } + _ = json.NewDecoder(w.Body).Decode(&resp) + if resp.Error.Code != tc.wantErrCode { + t.Fatalf("err code: want %q, got %q, body=%s", tc.wantErrCode, resp.Error.Code, w.Body.String()) + } + }) + } +} + +func TestRetryAuth_OrderNotFound(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + handler := NewCartHandler("", "http://localhost", 0) + req := orderRequest("POST", "/api/v1/orders/9999/retry-auth", nil, user.ID, "9999") + w := httptest.NewRecorder() + handler.RetryAuth(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("want 404, got %d body=%s", w.Code, w.Body.String()) + } +} diff --git a/api/routes/routes.go b/api/routes/routes.go index 818b164..9080d60 100644 --- a/api/routes/routes.go +++ b/api/routes/routes.go @@ -132,6 +132,7 @@ func SetupRoutes( // Orders r.Get("/orders", orderHandler.GetOrders) r.Get("/orders/{id}/status", orderHandler.GetOrderStatus) + r.With(middleware.RateLimitByUser(10, time.Minute)).Post("/orders/{id}/retry-auth", cartHandler.RetryAuth) r.Post("/orders/{id}/refund-request", orderHandler.CreateRefundRequest) r.Post("/orders/{id}/refund", orderHandler.RefundOrder) diff --git a/pkg/api/client.go b/pkg/api/client.go index 5009910..7003cce 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -563,6 +563,58 @@ func (c *Client) GetOrderStatus(id uint) (string, error) { return sResp.Data.Status, nil } +// RetryAuthOutcome is the result of POST /orders/{id}/retry-auth. +// status=="requires_action" -> RedirectURL is a fresh QR target; resume polling. +// Status=="succeeded" -> stripe approved frictionless; keep polling for the +// webhook to flip status=paid (RedirectURL will be empty) +type RetryAuthOutcome struct { + OrderID uint + Status string + RedirectURL string +} + +type retryAuthResponse struct { + Success bool `json:"success"` + Data struct { + OrderID uint `json:"order_id"` + Status string `json:"status"` + RedirectURL string `json:"redirect_url,omitempty"` + } `json:"data"` + Error *APIError `json:"error,omitempty"` +} + +// RetryAuth asks the server to re-Confirm the existing PaymentIntent for the +// order, producing a new 3DS challenge URL. Used by the TUI's authFailed +// screen so the customer can recover without re-entering the checkout flow. +func (c *Client) RetryAuth(orderID uint) (*RetryAuthOutcome, error) { + url := fmt.Sprintf("%s/api/v1/orders/%d/retry-auth", c.BaseURL, orderID) + req, err := http.NewRequest("POST", url, nil) + if err != nil { + return nil, fmt.Errorf("failed to create retry request: %w", err) + } + resp, err := c.doRequest(req) + if err != nil { + return nil, fmt.Errorf("failed to send retry request: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + var rResp retryAuthResponse + if err := json.NewDecoder(resp.Body).Decode(&rResp); err != nil { + return nil, fmt.Errorf("failed to decode retry response: %w", err) + } + if !rResp.Success { + if rResp.Error != nil { + return nil, fmt.Errorf("%s: %s", rResp.Error.Code, rResp.Error.Message) + } + return nil, fmt.Errorf("failed to retry authentication") + } + return &RetryAuthOutcome{ + OrderID: rResp.Data.OrderID, + Status: rResp.Data.Status, + RedirectURL: rResp.Data.RedirectURL, + }, nil +} + // GetCards fetches all saved cards for the user. func (c *Client) GetCards() ([]models.Card, error) { url := fmt.Sprintf("%s/api/v1/cards", c.BaseURL) diff --git a/pkg/audit/audit.go b/pkg/audit/audit.go index dbe0f5c..cb60c12 100644 --- a/pkg/audit/audit.go +++ b/pkg/audit/audit.go @@ -13,6 +13,11 @@ import ( "gorm.io/gorm" ) +// EventOrderRequiresAction is the audit event-type string emitted whenever a +// PaymentIntent is pushed into requires_action. explored so cart-flow handler +// can count retry attempts without hardcoding the string literal. +const EventOrderRequiresAction = "order_requires_action" + type eventType string const ( @@ -23,7 +28,7 @@ const ( eventOrderPaid eventType = "order_paid" eventOrderShipped eventType = "order_shipped" eventOrderFailed eventType = "order_failed" - eventOrderRequiresAction eventType = "order_requires_action" + eventOrderRequiresAction eventType = EventOrderRequiresAction eventOrderRefunded eventType = "order_refunded" eventPaymentCritical eventType = "payment_critical" eventLabelPurchased eventType = "label_purchased" @@ -313,7 +318,6 @@ func attrsToMap(a []any) map[string]any { return m } -//nolint:unused // called from ConvertCart in the next update func OrderRequiresAction(userID, orderID uint, paymentIntentID string) { e := event{ Event: eventOrderRequiresAction,