From ecb494de9a3f6aead68901f99a8eb540f26c1401 Mon Sep 17 00:00:00 2001 From: marcusDenslow Date: Sun, 7 Jun 2026 14:57:44 +0200 Subject: [PATCH] feat(sca): sweep abandoned 3ds requires_action orders to failed --- api/handlers/reconcile.go | 84 ++++++++++ api/handlers/reconcile_test.go | 272 +++++++++++++++++++++++++++++++++ api/main.go | 14 ++ pkg/audit/audit.go | 23 ++- pkg/config/config.go | 96 +++++++----- 5 files changed, 450 insertions(+), 39 deletions(-) create mode 100644 api/handlers/reconcile_test.go diff --git a/api/handlers/reconcile.go b/api/handlers/reconcile.go index d905b25..908b44c 100644 --- a/api/handlers/reconcile.go +++ b/api/handlers/reconcile.go @@ -5,6 +5,7 @@ import ( "log/slog" "time" + "terminalShop/pkg/audit" "terminalShop/pkg/database" "terminalShop/pkg/models" "terminalShop/pkg/notify" @@ -14,6 +15,11 @@ import ( "gorm.io/gorm" ) +// paymentIntentGetFn lets tests stub the Stripe Get call without spinning +// up an httptest backend. Production callers go straight through to the +// real paymentintent.Get. +var paymentIntentGetFn = paymentintent.Get + func reconcileLog() *slog.Logger { return slog.With("component", "reconcile") } // ReconcileOrders finds pending orders with no Stripe ID older than 10 minutes @@ -77,6 +83,84 @@ func ReconcileOrders(stripeKey string) { } } +// ReconcileStale3DSOrders flips requires_action orders that have been +// awaiting the customer's 3DS challenge longer than threshold to failed, +// after re-checking the PaymentIntent with Stripe so a late +// payment_intent.succeeded webhook is respected. Deliberately separate +// from ReconcileOrders: the empty-PI predicate there is load-bearing for +// the crash-min-create case (see sca-psd2-compliance.md caveat #20). +func ReconcileStale3DSOrders(stripeKey string, threshold time.Duration) { + stripe.Key = stripeKey + db := database.GetDB() + + var orders []models.Order + cutoff := time.Now().Add(-threshold) + if err := db.Where("status = ? AND updated_at < ?", + models.OrderStatusRequiresAction, cutoff).Find(&orders).Error; err != nil { + reconcileLog().Error("stale 3ds scan failed", "error", err) + return + } + if len(orders) == 0 { + return + } + + reconcileLog().Info("checking stale requires_action orders", "count", len(orders)) + + for i := range orders { + order := &orders[i] + actual, err := paymentIntentGetFn(order.StripePaymentID, nil) + if err != nil { + reconcileLog().Error("stripe get failed", + "order_id", order.ID, "pi", order.StripePaymentID, "error", err) + continue + } + switch actual.Status { + case stripe.PaymentIntentStatusSucceeded: + // Stripe says paid but our row still says requires_action, the + // payment_intent.succeeded webhook has not landed yet (or was + // dropped). Defer to handlePaymentIntentsucceeded; it owns the + // paid transition + cart cleanup atomically. The next sweep + // tick re-checks and surfaces the dropped webhook if it persists + reconcileLog().Warn("stripe reports succeeded for requires_action row; deferring to webhook", + "order_id", order.ID, "pi", order.StripePaymentID) + case stripe.PaymentIntentStatusProcessing, + stripe.PaymentIntentStatusRequiresCapture: + reconcileLog().Info("payment intent still transitioning: skipping", + "order_id", order.ID, "pi", order.StripePaymentID, "stripe_status", string(actual.Status)) + case stripe.PaymentIntentStatusRequiresAction, + stripe.PaymentIntentStatusRequiresPaymentMethod, + stripe.PaymentIntentStatusRequiresConfirmation, + stripe.PaymentIntentStatusCanceled: + flipStale3DSToFailed(db, order, string(actual.Status)) + default: + reconcileLog().Warn("unrecognized stripe status; skipping", + "order_id", order.ID, "pi", order.StripePaymentID, "stripe_status", string(actual.Status)) + } + } +} + +// flipStale3DSToFailed transitions a single stale requires_action row +// to failed. The UPDATE is conditional on status = requires_action so a +// payment_intent.succeeded webhook that landed between the stripe get and +// this write (caveat #20 race) wins and we silently no-op +func flipStale3DSToFailed(db *gorm.DB, order *models.Order, stripeStatus string) { + res := db.Model(&models.Order{}). + Where("id = ? AND status = ?", order.ID, models.OrderStatusRequiresAction). + Updates(map[string]any{"status": models.OrderStatusFailed}) + if res.Error != nil { + reconcileLog().Error("flip stale 3ds failed", "order_id", order.ID, "error", res.Error) + return + } + if res.RowsAffected == 0 { + reconcileLog().Info("stale 3ds row already transitioned; sweep no-op", + "order_id", order.ID) + return + } + audit.Order3DSAbandoned(order.UserID, order.ID, order.StripePaymentID) + reconcileLog().Info("3ds abandoned order flipped to failed", + "order_id", order.ID, "pi", order.StripePaymentID, "stripe_status", stripeStatus) +} + // ReconcileExpiredCards removes saved cards whose retention deadline elapsed. // Read paths filter out expired rows so users never see them; this job drives // the Stripe detach + audit + physical row delete out of band so request diff --git a/api/handlers/reconcile_test.go b/api/handlers/reconcile_test.go new file mode 100644 index 0000000..2f3f446 --- /dev/null +++ b/api/handlers/reconcile_test.go @@ -0,0 +1,272 @@ +package handlers + +import ( + "os" + "testing" + "time" + + "github.com/stripe/stripe-go/v78" + + "terminalShop/pkg/audit" + "terminalShop/pkg/database" + "terminalShop/pkg/models" +) + +// TestReconcileStale3DSOrders_FlipsAbandonedToFailed verifies the predicate +// and Stripe-Get branching: a row past the threshold whose PaymentIntent +// Stripe still reports as requires_payment_method (the customer never +// completed the challenge) transitions to failed and emits the new +// order_3ds_abandoned audit row. +func TestReconcileStale3DSOrders_FlipsAbandonedToFailed(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + audit.SetDB(db) + defer audit.SetDB(nil) + + order := seedRequiresActionOrder(t, user.ID, "pi_stale_abandoned", + time.Now().Add(-1*time.Hour)) + + orig := paymentIntentGetFn + defer func() { paymentIntentGetFn = orig }() + paymentIntentGetFn = func(id string, _ *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { + return &stripe.PaymentIntent{ + ID: id, + Status: stripe.PaymentIntentStatusRequiresPaymentMethod, + }, nil + } + + ReconcileStale3DSOrders("", 30*time.Minute) + + 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) + } + + var events []models.OrderEvent + if err := db.Where("order_id = ? AND type = ?", + order.ID, audit.EventOrder3DSAbandoned).Find(&events).Error; err != nil { + t.Fatalf("list events: %v", err) + } + if len(events) != 1 { + t.Fatalf("want 1 order_3ds_abandoned audit row, got %d", len(events)) + } +} + +// TestReconcileStale3DSOrders_SkipsRecent verifies the threshold predicate +// excludes rows still within the grace window. A fresh requires_action row +// must not even reach the Stripe Get call. +func TestReconcileStale3DSOrders_SkipsRecent(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + + order := seedRequiresActionOrder(t, user.ID, "pi_fresh_3ds", + time.Now().Add(-5*time.Minute)) + + called := false + orig := paymentIntentGetFn + defer func() { paymentIntentGetFn = orig }() + paymentIntentGetFn = func(id string, _ *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { + called = true + return &stripe.PaymentIntent{ + ID: id, + Status: stripe.PaymentIntentStatusRequiresPaymentMethod, + }, nil + } + + ReconcileStale3DSOrders("", 30*time.Minute) + + if called { + t.Fatalf("stripe Get called for in-window row") + } + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload: %v", err) + } + if reloaded.Status != models.OrderStatusRequiresAction { + t.Fatalf("status changed unexpectedly: %q", reloaded.Status) + } +} + +// TestReconcileStale3DSOrders_SkipsWhenStripeSaysSucceeded covers the +// webhook-race guard from caveat #20. If Stripe-side already says succeeded, +// the sweep must defer to handlePaymentIntentSucceeded rather than overwrite +// the authoritative end state. +func TestReconcileStale3DSOrders_SkipsWhenStripeSaysSucceeded(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + + order := seedRequiresActionOrder(t, user.ID, "pi_succeeded_late_webhook", + time.Now().Add(-1*time.Hour)) + + orig := paymentIntentGetFn + defer func() { paymentIntentGetFn = orig }() + paymentIntentGetFn = func(id string, _ *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { + return &stripe.PaymentIntent{ + ID: id, + Status: stripe.PaymentIntentStatusSucceeded, + }, nil + } + + ReconcileStale3DSOrders("", 30*time.Minute) + + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload: %v", err) + } + if reloaded.Status != models.OrderStatusRequiresAction { + t.Fatalf("must defer to webhook on succeeded; got %q", reloaded.Status) + } +} + +// TestReconcileStale3DSOrders_GuardsAgainstWebhookRace covers the conditional +// UPDATE. If the row flipped to paid between the Stripe Get and the flip, +// the WHERE status = requires_action clause prevents clobber. +func TestReconcileStale3DSOrders_GuardsAgainstWebhookRace(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + + order := seedRequiresActionOrder(t, user.ID, "pi_race_paid", + time.Now().Add(-1*time.Hour)) + + orig := paymentIntentGetFn + defer func() { paymentIntentGetFn = orig }() + paymentIntentGetFn = func(id string, _ *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { + db.Model(&models.Order{}).Where("id = ?", order.ID). + Update("status", models.OrderStatusPaid) + return &stripe.PaymentIntent{ + ID: id, + Status: stripe.PaymentIntentStatusRequiresPaymentMethod, + }, nil + } + + ReconcileStale3DSOrders("", 30*time.Minute) + + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload: %v", err) + } + if reloaded.Status != models.OrderStatusPaid { + t.Fatalf("conditional update must preserve paid; got %q", reloaded.Status) + } +} + +// TestReconcileStale3DSOrders_FlipsCanceledPI covers the canceled-PI branch: +// Stripe-terminal-canceled maps to OrderStatusFailed (matches RetryAuth's +// existing precedent — see api/handlers/cart.go RetryAuth canceled branch). +func TestReconcileStale3DSOrders_FlipsCanceledPI(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + audit.SetDB(db) + defer audit.SetDB(nil) + + order := seedRequiresActionOrder(t, user.ID, "pi_canceled_stale", + time.Now().Add(-1*time.Hour)) + + orig := paymentIntentGetFn + defer func() { paymentIntentGetFn = orig }() + paymentIntentGetFn = func(id string, _ *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { + return &stripe.PaymentIntent{ + ID: id, + Status: stripe.PaymentIntentStatusCanceled, + }, nil + } + + ReconcileStale3DSOrders("", 30*time.Minute) + + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload: %v", err) + } + if reloaded.Status != models.OrderStatusFailed { + t.Fatalf("canceled PI must flip to failed; got %q", reloaded.Status) + } +} + +// TestReconcileStale3DSOrders_DoesNotConsumeRetryCap regression-tests the +// caveat #17 decoupling: the new sweep emits EventOrder3DSAbandoned, never +// EventOrderRequiresAction, so a swept-then-stuck order does not silently +// shrink the retry ceiling for any future user-driven retry attempts. +func TestReconcileStale3DSOrders_DoesNotConsumeRetryCap(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + audit.SetDB(db) + defer audit.SetDB(nil) + + order := seedRequiresActionOrder(t, user.ID, "pi_decoupled_cap", + time.Now().Add(-1*time.Hour)) + + orig := paymentIntentGetFn + defer func() { paymentIntentGetFn = orig }() + paymentIntentGetFn = func(id string, _ *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { + return &stripe.PaymentIntent{ + ID: id, + Status: stripe.PaymentIntentStatusRequiresPaymentMethod, + }, nil + } + + ReconcileStale3DSOrders("", 30*time.Minute) + + var retryCount int64 + if err := db.Model(&models.OrderEvent{}). + Where("order_id = ? AND type = ?", order.ID, audit.EventOrderRequiresAction). + Count(&retryCount).Error; err != nil { + t.Fatalf("count retry events: %v", err) + } + if retryCount != 0 { + t.Fatalf("sweep must not emit EventOrderRequiresAction; got %d", retryCount) + } +} + +// seedRequiresActionOrder inserts a card + order pair in the requires_action +// state. updatedAt is forced via UpdateColumn so GORM's auto-stamping of +// UpdatedAt on Create does not bump the row out of the sweep predicate. +func seedRequiresActionOrder(t *testing.T, userID uint, piID string, updatedAt time.Time) models.Order { + t.Helper() + db := database.GetDB() + card := models.Card{ + UserID: userID, + StripePaymentID: "pm_test_" + 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: models.OrderStatusRequiresAction, + 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) + } + if err := db.Model(&order).UpdateColumn("updated_at", updatedAt).Error; err != nil { + t.Fatalf("backdate updated_at: %v", err) + } + return order +} diff --git a/api/main.go b/api/main.go index 46f34b9..a104ad2 100644 --- a/api/main.go +++ b/api/main.go @@ -144,6 +144,20 @@ func main() { } }() + go func() { + ticker := time.NewTicker(15 * time.Minute) + defer ticker.Stop() + threshold := time.Duration(cfg.Abandoned3DSThresholdMinutes) * time.Minute + for { + select { + case <-ticker.C: + handlers.ReconcileStale3DSOrders(cfg.StripeSecretKey, threshold) + case <-ctx.Done(): + return + } + } + }() + log.Println("API server started. Press Ctrl+C to shutdown.") // Block until signal received diff --git a/pkg/audit/audit.go b/pkg/audit/audit.go index cb60c12..08ee91f 100644 --- a/pkg/audit/audit.go +++ b/pkg/audit/audit.go @@ -14,10 +14,16 @@ import ( ) // EventOrderRequiresAction is the audit event-type string emitted whenever a -// PaymentIntent is pushed into requires_action. explored so cart-flow handler +// PaymentIntent is pushed into requires_action. Exposed so cart-flow handler // can count retry attempts without hardcoding the string literal. const EventOrderRequiresAction = "order_requires_action" +// EventOrder3DSAbandoned is the audit event-type string emitted by the +// abandoned-3DS sweep when it flips a stale requires_action row to failed. +// Distinct from EventOrderRequiresAction (caveat #17) so the retry-cap count +// stays decoupled from sweep activity +const EventOrder3DSAbandoned = "order_3ds_abandoned" + type eventType string const ( @@ -29,6 +35,7 @@ const ( eventOrderShipped eventType = "order_shipped" eventOrderFailed eventType = "order_failed" eventOrderRequiresAction eventType = EventOrderRequiresAction + eventOrder3DSAbandoned eventType = EventOrder3DSAbandoned eventOrderRefunded eventType = "order_refunded" eventPaymentCritical eventType = "payment_critical" eventLabelPurchased eventType = "label_purchased" @@ -328,3 +335,17 @@ func OrderRequiresAction(userID, orderID uint, paymentIntentID string) { slog.Info("audit", e.attrs()...) persist(orderID, eventOrderRequiresAction, e) } + +// Order3DSAbandoned records the abandoned-3DS sweep flipping a stale +// requires_action row to failed because Stripe still reports the underlying +// PaymentIntent as outstanding past the sweep threshold. +func Order3DSAbandoned(userID, orderID uint, paymentIntentID string) { + e := event{ + Event: eventOrder3DSAbandoned, + UserID: userID, + OrderID: orderID, + StripeID: paymentIntentID, + } + slog.Info("audit", e.attrs()...) + persist(orderID, eventOrder3DSAbandoned, e) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 8a84fe7..c4eeca2 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -13,25 +13,26 @@ import ( // Config holds all application configuration type Config struct { - APIPort string - APIURL string - AppURL string - SSHPort string - Environment string - DatabaseURL string - JWTSecret string - StripeSecretKey string - StripePublicKey string - StripeWebhookSecret string - ShippoAPIKey string - AuthFingerprintKey string - BringAPIUID string - BringAPIKey string - BringCustomerNumber string - ShippoWebhookSecret string - SlackSigningSecret string - AdminAPIKey string - MaxOrderCents int + APIPort string + APIURL string + AppURL string + SSHPort string + Environment string + DatabaseURL string + JWTSecret string + StripeSecretKey string + StripePublicKey string + StripeWebhookSecret string + ShippoAPIKey string + AuthFingerprintKey string + BringAPIUID string + BringAPIKey string + BringCustomerNumber string + ShippoWebhookSecret string + SlackSigningSecret string + AdminAPIKey string + MaxOrderCents int + Abandoned3DSThresholdMinutes int } // Load reads configuration from environment variables and validates required secrets. @@ -50,25 +51,26 @@ func Load() (*Config, error) { apiPort := getEnvOrDefault("API_PORT", "8080") apiURL := getEnvOrDefault("API_URL", fmt.Sprintf("http://localhost:%s", apiPort)) cfg := &Config{ - APIPort: apiPort, - APIURL: apiURL, - AppURL: getEnvOrDefault("APP_URL", apiURL), - SSHPort: getEnvOrDefault("SSH_PORT", "23457"), - Environment: getEnvOrDefault("ENVIRONMENT", "development"), - DatabaseURL: getEnvOrDefault("DATABASE_URL", "terminalshop.db"), - JWTSecret: os.Getenv("JWT_SECRET"), - StripeSecretKey: os.Getenv("STRIPE_SECRET_KEY"), - StripePublicKey: os.Getenv("STRIPE_PUBLIC_KEY"), - StripeWebhookSecret: os.Getenv("STRIPE_WEBHOOK_SECRET"), - ShippoAPIKey: os.Getenv("SHIPPO_API_KEY"), - AuthFingerprintKey: os.Getenv("AUTH_FINGERPRINT_KEY"), - BringAPIUID: os.Getenv("BRING_API_UID"), - BringAPIKey: os.Getenv("BRING_API_KEY"), - BringCustomerNumber: os.Getenv("BRING_CUSTOMER_NUMBER"), - ShippoWebhookSecret: os.Getenv("SHIPPO_WEBHOOK_SECRET"), - SlackSigningSecret: os.Getenv("SLACK_SIGNING_SECRET"), - AdminAPIKey: os.Getenv("ADMIN_API_KEY"), - MaxOrderCents: loadMaxOrderCents(), + APIPort: apiPort, + APIURL: apiURL, + AppURL: getEnvOrDefault("APP_URL", apiURL), + SSHPort: getEnvOrDefault("SSH_PORT", "23457"), + Environment: getEnvOrDefault("ENVIRONMENT", "development"), + DatabaseURL: getEnvOrDefault("DATABASE_URL", "terminalshop.db"), + JWTSecret: os.Getenv("JWT_SECRET"), + StripeSecretKey: os.Getenv("STRIPE_SECRET_KEY"), + StripePublicKey: os.Getenv("STRIPE_PUBLIC_KEY"), + StripeWebhookSecret: os.Getenv("STRIPE_WEBHOOK_SECRET"), + ShippoAPIKey: os.Getenv("SHIPPO_API_KEY"), + AuthFingerprintKey: os.Getenv("AUTH_FINGERPRINT_KEY"), + BringAPIUID: os.Getenv("BRING_API_UID"), + BringAPIKey: os.Getenv("BRING_API_KEY"), + BringCustomerNumber: os.Getenv("BRING_CUSTOMER_NUMBER"), + ShippoWebhookSecret: os.Getenv("SHIPPO_WEBHOOK_SECRET"), + SlackSigningSecret: os.Getenv("SLACK_SIGNING_SECRET"), + AdminAPIKey: os.Getenv("ADMIN_API_KEY"), + MaxOrderCents: loadMaxOrderCents(), + Abandoned3DSThresholdMinutes: loadAbandoned3DSThresholdMinutes(), } if err := cfg.validate(); err != nil { @@ -150,3 +152,21 @@ func loadMaxOrderCents() int { } return n } + +// loadAbandoned3DSThresholdMinutes read ABANDONED_3DS_THRESHOLD_MINUTES. +// The abandoned-3DS sweep only flips requires_action rows whose updated_at iconfigs +// older than this many minutes. Default 30 min. Values below 1 fall back to +// the default so a typo cannot collapse the grace window the TUI polls inside. +func loadAbandoned3DSThresholdMinutes() int { + const defaultMinutes = 30 + raw := os.Getenv("ABANDONED_3DS_THRESHOLD_MINUTES") + if raw == "" { + return defaultMinutes + } + n, err := strconv.Atoi(raw) + if err != nil || n < 1 { + fmt.Printf("WARNING: ABANDONED_3DS_THRESHOLD_MINUTES=%q invalid, falling back to default %d\n", raw, defaultMinutes) + return defaultMinutes + } + return n +}