mirror of
https://github.com/marcusDenslow/terminalShop.git
synced 2026-10-25 10:02:21 +01:00
fix: prevent charge-without-order by creating order before charging
When paying the application charges the card and then sends a payment record to the database after, in a situation where something goes wrong between those two steps, we could potentially end up where a users card was charged but no record on what is was charged for. The fix is to re-order ConvertCart to write a pending order record before calling Stripe, this way a DB record always exists if anything fails mid-operation. This implementation uses an idempotency key keyed to the order ID to prevent double-charges on retry. Wraps the post-charge DB updates in a transaction so marking paid andd clearing the cart are atomic. Adds OrderStatusFailed and a CRITICAL log for the unrecoverable case. Includes a checkout flow doc with test cards and reconcilation steps.
This commit is contained in:
@@ -83,7 +83,7 @@ func (h *CardHandler) SetDefaultCard(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
db.Model(&models.Card{}).Where("user_id = ?", userID).Update("id_default", false)
|
||||
db.Model(&models.Card{}).Where("user_id = ?", userID).Update("is_default", false)
|
||||
|
||||
card.IsDefault = true
|
||||
db.Save(&card)
|
||||
|
||||
@@ -3,6 +3,7 @@ package handlers
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"log"
|
||||
"net/http"
|
||||
|
||||
"terminalShop/api/middleware"
|
||||
@@ -10,6 +11,7 @@ import (
|
||||
"terminalShop/pkg/models"
|
||||
"terminalShop/pkg/utils"
|
||||
|
||||
"gorm.io/gorm"
|
||||
"github.com/stripe/stripe-go/v78"
|
||||
"github.com/stripe/stripe-go/v78/customer"
|
||||
"github.com/stripe/stripe-go/v78/paymentintent"
|
||||
@@ -339,45 +341,13 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) {
|
||||
db.Save(&card)
|
||||
}
|
||||
|
||||
// Charge via PaymentIntent (matching reference implementation)
|
||||
piParams := &stripe.PaymentIntentParams{
|
||||
Amount: stripe.Int64(int64(total)),
|
||||
Currency: stripe.String(string(stripe.CurrencyUSD)),
|
||||
Customer: stripe.String(user.StripeCustomerID),
|
||||
PaymentMethod: stripe.String(paymentMethodID),
|
||||
AutomaticPaymentMethods: &stripe.PaymentIntentAutomaticPaymentMethodsParams{
|
||||
Enabled: stripe.Bool(true),
|
||||
AllowRedirects: stripe.String("never"),
|
||||
},
|
||||
Confirm: stripe.Bool(true),
|
||||
Description: stripe.String(fmt.Sprintf("terminal.shop order for user %d", user.ID)),
|
||||
Shipping: &stripe.ShippingDetailsParams{
|
||||
Name: stripe.String(address.Name),
|
||||
Address: &stripe.AddressParams{
|
||||
Line1: stripe.String(address.Street),
|
||||
Line2: stripe.String(address.Street2),
|
||||
City: stripe.String(address.City),
|
||||
State: stripe.String(address.State),
|
||||
PostalCode: stripe.String(address.Zip),
|
||||
Country: stripe.String(address.Country),
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
pi, err := paymentintent.New(piParams)
|
||||
if err != nil {
|
||||
if stripeErr, ok := err.(*stripe.Error); ok && stripeErr.Type == stripe.ErrorTypeCard {
|
||||
utils.RespondError(w, http.StatusPaymentRequired, "CARD_ERROR", stripeErr.Msg, nil)
|
||||
return
|
||||
}
|
||||
utils.RespondError(w, http.StatusPaymentRequired, "PAYMENT_FAILED", "payment failed", nil)
|
||||
return
|
||||
}
|
||||
order := models.Order{
|
||||
// Create the order record first before charging the card so we always
|
||||
// have a record we can reconcile against if anything goes wrong mid-flight.
|
||||
UserID: user.ID,
|
||||
CardID: card.ID,
|
||||
StripePaymentID: pi.ID,
|
||||
Status: models.OrderStatusPaid,
|
||||
StripePaymentID: "",
|
||||
Status: models.OrderStatusPending,
|
||||
Subtotal: subtotal,
|
||||
ShippingCost: cart.ShippingCost,
|
||||
Total: total,
|
||||
@@ -397,12 +367,63 @@ func (h *CartHandler) ConvertCart(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
db.Where("cart_id = ?", cart.ID).Delete(&models.CartItem{})
|
||||
// Charge via stripe, keyed to the order ID so retrieves never double-charge.
|
||||
piParams := &stripe.PaymentIntentParams{
|
||||
Amount: stripe.Int64(int64(total)),
|
||||
Currency: stripe.String(string(stripe.CurrencyUSD)),
|
||||
Customer: stripe.String(user.StripeCustomerID),
|
||||
PaymentMethod: stripe.String(paymentMethodID),
|
||||
AutomaticPaymentMethods: &stripe.PaymentIntentAutomaticPaymentMethodsParams{
|
||||
Enabled: stripe.Bool(true),
|
||||
AllowRedirects: stripe.String("never"),
|
||||
},
|
||||
Confirm: stripe.Bool(true),
|
||||
Description: stripe.String(fmt.Sprintf("terminal.shop order for user %d", userID)),
|
||||
Shipping: &stripe.ShippingDetailsParams{
|
||||
Name: stripe.String(address.Name),
|
||||
Address: &stripe.AddressParams{
|
||||
Line1: stripe.String(address.Street),
|
||||
Line2: stripe.String(address.Street2),
|
||||
City: stripe.String(address.City),
|
||||
State: stripe.String(address.State),
|
||||
PostalCode: stripe.String(address.Zip),
|
||||
Country: stripe.String(address.Country),
|
||||
},
|
||||
},
|
||||
}
|
||||
piParams.SetIdempotencyKey(fmt.Sprintf("order-%d", order.ID))
|
||||
|
||||
db.Model(cart).Updates(map[string]interface{}{
|
||||
"address_id": nil,
|
||||
"card_id": nil,
|
||||
pi, err := paymentintent.New(piParams)
|
||||
if err != nil {
|
||||
// charge failed, mark the otder so it can be identified and or cleaned up
|
||||
db.Model(&order).Update("status", models.OrderStatusFailed)
|
||||
if stripeErr, ok := err.(*stripe.Error); ok && stripeErr.Type == stripe.ErrorTypeCard {
|
||||
utils.RespondError(w, http.StatusPaymentRequired, "CARD_ERROR", stripeErr.Msg, nil)
|
||||
return
|
||||
}
|
||||
utils.RespondError(w, http.StatusPaymentRequired, "PAYMENT_FAILED", "payment failed", nil)
|
||||
return
|
||||
}
|
||||
|
||||
txErr := db.Transaction(func(tx *gorm.DB) error {
|
||||
order.StripePaymentID = pi.ID
|
||||
order.Status = models.OrderStatusPaid
|
||||
if err := tx.Save(&order).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
if err := tx.Where("cart_id = ?", cart.ID).Delete(&models.CartItem{}).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
return tx.Model(cart).Updates(map[string]interface{}{
|
||||
"address_id": nil,
|
||||
"card_id": nil,
|
||||
}).Error
|
||||
})
|
||||
if txErr != nil {
|
||||
log.Printf("[CRITICAL] order %d charged (pi=%s) but failed to record: %v", order.ID, pi.ID, txErr)
|
||||
utils.RespondError(w, http.StatusInternalServerError, "DATABASE_ERROR", "payment succeeded but failed to record order", nil)
|
||||
return
|
||||
}
|
||||
|
||||
db.Preload("Items").First(&order, order.ID)
|
||||
|
||||
|
||||
83
docs/checkout.md
Normal file
83
docs/checkout.md
Normal file
@@ -0,0 +1,83 @@
|
||||
# Checkout FLow
|
||||
|
||||
## The Problemo
|
||||
|
||||
Charging a card and recording the order are two different operations accross
|
||||
two seperate systems (Stripe and the database). If anything fails betweenj
|
||||
them, you can end up in a state where a customer was charged but no
|
||||
order exists in the database.
|
||||
|
||||
## How It Works
|
||||
|
||||
The ´ConvertCart´ handler (´api/handlers/cart.go´) avoids this by following
|
||||
a specific order of operations:
|
||||
|
||||
1. **Create the order first** written to the DB with ´status = "pending"´
|
||||
and no Stripe ID yet. This means there is always a DB record before any
|
||||
money moves.
|
||||
|
||||
+2. **Charge Stripe** — using an idempotency key of `order-{id}`. If the
|
||||
request is retried for any reason, Stripe will return the original result
|
||||
instead of charging twice.
|
||||
|
||||
+3. **On charge failure** — the order is marked `status = "failed"` and the
|
||||
error is returned to the client. The cart is left untouched.
|
||||
|
||||
+4. **On charge success** — a single DB transaction atomically:
|
||||
- Sets `status = "paid"` and writes the Stripe PaymentIntent ID
|
||||
- Deletes the cart items
|
||||
- Clears the address and card from the cart
|
||||
|
||||
+5. **If the transaction fails after a successful charge** — this is the
|
||||
worst case. The charge went through but the DB update failed. A
|
||||
`[CRITICAL]` log line is emitted with the order ID and PaymentIntent ID
|
||||
for manual reconciliation. See below.
|
||||
|
||||
## Reconciliation
|
||||
|
||||
If a `[CRITICAL]` log appears, find the order and patch it manually:
|
||||
|
||||
```sql
|
||||
UPDATE orders
|
||||
SET status = 'paid', stripe_payment_id = '<pi_id_from_log>'
|
||||
WHERE id = <order_id_from_log>;
|
||||
```
|
||||
|
||||
Then verify the PaymentIntent status in the Stripe dashboard matches.
|
||||
|
||||
A pending order with no ´stripe_payment_id´ after more than a few minutes
|
||||
indicates a failed charge (status should be ´failed´). A pending order
|
||||
with a ´stripe_payment_id´ indicates a reconciliation gap.
|
||||
|
||||
## Testing
|
||||
|
||||
Use Stripe´s test mode with the following card numbers (any future expiry,
|
||||
any CVC, any ZIP):
|
||||
|
||||
the folling table is constructed by ["Card number"]["Behaviour"]
|
||||
|
||||
4242 4242 4242 4242 - Success
|
||||
4000 0000 0000 0002 - Generic decline
|
||||
4000 0000 0000 9995 - Insufficient funds
|
||||
4000 0000 0000 0069 - Expired card
|
||||
4000 0000 0000 0127 - Incorrect CVC
|
||||
|
||||
### What to verify on success
|
||||
- Response is ´200´ with an order object
|
||||
- Order in DB has ´status = "paid"´ and a non-empty ´stripe_payment_id´
|
||||
- Cart items for that user are deleted
|
||||
- Cart ´address_id´ and ´card_id´ are cleared
|
||||
|
||||
### What to verify on decline
|
||||
- Response is `402` with `CARD_ERROR`
|
||||
- Order in DB has `status = "failed"`
|
||||
- Cart items are untouched
|
||||
|
||||
## Known Limitations
|
||||
|
||||
A server crash (not an error — a literal process kill) between the Stripe
|
||||
charge succeeding and the DB transaction committing will leave the order as
|
||||
`pending` with no `stripe_payment_id`. The `[CRITICAL]` log won't fire
|
||||
because the process is dead. The fix is a reconciliation job that
|
||||
periodically queries Stripe for PaymentIntents and cross-checks them
|
||||
against orders — not implemented yet.
|
||||
@@ -16,6 +16,7 @@ const (
|
||||
OrderStatusDelivered OrderStatus = "delivered"
|
||||
OrderStatusCancelled OrderStatus = "cancelled"
|
||||
OrderStatusRefunded OrderStatus = "refunded"
|
||||
OrderStatusFailed OrderStatus = "failed"
|
||||
)
|
||||
|
||||
// Order represents a completed purchase.
|
||||
|
||||
Reference in New Issue
Block a user