From 9d5096b2707d81a23a53aa008ba1c65499807ca5 Mon Sep 17 00:00:00 2001 From: marcus06 Date: Fri, 19 Jun 2026 10:04:35 +0200 Subject: [PATCH] fix(privacy): forget saved card after privacy-mode checkout (#59) --- api/handlers/cart.go | 30 ++++++++-- api/handlers/ephemeral_test.go | 106 +++++++++++++++++++++++++++++---- api/handlers/webhook.go | 16 ++++- 3 files changed, 132 insertions(+), 20 deletions(-) diff --git a/api/handlers/cart.go b/api/handlers/cart.go index 01d91fa..b6ab96e 100644 --- a/api/handlers/cart.go +++ b/api/handlers/cart.go @@ -335,11 +335,15 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) { return } - // Privacy mode is a SOFT default enforced in the TUI, not here: the backend - // trusts the checkout's explicit per-order intent so a customer can override - // their account default with "save anyway" at checkout. See - // AccountHandler.SetPrivacyMode. + // Per-order intent drives the charge SHAPE: req.Ephemeral picks the one-time + // token path (no saved card). Privacy mode adds a second guarantee on the + // saved-card path — the live QR add-card flow has no "save anyway" prompt, so + // a privacy user who selects a (QR-collected) card still must not have it + // retained. forgetCard charges that card normally, then detaches the + // PaymentMethod and hard-deletes the local Card row once the order settles. + // It never applies to the token path, which has no local row. ephemeral := req.Ephemeral + forgetCard := user.PrivacyMode && !ephemeral var card models.Card var paymentMethodID string @@ -488,7 +492,7 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) { UserID: user.ID, CardID: card.ID, StripePaymentID: "", - Ephemeral: ephemeral, + Ephemeral: ephemeral || forgetCard, Status: models.OrderStatusPending, Subtotal: subtotal, ShippingCost: cart.ShippingCost, @@ -625,11 +629,25 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) { return } - if ephemeral { + if ephemeral || forgetCard { if _, derr := paymentMethodDetachFn(paymentMethodID, nil); derr != nil { slog.Warn("failed to detach ephemeral payment method", "order_id", order.ID, "error", derr) } } + if forgetCard { + // Privacy mode: the card was persisted only long enough to select and + // charge it. Null the order's reference and hard-delete the local row so + // no card data (last4/brand/token) survives the checkout. Unscoped because + // Card uses gorm soft-delete; a plain delete would leave the data on disk. + // Mirrors upstream Card.remove (terminal-shop-source core/src/card/index.ts): + // detach the PM, drop the FK, delete the row. + if err := db.Model(&order).Update("card_id", 0).Error; err != nil { + slog.Warn("failed to null forgotten card reference", "order_id", order.ID, "error", err) + } + if err := db.Unscoped().Delete(&models.Card{}, card.ID).Error; err != nil { + slog.Warn("failed to delete forgotten card", "order_id", order.ID, "card_id", card.ID, "error", err) + } + } audit.OrderPaid(userID, order.ID, total, pi.ID) middleware.RecordOrderCreated(string(order.Status)) diff --git a/api/handlers/ephemeral_test.go b/api/handlers/ephemeral_test.go index 2220d52..b68f269 100644 --- a/api/handlers/ephemeral_test.go +++ b/api/handlers/ephemeral_test.go @@ -290,11 +290,14 @@ func TestHandlePaymentIntentSucceeded_EphemeralDetach(t *testing.T) { } } -// TestConvertCart_PrivacyModeSoftDefault proves PrivacyMode is NOT enforced -// server-side: a user with privacy_mode on who explicitly checks out a saved -// card (ephemeral=false) gets the normal kept-card charge — Customer attached, -// PM not detached, order not Ephemeral. This is what makes "save anyway" work. -func TestConvertCart_PrivacyModeSoftDefault(t *testing.T) { +// TestConvertCart_PrivacyModeForgetsCard proves the privacy guarantee on the +// saved-card path: a privacy_mode user who checks out a saved (QR-collected) card +// is charged normally (Customer attached — the PM is customer-attached, so the +// charge requires it), but once the charge settles the PaymentMethod is detached +// and the local Card row is hard-deleted, and the order is marked Ephemeral. The +// live QR add-card flow has no "save anyway" prompt, so privacy mode is the +// authoritative signal here. +func TestConvertCart_PrivacyModeForgetsCard(t *testing.T) { testDB, user := setupCartTestDB(t) defer func() { _ = os.Remove(testDB) }() defer database.ResetForTesting() @@ -306,15 +309,15 @@ func TestConvertCart_PrivacyModeSoftDefault(t *testing.T) { seedConvertReadyCart(t, user) // seeds a SAVED card + address + item var piParams *stripe.PaymentIntentParams - detachCalled := false + detached := "" origPI, origDetach := paymentIntentNewFn, paymentMethodDetachFn defer func() { paymentIntentNewFn, paymentMethodDetachFn = origPI, origDetach }() paymentIntentNewFn = func(p *stripe.PaymentIntentParams) (*stripe.PaymentIntent, error) { piParams = p - return &stripe.PaymentIntent{ID: "pi_kept", Status: stripe.PaymentIntentStatusSucceeded}, nil + return &stripe.PaymentIntent{ID: "pi_forget", Status: stripe.PaymentIntentStatusSucceeded}, nil } paymentMethodDetachFn = func(id string, _ *stripe.PaymentMethodDetachParams) (*stripe.PaymentMethod, error) { - detachCalled = true + detached = id return &stripe.PaymentMethod{ID: id}, nil } @@ -325,17 +328,94 @@ func TestConvertCart_PrivacyModeSoftDefault(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("convert: want 200, got %d body=%s", w.Code, w.Body.String()) } + // The PM is attached to the Stripe customer, so the charge must still set + // Customer — you cannot charge an attached PM without it. if piParams == nil || piParams.Customer == nil { - t.Error("saved-card path must attach a Customer even when privacy_mode is on") + t.Error("saved-card charge must attach a Customer") } - if detachCalled { - t.Error("must NOT detach a saved card") + // ...but privacy mode forgets it afterward: detach + hard-delete. + if detached != "pm_test_cit" { + t.Errorf("forgotten card PM must be detached after settle; detached=%q", detached) + } + var cardCount int64 + db.Unscoped().Model(&models.Card{}).Where("user_id = ?", user.ID).Count(&cardCount) + if cardCount != 0 { + t.Errorf("privacy checkout must HARD-delete the local card row; found %d", cardCount) } var order models.Order if err := db.Where("user_id = ?", user.ID).First(&order).Error; err != nil { t.Fatalf("load order: %v", err) } - if order.Ephemeral { - t.Error("explicit ephemeral=false must win over privacy_mode (soft default)") + if !order.Ephemeral { + t.Error("forgotten-card order must be marked Ephemeral") + } + if order.CardID != 0 { + t.Errorf("forgotten-card order must null its CardID, got %d", order.CardID) + } + if order.Status != models.OrderStatusPaid { + t.Errorf("status: want paid, got %q", order.Status) + } +} + +// TestHandlePaymentIntentSucceeded_ForgetsCard covers the deferred (3DS) twin of +// the inline forget-card cleanup: an Ephemeral order that still carries a local +// Card row (CardID != 0, the privacy + saved/QR-card case) has that row +// hard-deleted and its CardID nulled when the webhook marks it paid. +func TestHandlePaymentIntentSucceeded_ForgetsCard(t *testing.T) { + testDB, user := setupCartTestDB(t) + defer func() { _ = os.Remove(testDB) }() + defer database.ResetForTesting() + + db := database.GetDB() + card := models.Card{ + UserID: user.ID, StripePaymentID: "pm_forget_webhook", + Last4: "4242", Brand: "Visa", ExpMonth: 12, ExpYear: 2030, + } + if err := db.Create(&card).Error; err != nil { + t.Fatalf("seed card: %v", err) + } + order := models.Order{ + UserID: user.ID, CardID: card.ID, Ephemeral: true, + StripePaymentID: "pi_forget_webhook", 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) + } + + origDetach := paymentMethodDetachFn + defer func() { paymentMethodDetachFn = origDetach }() + paymentMethodDetachFn = func(id string, _ *stripe.PaymentMethodDetachParams) (*stripe.PaymentMethod, error) { + return &stripe.PaymentMethod{ID: id}, nil + } + + pi := map[string]any{ + "id": order.StripePaymentID, + "amount": order.Total, + "currency": "usd", + "payment_method": "pm_forget_webhook", + "metadata": map[string]string{"order_id": fmt.Sprintf("%d", order.ID)}, + } + raw, _ := json.Marshal(pi) + evt := stripe.Event{Type: "payment_intent.succeeded", Data: &stripe.EventData{Raw: raw}} + + NewWebhookHandler("", "").handlePaymentIntentSucceeded(context.Background(), evt) + + var cardCount int64 + db.Unscoped().Model(&models.Card{}).Where("id = ?", card.ID).Count(&cardCount) + if cardCount != 0 { + t.Errorf("webhook must hard-delete the forgotten card; found %d", cardCount) + } + var reloaded models.Order + if err := db.First(&reloaded, order.ID).Error; err != nil { + t.Fatalf("reload order: %v", err) + } + if reloaded.Status != models.OrderStatusPaid { + t.Errorf("status: want paid, got %q", reloaded.Status) + } + if reloaded.CardID != 0 { + t.Errorf("order CardID must be nulled, got %d", reloaded.CardID) } } diff --git a/api/handlers/webhook.go b/api/handlers/webhook.go index 5cfd094..ba397dd 100644 --- a/api/handlers/webhook.go +++ b/api/handlers/webhook.go @@ -174,7 +174,9 @@ func (h *WebhookHandler) handlePaymentIntentSucceeded(ctx context.Context, event return } - if order.CardID != 0 { + // A retained (non-ephemeral) card gets its storage TTL refreshed on use. + // Ephemeral orders are about to lose their card entirely, so skip the refresh. + if order.CardID != 0 && !order.Ephemeral { if err := refreshCardStorageTTL(db, order.CardID, time.Now()); err != nil { webhookLog().Warn("failed to refresh card storage ttl", "card_id", order.CardID, "error", err) } @@ -187,6 +189,18 @@ func (h *WebhookHandler) handlePaymentIntentSucceeded(ctx context.Context, event } } + // Deferred (3DS) twin of the inline forget-card cleanup in ConvertCart: a + // privacy-mode order that selected a saved/QR card carries CardID != 0. Now + // that it is paid, hard-delete the local row (Unscoped: Card soft-deletes by + // default). Token-ephemeral orders have CardID == 0 and skip this. + if order.Ephemeral && order.CardID != 0 { + if err := db.Unscoped().Delete(&models.Card{}, order.CardID).Error; err != nil { + webhookLog().Warn("failed to delete forgotten card", "order_id", order.ID, "card_id", order.CardID, "error", err) + } else if err := db.Model(&order).Update("card_id", 0).Error; err != nil { + webhookLog().Warn("failed to null forgotten card reference", "order_id", order.ID, "error", err) + } + } + audit.OrderPaid(order.UserID, order.ID, int(pi.Amount), pi.ID) go notify.SlackOrderPaid(&order) webhookLog().Info("order marked paid", "order_id", orderIDStr, "pi", pi.ID)