diff --git a/api/handlers/card_test.go b/api/handlers/card_test.go index 66906f7..d88d245 100644 --- a/api/handlers/card_test.go +++ b/api/handlers/card_test.go @@ -46,7 +46,7 @@ func TestGetCards_FiltersExpiredRows(t *testing.T) { } } - handler := NewCardHandler("", "") + handler := NewCardHandler("") req := authRequest("GET", "/api/v1/cards", nil, user.ID) w := httptest.NewRecorder() handler.GetCards(w, req) @@ -91,7 +91,7 @@ func TestSetDefaultCard_ExpiredReturns410(t *testing.T) { t.Fatalf("seed card: %v", err) } - handler := NewCardHandler("", "") + handler := NewCardHandler("") idStr := strconv.FormatUint(uint64(card.ID), 10) req := authRequest("PUT", "/api/v1/cards/"+idStr+"/default", nil, user.ID) routeCtx := chi.NewRouteContext() diff --git a/api/handlers/cards.go b/api/handlers/cards.go index c73c3db..359f37e 100644 --- a/api/handlers/cards.go +++ b/api/handlers/cards.go @@ -67,13 +67,13 @@ func PayRedirect(w http.ResponseWriter, r *http.Request) { // CardHandler handles saved payment method CRUD. type CardHandler struct { - stripeKey string - appURL string + appURL string } -// NewCardHandler creates a new card handler with the Stripe secret key. -func NewCardHandler(stripeSecretKey string, appURL string) *CardHandler { - return &CardHandler{stripeKey: stripeSecretKey, appURL: appURL} +// NewCardHandler creates a new card handler. Stripe credentials are wired +// once at startup via stripe.Key in api/main.go. +func NewCardHandler(appURL string) *CardHandler { + return &CardHandler{appURL: appURL} } // GetCards returns all saved cards for the authenticated user. @@ -137,7 +137,7 @@ func (h *CardHandler) SetDefaultCard(w http.ResponseWriter, r *http.Request) { } if card.IsStorageExpired(time.Now()) { - if err := expireStoredCard(db, &card, h.stripeKey); err != nil { + if err := expireStoredCard(db, &card); err != nil { utils.RespondError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to expire card", nil) return } @@ -171,8 +171,6 @@ func (h *CardHandler) CollectCard(w http.ResponseWriter, r *http.Request) { return } - stripe.Key = h.stripeKey - if err := ensureStripeCustomer(db, &user); err != nil { utils.RespondError(w, http.StatusInternalServerError, "STRIPE_ERROR", "failed to create stripe customer", nil) return @@ -234,8 +232,6 @@ func (h *CardHandler) SaveCard(w http.ResponseWriter, r *http.Request) { return } - stripe.Key = h.stripeKey - // Ensure the user has a Stripe customer, creating one if needed. if err := ensureStripeCustomer(db, &user); err != nil { utils.RespondError(w, http.StatusInternalServerError, "STRIPE_ERROR", "failed to create stripe customer", nil) @@ -335,7 +331,6 @@ func (h *CardHandler) DeleteCard(w http.ResponseWriter, r *http.Request) { // Detach from Stripe (best-effort; don't block deletion on Stripe errors). if len(card.StripePaymentID) >= 3 && card.StripePaymentID[:3] == "pm_" { - stripe.Key = h.stripeKey _, _ = paymentmethod.Detach(card.StripePaymentID, &stripe.PaymentMethodDetachParams{}) } @@ -355,8 +350,10 @@ func (h *CardHandler) DeleteCard(w http.ResponseWriter, r *http.Request) { // expireStoredCard removes a card whose retention deadline elapsed. // Local state (cart references + card row) is committed atomically; // Stripe detach and audit run only after the local commit so retries -// cannot double-fire the audit event. -func expireStoredCard(db *gorm.DB, card *models.Card, stripeKey string) error { +// cannot double-fire the audit event. Stripe detach is gated on +// stripe.Key being non-empty so tests (which never init the SDK key) +// skip the real network call without an explicit flag. +func expireStoredCard(db *gorm.DB, card *models.Card) error { if err := db.Transaction(func(tx *gorm.DB) error { if err := tx.Model(&models.Cart{}).Where("card_id = ? AND user_id = ?", card.ID, card.UserID).Update("card_id", nil).Error; err != nil { return fmt.Errorf("failed to clear expired card from cart: %w", err) @@ -369,8 +366,7 @@ func expireStoredCard(db *gorm.DB, card *models.Card, stripeKey string) error { return err } - if stripeKey != "" && strings.HasPrefix(card.StripePaymentID, "pm_") { - stripe.Key = stripeKey + if stripe.Key != "" && strings.HasPrefix(card.StripePaymentID, "pm_") { if _, err := paymentmethod.Detach(card.StripePaymentID, &stripe.PaymentMethodDetachParams{}); err != nil { slog.Warn("failed to detach expired card", "card_id", card.ID, "error", err) } diff --git a/api/handlers/cards_expire_test.go b/api/handlers/cards_expire_test.go index 15b66a2..cc15608 100644 --- a/api/handlers/cards_expire_test.go +++ b/api/handlers/cards_expire_test.go @@ -58,7 +58,7 @@ func TestReconcileExpiredCards_RemovesPastDeadlineOnly(t *testing.T) { t.Fatalf("seed cart: %v", err) } - ReconcileExpiredCards("") + ReconcileExpiredCards() var remaining []models.Card if err := db.Where("user_id = ?", user.ID).Find(&remaining).Error; err != nil { @@ -117,7 +117,7 @@ func TestReconcileExpiredCards_SweepsAcrossUsers(t *testing.T) { } } - ReconcileExpiredCards("") + ReconcileExpiredCards() var count int64 if err := db.Model(&models.Card{}).Count(&count).Error; err != nil { diff --git a/api/handlers/cart.go b/api/handlers/cart.go index c9065ca..f87b70d 100644 --- a/api/handlers/cart.go +++ b/api/handlers/cart.go @@ -25,7 +25,6 @@ import ( ) type CartHandler struct { - stripeKey string appURL string maxOrderCents int } @@ -36,9 +35,10 @@ type CartHandler struct { // 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} +// NewCartHandler creates a new cart handler. Stripe credentials are wired +// once at startup via stripe.Key in api/main.go. +func NewCartHandler(appURL string, maxOrderCents int) *CartHandler { + return &CartHandler{appURL: appURL, maxOrderCents: maxOrderCents} } // getOrCreateCart finds the users cart or creates one. @@ -219,7 +219,7 @@ func (h *CartHandler) SetCard(w http.ResponseWriter, r *http.Request) { } if card.IsStorageExpired(time.Now()) { - if err := expireStoredCard(db, &card, h.stripeKey); err != nil { + if err := expireStoredCard(db, &card); err != nil { utils.RespondError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to expire card", nil) return } @@ -315,7 +315,7 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) { if card.IsStorageExpired(time.Now()) { middleware.RecordCartConversion("validation_card_expired") - if err := expireStoredCard(db, &card, h.stripeKey); err != nil { + if err := expireStoredCard(db, &card); err != nil { utils.RespondError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to expire card", nil) return } @@ -330,8 +330,6 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) { return } - stripe.Key = h.stripeKey - // Ensure the user has a Stripe customer (shared helper with cards handler). if err := ensureStripeCustomer(db, &user); err != nil { middleware.RecordCartConversion("stripe_customer_failed") @@ -706,7 +704,7 @@ func (h *CartHandler) RetryAuth(w http.ResponseWriter, r *http.Request) { return } if card.IsStorageExpired(time.Now()) { - if err := expireStoredCard(db, &card, h.stripeKey); err != nil { + if err := expireStoredCard(db, &card); err != nil { utils.RespondError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to expire card", nil) return } @@ -714,8 +712,6 @@ func (h *CartHandler) RetryAuth(w http.ResponseWriter, r *http.Request) { return } - stripe.Key = h.stripeKey - stripeStart := time.Now() confirmed, cerr := paymentintent.Confirm(order.StripePaymentID, &stripe.PaymentIntentConfirmParams{ PaymentMethod: stripe.String(card.StripePaymentID), diff --git a/api/handlers/cart_test.go b/api/handlers/cart_test.go index 52d5795..e87fa12 100644 --- a/api/handlers/cart_test.go +++ b/api/handlers/cart_test.go @@ -69,7 +69,7 @@ func TestGetCartEmpty(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) req := authRequest("GET", "/api/v1/cart", nil, user.ID) w := httptest.NewRecorder() @@ -108,7 +108,7 @@ func TestSetItemAndGetCart(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) // Add item (CoffeeID=1, Quantity=2) body, _ := json.Marshal(map[string]any{ @@ -164,7 +164,7 @@ func TestSetItemUpdateQuantity(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) // Add item body, _ := json.Marshal(map[string]any{"coffee_id": 1, "quantity": 2}) @@ -209,7 +209,7 @@ func TestSetItemRemove(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) // Add item body, _ := json.Marshal(map[string]any{"coffee_id": 1, "quantity": 3}) @@ -248,7 +248,7 @@ func TestSetItemInvalidProduct(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) body, _ := json.Marshal(map[string]any{"coffee_id": 999, "quantity": 1}) req := authRequest("PUT", "/api/v1/cart/item", body, user.ID) @@ -265,7 +265,7 @@ func TestSetItemMissingCoffeeID(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) body, _ := json.Marshal(map[string]any{"quantity": 1}) req := authRequest("PUT", "/api/v1/cart/item", body, user.ID) @@ -282,7 +282,7 @@ func TestClearCart(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) // Add two items for _, coffeeID := range []uint{1, 2} { @@ -338,7 +338,7 @@ func TestSetAddress(t *testing.T) { } db.Create(&addr) - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) body, _ := json.Marshal(map[string]any{"address_id": addr.ID}) req := authRequest("PUT", "/api/v1/cart/address", body, user.ID) @@ -392,7 +392,7 @@ func TestSetAddressWrongUser(t *testing.T) { } db.Create(&addr) - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) body, _ := json.Marshal(map[string]any{"address_id": addr.ID}) req := authRequest("PUT", "/api/v1/cart/address", body, user.ID) @@ -420,7 +420,7 @@ func TestSetCard(t *testing.T) { } db.Create(&card) - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) body, _ := json.Marshal(map[string]any{"card_id": card.ID}) req := authRequest("PUT", "/api/v1/cart/card", body, user.ID) @@ -472,7 +472,7 @@ func TestSetCardWrongUser(t *testing.T) { } db.Create(&card) - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) body, _ := json.Marshal(map[string]any{"card_id": card.ID}) req := authRequest("PUT", "/api/v1/cart/card", body, user.ID) @@ -489,7 +489,7 @@ func TestConvertCartMissingAddress(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) // Add item but no address or card body, _ := json.Marshal(map[string]any{"coffee_id": 1, "quantity": 1}) @@ -512,7 +512,7 @@ func TestConvertCartMissingCard(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) db := database.GetDB() // Add item and address @@ -543,7 +543,7 @@ func TestConvertCartEmpty(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) // Empty cart convert req := authRequest("POST", "/api/v1/cart/convert", nil, user.ID) @@ -560,7 +560,7 @@ func TestMultipleItems(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) // Add three different items for i := uint(1); i <= 3; i++ { @@ -609,7 +609,7 @@ func TestCartIsolation(t *testing.T) { } db.Create(&user2) - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) r := chi.NewRouter() r.Put("/cart/item", handler.SetItem) r.Get("/cart", handler.GetCart) @@ -718,7 +718,7 @@ func TestConvertCartLimit(t *testing.T) { t.Fatalf("pin coffee price: %v", err) } - handler := NewCartHandler("", "", tc.capCents) + handler := NewCartHandler("", tc.capCents) body, _ := json.Marshal(map[string]any{"coffee_id": 4, "quantity": tc.quantity}) req := authRequest("PUT", "/api/v1/cart/item", body, user.ID) @@ -795,7 +795,7 @@ func TestSetCard_ExpiredReturns410(t *testing.T) { t.Fatalf("seed card: %v", err) } - handler := NewCartHandler("", "", 0) + handler := NewCartHandler("", 0) body, _ := json.Marshal(map[string]any{"card_id": card.ID}) req := authRequest("PUT", "/api/v1/cart/card", body, user.ID) w := httptest.NewRecorder() @@ -836,7 +836,7 @@ func TestConvertCart_ExpiredReturns410(t *testing.T) { body, _ := json.Marshal(map[string]any{"coffee_id": 1, "quantity": 1}) req := authRequest("PUT", "/api/v1/vart/item", body, user.ID) w := httptest.NewRecorder() - NewCartHandler("", "", 0).SetItem(w, req) + NewCartHandler("", 0).SetItem(w, req) addr := models.Address{ UserID: user.ID, Name: "Test", Street: "123 St", @@ -846,7 +846,7 @@ func TestConvertCart_ExpiredReturns410(t *testing.T) { body, _ = json.Marshal(map[string]any{"address_id": addr.ID}) req = authRequest("PUT", "/api/v1/cart/address", body, user.ID) w = httptest.NewRecorder() - NewCartHandler("", "", 0).SetAddress(w, req) + NewCartHandler("", 0).SetAddress(w, req) past := time.Now().Add(-1 * time.Hour) card := models.Card{ @@ -862,7 +862,7 @@ func TestConvertCart_ExpiredReturns410(t *testing.T) { req = authRequest("POST", "/api/v1/cart/convert", nil, user.ID) w = httptest.NewRecorder() - NewCartHandler("", "", 0).ConvertCart(w, req) + NewCartHandler("", 0).ConvertCart(w, req) if w.Code != http.StatusGone { t.Fatalf("want 410, got %d, body: %s", w.Code, w.Body.String()) @@ -884,8 +884,9 @@ func TestConvertCart_ExpiredReturns410(t *testing.T) { // 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. +// NewCartHandler takes 2 args (appURL, maxOrderCents) 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 @@ -992,7 +993,7 @@ func TestRetryAuth(t *testing.T) { } } - handler := NewCartHandler("", "http://localhost", 0) + 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() @@ -1020,7 +1021,7 @@ func TestRetryAuth_OrderNotFound(t *testing.T) { defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() - handler := NewCartHandler("", "http://localhost", 0) + 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) @@ -1065,7 +1066,7 @@ func TestRespondRequiresAction_SetStatusRequiresAction(t *testing.T) { }, } - handler := NewCartHandler("", "http://localhost", 0) + handler := NewCartHandler("http://localhost", 0) w := httptest.NewRecorder() handler.respondRequiresAction(w, &order, card.StripePaymentID, pi) @@ -1147,7 +1148,7 @@ func TestRespondRequiresAction_OverridesFailedFromWebhookRace(t *testing.T) { }, } - handler := NewCartHandler("", "http://localhost", 0) + handler := NewCartHandler("http://localhost", 0) w := httptest.NewRecorder() handler.respondRequiresAction(w, &order, card.StripePaymentID, pi) diff --git a/api/handlers/orders.go b/api/handlers/orders.go index 1a3027b..bafdb78 100644 --- a/api/handlers/orders.go +++ b/api/handlers/orders.go @@ -28,7 +28,6 @@ import ( ) type OrderHandler struct { - stripeKey string bringAPIUID string bringAPIKey string bringCustomerNumber string @@ -44,9 +43,10 @@ type refundRequestPayload struct { // repeated submissions from spamming the operator's Slack channel. const refundRequestCooldown = 15 * time.Minute -func NewOrderHandler(stripeSecretKey, bringAPIUID, bringAPIKey, bringCustomerNumber string) *OrderHandler { +// NewOrderHandler creates an order handler. Stripe credentials are wired +// once at startup via stripe.Key in api/main.go. +func NewOrderHandler(bringAPIUID, bringAPIKey, bringCustomerNumber string) *OrderHandler { return &OrderHandler{ - stripeKey: stripeSecretKey, bringAPIUID: bringAPIUID, bringAPIKey: bringAPIKey, bringCustomerNumber: bringCustomerNumber, @@ -200,8 +200,6 @@ func (h *OrderHandler) RefundOrder(w http.ResponseWriter, r *http.Request) { return } - stripe.Key = h.stripeKey - rf, err := refund.New(&stripe.RefundParams{ PaymentIntent: stripe.String(order.StripePaymentID), }) diff --git a/api/handlers/orders_test.go b/api/handlers/orders_test.go index d4aafef..37014b5 100644 --- a/api/handlers/orders_test.go +++ b/api/handlers/orders_test.go @@ -60,7 +60,7 @@ func contextWithRoute(ctx context.Context, routeCtx *chi.Context) context.Contex func TestCreateRefundRequestRequiresMessageForOther(t *testing.T) { user := setupOrderTestDB(t) - handler := NewOrderHandler("", "", "", "") + handler := NewOrderHandler("", "", "") body := []byte(`{"reason":"Other","message":" "}`) req := orderRequest("POST", "/api/v1/orders/1/refund-request", body, user.ID, "1") w := httptest.NewRecorder() @@ -100,7 +100,7 @@ func TestCreateRefundRequestRejectsInvalidOrderState(t *testing.T) { t.Fatalf("failed to create order: %v", err) } - handler := NewOrderHandler("", "", "", "") + handler := NewOrderHandler("", "", "") body := []byte(`{"reason":"Quality issue","message":""}`) req := orderRequest("POST", "/api/v1/orders/1/refund-request", body, user.ID, fmt.Sprintf("%d", order.ID)) w := httptest.NewRecorder() @@ -142,7 +142,7 @@ func TestCreateRefundRequestEnforcesCooldown(t *testing.T) { t.Fatalf("failed to create order: %v", err) } - handler := NewOrderHandler("", "", "", "") + handler := NewOrderHandler("", "", "") body := []byte(`{"reason":"Quality issue","message":""}`) req := orderRequest("POST", "/api/v1/orders/1/refund-request", body, user.ID, fmt.Sprintf("%d", order.ID)) w := httptest.NewRecorder() diff --git a/api/handlers/reconcile.go b/api/handlers/reconcile.go index d905b25..94a3cad 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,14 +15,18 @@ 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 // and checks whether Stripe has a succeeded PaymentIntent for them. This fixes // the case where the server crashed after charging but before the DB transaction // completed. -func ReconcileOrders(stripeKey string) { - stripe.Key = stripeKey +func ReconcileOrders() { db := database.GetDB() var orders []models.Order @@ -77,11 +82,103 @@ 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-mid-create case (see sca-psd2-compliance.md caveat #20). +// stripe.Key is wired once at startup in api/main.go. +func ReconcileStale3DSOrders(threshold time.Duration) { + db := database.GetDB() + + var orders []models.Order + // Predicate keys off updated_at, not created_at: respondRequiresAction + // and RetryAuth both bump updated_at via GORM auto-stamping, so every + // customer-driven retry extends the grace window. Intentional - sweeping + // a row the customer just re-armed would race the new challenge. + 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] + if order.StripePaymentID == "" { + // respondRequiresAction writes stripe_payment_id and status in + // one UPDATE so this should be unreachable; guard anyway so a + // future regression noisily logs instead of 4xx'ing Stripe. + reconcileLog().Warn("requires_action row missing PI id; skipping", "order_id", order.ID) + continue + } + 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) { + // Map form so the single-field UPDATE is explicit at the call site. + // GORM still auto-stamps updated_at; the row exits the sweep predicate + // naturally on the next tick because status no longer matches anyway. + 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 // handlers never block on Stripe API calls during a card sweep. -func ReconcileExpiredCards(stripeKey string) { +func ReconcileExpiredCards() { db := database.GetDB() var cards []models.Card @@ -98,7 +195,7 @@ func ReconcileExpiredCards(stripeKey string) { reconcileLog().Info("expiring inactive saved cards", "count", len(cards)) for i := range cards { - if err := expireStoredCard(db, &cards[i], stripeKey); err != nil { + if err := expireStoredCard(db, &cards[i]); err != nil { reconcileLog().Error("expire card failed", "card_id", cards[i].ID, "error", err) } } diff --git a/api/handlers/reconcile_test.go b/api/handlers/reconcile_test.go new file mode 100644 index 0000000..7c62b61 --- /dev/null +++ b/api/handlers/reconcile_test.go @@ -0,0 +1,429 @@ +package handlers + +import ( + "fmt" + "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() + audit.SetDB(db) + defer audit.SetDB(nil) + + 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() + audit.SetDB(db) + defer audit.SetDB(nil) + + 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() + audit.SetDB(db) + defer audit.SetDB(nil) + + 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) + } +} + +// TestReconcileStale3DSOrders_SkipsTransitioningStatuses covers the +// processing and requires_capture branches: Stripe still owns the PI's +// next move, so the sweep must not flip even past the threshold. +func TestReconcileStale3DSOrders_SkipsTransitioningStatuses(t *testing.T) { + cases := []struct { + name string + status stripe.PaymentIntentStatus + }{ + {"processing", stripe.PaymentIntentStatusProcessing}, + {"requires_capture", stripe.PaymentIntentStatusRequiresCapture}, + } + 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() + audit.SetDB(db) + defer audit.SetDB(nil) + + order := seedRequiresActionOrder(t, user.ID, + "pi_transitioning_"+tc.name, 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: tc.status}, 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("%s must stay requires_action; got %q", tc.name, reloaded.Status) + } + }) + } +} + +// TestReconcileStale3DSOrders_SkipsUnknownStatus covers the switch's default +// arm: an unrecognized Stripe status logs and skips rather than guessing a +// transition. Defends against new PI statuses appearing in future SDKs. +func TestReconcileStale3DSOrders_SkipsUnknownStatus(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_unknown_status", + 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.PaymentIntentStatus("some_new_future_status"), + }, 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("unknown status must not flip; got %q", reloaded.Status) + } +} + +// TestReconcileStale3DSOrders_SkipsOnStripeError verifies a Stripe Get +// failure logs and leaves the row alone. Transient network errors must not +// look like an abandoned 3DS. +func TestReconcileStale3DSOrders_SkipsOnStripeError(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_stripe_error", + time.Now().Add(-1*time.Hour)) + + orig := paymentIntentGetFn + defer func() { paymentIntentGetFn = orig }() + paymentIntentGetFn = func(_ string, _ *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { + return nil, fmt.Errorf("simulated stripe outage") + } + + 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("stripe error must not flip; got %q", reloaded.Status) + } +} + +// TestReconcileStale3DSOrders_SkipsEmptyPaymentIntentID guards the +// defensive empty-PI short-circuit: even past the threshold, a row missing +// stripe_payment_id must not reach paymentintent.Get (which would 4xx). +func TestReconcileStale3DSOrders_SkipsEmptyPaymentIntentID(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_will_be_blanked", + time.Now().Add(-1*time.Hour)) + if err := db.Model(&order).UpdateColumn("stripe_payment_id", "").Error; err != nil { + t.Fatalf("blank PI id: %v", err) + } + + called := false + orig := paymentIntentGetFn + defer func() { paymentIntentGetFn = orig }() + paymentIntentGetFn = func(_ string, _ *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { + called = true + return &stripe.PaymentIntent{Status: stripe.PaymentIntentStatusRequiresPaymentMethod}, nil + } + + ReconcileStale3DSOrders(30 * time.Minute) + + if called { + t.Fatalf("stripe Get called for empty-PI 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("empty-PI guard must leave status alone; got %q", reloaded.Status) + } +} + +// 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/handlers/webhook.go b/api/handlers/webhook.go index fdcd569..88cc85e 100644 --- a/api/handlers/webhook.go +++ b/api/handlers/webhook.go @@ -28,13 +28,13 @@ func webhookLog() *slog.Logger { return slog.With("component", "webhook") } // WebhookHandler handles inbound Stripe webhook events. type WebhookHandler struct { webhookSecret string - stripeKey string shippoSecret string } -// NewWebhookHandler creates a new webhook handler. -func NewWebhookHandler(webhookSecret, stripeKey, shippoSecret string) *WebhookHandler { - return &WebhookHandler{webhookSecret: webhookSecret, stripeKey: stripeKey, shippoSecret: shippoSecret} +// NewWebhookHandler creates a new webhook handler. Stripe credentials are +// wired once at startup via stripe.Key in api/main.go. +func NewWebhookHandler(webhookSecret, shippoSecret string) *WebhookHandler { + return &WebhookHandler{webhookSecret: webhookSecret, shippoSecret: shippoSecret} } // HandleStripe validates the Stripe signature and routes the event. @@ -212,8 +212,6 @@ func (h *WebhookHandler) handleCheckoutSessionCompleted(ctx context.Context, eve return } - stripe.Key = h.stripeKey - si, err := setupintent.Get(sess.SetupIntent.ID, nil) if err != nil { webhookLog().Error("failed to get setup intent", "setup_intent", sess.SetupIntent.ID, "error", err) diff --git a/api/handlers/webhook_test.go b/api/handlers/webhook_test.go index 988202a..0d7de59 100644 --- a/api/handlers/webhook_test.go +++ b/api/handlers/webhook_test.go @@ -74,7 +74,7 @@ func TestHandlePaymentIntentFailed_RequiresActionFlipsToFailed(t *testing.T) { order := seedOrderForWebhook(t, user.ID, models.OrderStatusRequiresAction, "pi_test_req_action_failed") - h := NewWebhookHandler("", "", "") + h := NewWebhookHandler("", "") evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined") h.handlePaymentIntentFailed(context.Background(), evt) @@ -100,7 +100,7 @@ func TestHandlePaymentIntentFailed_RequiresActionAuthRequiredShortCircuits(t *te order := seedOrderForWebhook(t, user.ID, models.OrderStatusRequiresAction, "pi_test_req_action_authreq") - h := NewWebhookHandler("", "", "") + h := NewWebhookHandler("", "") evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "authentication_required") h.handlePaymentIntentFailed(context.Background(), evt) @@ -124,7 +124,7 @@ func TestHandlePaymentIntentFailed_PendingStillFlips(t *testing.T) { order := seedOrderForWebhook(t, user.ID, models.OrderStatusPending, "pi_test_pending_failed") - h := NewWebhookHandler("", "", "") + h := NewWebhookHandler("", "") evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined") h.handlePaymentIntentFailed(context.Background(), evt) @@ -150,7 +150,7 @@ func TestHandlePaymentIntentFailed_PaidNotTouched(t *testing.T) { order := seedOrderForWebhook(t, user.ID, models.OrderStatusPaid, "pi_test_paid_untouched") - h := NewWebhookHandler("", "", "") + h := NewWebhookHandler("", "") evt := buildPaymentFailedEvent(t, order.StripePaymentID, order.ID, "card_declined") h.handlePaymentIntentFailed(context.Background(), evt) @@ -182,7 +182,7 @@ func TestHandlePaymentIntentFailed_MissingMetadataNoOp(t *testing.T) { Data: &stripe.EventData{Raw: raw}, } - h := NewWebhookHandler("", "", "") + h := NewWebhookHandler("", "") h.handlePaymentIntentFailed(context.Background(), evt) // must not panic } @@ -207,7 +207,7 @@ func TestHandlePaymentIntentSucceeded_RequiresActionFlipsToPaid(t *testing.T) { Data: &stripe.EventData{Raw: raw}, } - h := NewWebhookHandler("", "", "") + h := NewWebhookHandler("", "") h.handlePaymentIntentSucceeded(context.Background(), evt) db := database.GetDB() diff --git a/api/main.go b/api/main.go index 46f34b9..0140d4d 100644 --- a/api/main.go +++ b/api/main.go @@ -19,6 +19,8 @@ import ( "terminalShop/pkg/database" "terminalShop/pkg/observability" "terminalShop/pkg/stripeclient" + + "github.com/stripe/stripe-go/v78" ) const version = "v0.1.0" @@ -49,6 +51,12 @@ func main() { } audit.SetDB(db) + // wire the stripe SDK's package-level key once at startup so concurrent + // reconcilers and the request path don't race on stripe.Key writes. + // NOTE: assumes single-process — if you ever embed both the SSH and API + // binaries together, the last writer wins and one of them silently breaks. + stripe.Key = cfg.StripeSecretKey + // Seed database with initial data if err := database.Seed(db); err != nil { log.Fatalf("Failed to seed database: %v", err) @@ -71,7 +79,7 @@ func main() { jwtManager := auth.NewJWTManager(cfg.JWTSecret, 30*time.Minute) // Setup routes - router := routes.SetupRoutes(version, cfg.StripeSecretKey, cfg.StripeWebhookSecret, jwtManager, cfg.AuthFingerprintKey, cfg.ShippoAPIKey, cfg.BringAPIUID, cfg.BringAPIKey, cfg.BringCustomerNumber, cfg.ShippoWebhookSecret, cfg.AppURL, cfg.SlackSigningSecret, cfg.AdminAPIKey, cfg.APIPort, cfg.MaxOrderCents) + router := routes.SetupRoutes(version, cfg.StripeWebhookSecret, jwtManager, cfg.AuthFingerprintKey, cfg.ShippoAPIKey, cfg.BringAPIUID, cfg.BringAPIKey, cfg.BringCustomerNumber, cfg.ShippoWebhookSecret, cfg.AppURL, cfg.SlackSigningSecret, cfg.AdminAPIKey, cfg.APIPort, cfg.MaxOrderCents) // Create HTTP server server := &http.Server{ @@ -106,7 +114,7 @@ func main() { for { select { case <-ticker.C: - handlers.ReconcileOrders(cfg.StripeSecretKey) + handlers.ReconcileOrders() case <-ctx.Done(): return } @@ -137,7 +145,21 @@ func main() { for { select { case <-ticker.C: - handlers.ReconcileExpiredCards(cfg.StripeSecretKey) + handlers.ReconcileExpiredCards() + case <-ctx.Done(): + return + } + } + }() + + 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(threshold) case <-ctx.Done(): return } diff --git a/api/routes/routes.go b/api/routes/routes.go index 9080d60..eb1816d 100644 --- a/api/routes/routes.go +++ b/api/routes/routes.go @@ -16,7 +16,6 @@ import ( // SetupRoutes configures all API routes func SetupRoutes( version string, - stripeSecretKey string, stripeWebhookSecret string, jwtManager *auth.JWTManager, authFingerprintKey string, @@ -47,12 +46,12 @@ func SetupRoutes( healthHandler := handlers.NewHealthHandler(version) productHandler := handlers.NewProductHandler() authHandler := handlers.NewAuthHandler(jwtManager, authFingerprintKey) - cartHandler := handlers.NewCartHandler(stripeSecretKey, appURL, maxOrderCents) - cardHandler := handlers.NewCardHandler(stripeSecretKey, appURL) - orderHandler := handlers.NewOrderHandler(stripeSecretKey, bringAPIUID, bringAPIKey, bringCustomerNumber) + cartHandler := handlers.NewCartHandler(appURL, maxOrderCents) + cardHandler := handlers.NewCardHandler(appURL) + orderHandler := handlers.NewOrderHandler(bringAPIUID, bringAPIKey, bringCustomerNumber) addressHandler := handlers.NewAddressHandler(shippoAPIKey, bringAPIUID, bringAPIKey) viewHandler := handlers.NewViewHandler() - webhookHandler := handlers.NewWebhookHandler(stripeWebhookSecret, stripeSecretKey, shippoWebhookSecret) + webhookHandler := handlers.NewWebhookHandler(stripeWebhookSecret, shippoWebhookSecret) slackHandler := handlers.NewSlackHandler(slackSigningSecret, adminAPIKey, apiPort, shippoWebhookSecret) // Short payment redirect and success page — no auth required. diff --git a/main.go b/main.go index a2cbfdd..05fc10d 100644 --- a/main.go +++ b/main.go @@ -15,6 +15,7 @@ import ( "charm.land/wish/v2/bubbletea" "charm.land/wish/v2/logging" "github.com/charmbracelet/ssh" + "github.com/stripe/stripe-go/v78" "terminalShop/pkg/auth" "terminalShop/pkg/config" @@ -31,7 +32,6 @@ func listenHost() string { var ( apiURL string authFingerprintKey string - stripePublicKey string ) func teaHandler(s ssh.Session) (tea.Model, []tea.ProgramOption) { @@ -43,7 +43,7 @@ func teaHandler(s ssh.Session) (tea.Model, []tea.ProgramOption) { pubKeyStr = auth.FormatSSHPublicKey(pubKey) } - m := tui.NewModelWithAuth(fingerprint, pubKeyStr, apiURL, authFingerprintKey, stripePublicKey) + m := tui.NewModelWithAuth(fingerprint, pubKeyStr, apiURL, authFingerprintKey) return m, nil } @@ -57,7 +57,15 @@ func main() { // Set API URL and auth key from config for TUI connections apiURL = cfg.APIURL authFingerprintKey = cfg.AuthFingerprintKey - stripePublicKey = cfg.StripePublicKey + + // Wire the publishable key into stripe-go's package-level global once. + // TUI uses the legacy Tokens API (stripetoken.New) which accepts the + // publishable key; per-session writes would race across concurrent SSH + // sessions even though they'd all write the same value. + // NOTE: assumes single-process — if you ever embed both the SSH and API + // binaries together, the last writer wins and one of them silently breaks. + stripe.Key = cfg.StripePublicKey + log.Printf("TUI will connect to API at: %s", apiURL) // Create SSH server 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..57f4b3a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -3,6 +3,7 @@ package config import ( "errors" "fmt" + "log/slog" "net/url" "os" "strconv" @@ -13,25 +14,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 +52,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 +153,26 @@ 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 { + slog.Warn("invalid ABANDONED_3DS_THRESHOLD_MINUTES, falling back", + "raw", raw, "default", defaultMinutes) + return defaultMinutes + } + if n < 10 { + slog.Warn("ABANDONED_3DS_THRESHOLD_MINUTES below 10 may race in-flight 3ds challenges", + "value", n) + } + return n +} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 7592f6b..f6ad27f 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1,16 +1,30 @@ package config import ( + "io" + "log/slog" + "os" "testing" ) +// TestMain silences the package's slog warnings so the negative/garbage +// env-var cases plus the sub-10 valid-minimum case don't spam test output. +// The actual fall-back behavior is still asserted via the case tables. +func TestMain(m *testing.M) { + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(io.Discard, nil))) + code := m.Run() + slog.SetDefault(prev) + os.Exit(code) +} + func TestLoadMaxOrderCents(t *testing.T) { cases := []struct { name string env string // "" means unset want int }{ - {"unset uses default", "", 20000}, + {"empty falls back to default", "", 20000}, {"explicit zero disables", "0", 0}, {"valid override", "50000", 50000}, {"negative falls back to default", "-1", 20000}, @@ -32,3 +46,33 @@ func TestLoadMaxOrderCents(t *testing.T) { }) } } + +// TestLoadAbandoned3DSThresholdMinutes covers the env-parsing surface for +// the sweep threshold. Zero and negative fall back to the default (unlike +// MaxOrderCents where zero is a deliberate off-switch) because a zero +// threshold would sweep every requires_action row on the first tick and +// race the customer's in-progress challenge. +func TestLoadAbandoned3DSThresholdMinutes(t *testing.T) { + cases := []struct { + name string + env string // "" means unset + want int + }{ + {"empty falls back to default", "", 30}, + {"valid override", "45", 45}, + {"zero falls back to default", "0", 30}, + {"negative falls back to default", "-5", 30}, + {"garbage falls back to default", "half an hour", 30}, + {"valid minimum (warns)", "1", 1}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Setenv("ABANDONED_3DS_THRESHOLD_MINUTES", tc.env) + got := loadAbandoned3DSThresholdMinutes() + if got != tc.want { + t.Errorf("loadAbandoned3DSThresholdMinutes() = %d, want %d", got, tc.want) + } + }) + } +} diff --git a/pkg/tui/model.go b/pkg/tui/model.go index aaf6b7e..7923910 100644 --- a/pkg/tui/model.go +++ b/pkg/tui/model.go @@ -75,7 +75,6 @@ type Model struct { Fingerprint string AccessToken string AuthFingerprintKey string // shared secret for /auth/token refresh - StripePublicKey string // One-shop auth command - capture pubKeyStr in closure, not stored on Model authCmd tea.Cmd @@ -736,7 +735,6 @@ func (m Model) saveCardOnlyCmd(form PaymentFormCompleteMsg) tea.Cmd { if m.APIClient == nil { return CardSavedForReviewMsg{Err: fmt.Errorf("API client not available")} } - stripe.Key = m.StripePublicKey tok, err := stripetoken.New(&stripe.TokenParams{ Card: &stripe.CardParams{ Name: stripe.String(form.CardName), @@ -905,11 +903,12 @@ func NewModel(username string) Model { } // NewModelWithAuth creates a new model with user authentication context. -func NewModelWithAuth(fingerprint string, pubKeyStr string, apiURL string, clientSecret string, stripePublicKey string) Model { +// The publishable Stripe key is wired into stripe.Key once at startup in +// the SSH binary's main, so it is not threaded through here. +func NewModelWithAuth(fingerprint string, pubKeyStr string, apiURL string, clientSecret string) Model { m := NewModel("") m.Fingerprint = fingerprint m.AuthFingerprintKey = clientSecret - m.StripePublicKey = stripePublicKey m.currentPage = splashPage if apiURL != "" { m.APIClient = api.NewClient(apiURL, "")