From 9be048b30349ec2f3ddf710a09203ce69074d372 Mon Sep 17 00:00:00 2001 From: marcusDenslow Date: Sun, 7 Jun 2026 18:08:16 +0200 Subject: [PATCH] refactor(stripe): drop dead stripeKey fields and pre-call sdk params --- api/handlers/card_test.go | 4 +-- api/handlers/cards.go | 20 ++++++----- api/handlers/cards_expire_test.go | 4 +-- api/handlers/cart.go | 14 ++++---- api/handlers/cart_test.go | 55 ++++++++++++++++--------------- api/handlers/orders.go | 6 ++-- api/handlers/orders_test.go | 6 ++-- api/handlers/reconcile.go | 13 +++----- api/handlers/reconcile_test.go | 20 +++++------ api/handlers/webhook.go | 8 ++--- api/handlers/webhook_test.go | 12 +++---- api/main.go | 10 +++--- api/routes/routes.go | 9 +++-- main.go | 6 ++-- pkg/config/config_test.go | 16 ++++++++- pkg/tui/model.go | 6 ++-- 16 files changed, 112 insertions(+), 97 deletions(-) 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 6785458..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 } @@ -350,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) @@ -364,7 +366,7 @@ func expireStoredCard(db *gorm.DB, card *models.Card, stripeKey string) error { return err } - if stripeKey != "" && strings.HasPrefix(card.StripePaymentID, "pm_") { + 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 106e5e7..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 } @@ -704,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 } 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 cd9dafd..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, 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 0955283..94a3cad 100644 --- a/api/handlers/reconcile.go +++ b/api/handlers/reconcile.go @@ -26,7 +26,7 @@ func reconcileLog() *slog.Logger { return slog.With("component", "reconcile") } // 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(_ string) { +func ReconcileOrders() { db := database.GetDB() var orders []models.Order @@ -88,11 +88,8 @@ func ReconcileOrders(_ string) { // 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). -// -// stripeKey is unused; stripe.Key is wired once in main.go at startup -// so concurrent reconcilers don't race on the package-level global. Parameter -// kept to match the other reconciler's signature for the ticker call site. -func ReconcileStale3DSOrders(_ string, threshold time.Duration) { +// stripe.Key is wired once at startup in api/main.go. +func ReconcileStale3DSOrders(threshold time.Duration) { db := database.GetDB() var orders []models.Order @@ -181,7 +178,7 @@ func flipStale3DSToFailed(db *gorm.DB, order *models.Order, stripeStatus string) // 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 @@ -198,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 index 1b25b5d..7c62b61 100644 --- a/api/handlers/reconcile_test.go +++ b/api/handlers/reconcile_test.go @@ -39,7 +39,7 @@ func TestReconcileStale3DSOrders_FlipsAbandonedToFailed(t *testing.T) { }, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) var reloaded models.Order if err := db.First(&reloaded, order.ID).Error; err != nil { @@ -85,7 +85,7 @@ func TestReconcileStale3DSOrders_SkipsRecent(t *testing.T) { }, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) if called { t.Fatalf("stripe Get called for in-window row") @@ -124,7 +124,7 @@ func TestReconcileStale3DSOrders_SkipsWhenStripeSaysSucceeded(t *testing.T) { }, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) var reloaded models.Order if err := db.First(&reloaded, order.ID).Error; err != nil { @@ -161,7 +161,7 @@ func TestReconcileStale3DSOrders_GuardsAgainstWebhookRace(t *testing.T) { }, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) var reloaded models.Order if err := db.First(&reloaded, order.ID).Error; err != nil { @@ -196,7 +196,7 @@ func TestReconcileStale3DSOrders_FlipsCanceledPI(t *testing.T) { }, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) var reloaded models.Order if err := db.First(&reloaded, order.ID).Error; err != nil { @@ -232,7 +232,7 @@ func TestReconcileStale3DSOrders_DoesNotConsumeRetryCap(t *testing.T) { }, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) var retryCount int64 if err := db.Model(&models.OrderEvent{}). @@ -275,7 +275,7 @@ func TestReconcileStale3DSOrders_SkipsTransitioningStatuses(t *testing.T) { return &stripe.PaymentIntent{ID: id, Status: tc.status}, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) var reloaded models.Order if err := db.First(&reloaded, order.ID).Error; err != nil { @@ -312,7 +312,7 @@ func TestReconcileStale3DSOrders_SkipsUnknownStatus(t *testing.T) { }, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) var reloaded models.Order if err := db.First(&reloaded, order.ID).Error; err != nil { @@ -344,7 +344,7 @@ func TestReconcileStale3DSOrders_SkipsOnStripeError(t *testing.T) { return nil, fmt.Errorf("simulated stripe outage") } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) var reloaded models.Order if err := db.First(&reloaded, order.ID).Error; err != nil { @@ -381,7 +381,7 @@ func TestReconcileStale3DSOrders_SkipsEmptyPaymentIntentID(t *testing.T) { return &stripe.PaymentIntent{Status: stripe.PaymentIntentStatusRequiresPaymentMethod}, nil } - ReconcileStale3DSOrders("", 30*time.Minute) + ReconcileStale3DSOrders(30 * time.Minute) if called { t.Fatalf("stripe Get called for empty-PI row") diff --git a/api/handlers/webhook.go b/api/handlers/webhook.go index 55e39ee..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. 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 8c4c4ad..0140d4d 100644 --- a/api/main.go +++ b/api/main.go @@ -53,6 +53,8 @@ func main() { // 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 @@ -77,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{ @@ -112,7 +114,7 @@ func main() { for { select { case <-ticker.C: - handlers.ReconcileOrders(cfg.StripeSecretKey) + handlers.ReconcileOrders() case <-ctx.Done(): return } @@ -143,7 +145,7 @@ func main() { for { select { case <-ticker.C: - handlers.ReconcileExpiredCards(cfg.StripeSecretKey) + handlers.ReconcileExpiredCards() case <-ctx.Done(): return } @@ -157,7 +159,7 @@ func main() { for { select { case <-ticker.C: - handlers.ReconcileStale3DSOrders(cfg.StripeSecretKey, threshold) + 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 c0c69b5..05fc10d 100644 --- a/main.go +++ b/main.go @@ -32,7 +32,6 @@ func listenHost() string { var ( apiURL string authFingerprintKey string - stripePublicKey string ) func teaHandler(s ssh.Session) (tea.Model, []tea.ProgramOption) { @@ -44,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 } @@ -58,12 +57,13 @@ 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) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index e3fd7c4..f6ad27f 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1,9 +1,23 @@ 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 @@ -49,7 +63,7 @@ func TestLoadAbandoned3DSThresholdMinutes(t *testing.T) { {"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", "1", 1}, + {"valid minimum (warns)", "1", 1}, } for _, tc := range cases { diff --git a/pkg/tui/model.go b/pkg/tui/model.go index f584b6b..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 @@ -904,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, "")