From 6aae653dac1c20cf03ff413babc7685ef183d3a4 Mon Sep 17 00:00:00 2001 From: "Fabian @ Blax Software" Date: Fri, 25 Sep 2026 12:03:38 +0200 Subject: [PATCH] fix(stripe): safe return from Checkout, retries and duplicate events - checkoutSession() keeps pending purchases in step with the cart: a buyer who cancels on Stripe and edits the cart pays for (and is invoiced) the cart as it is, not the first attempt. - checkout.session.completed is idempotent and locked per payment: a redelivered event, a concurrent charge.succeeded or the success page record the payment once. An unpaid (async) session creates the order but records no payment until Stripe settles it. - New StripeCheckoutConfirmation + WS cart.confirm {session_id}: the success page settles a paid session through the webhook's code path, so the order exists the moment the buyer is back. - CartService::guest() never hands a converted cart back to its session. Co-Authored-By: Claude Opus 5.5 (1M context) --- lang/de/cart.php | 1 + lang/en/cart.php | 1 + lang/pl/cart.php | 1 + .../Controllers/StripeWebhookController.php | 69 ++++- src/Models/Cart.php | 33 ++- src/Services/CartService.php | 12 + src/Services/StripeCheckoutConfirmation.php | 82 ++++++ src/Websocket/Controllers/CartController.php | 44 ++++ .../Unit/Stripe/StripeCheckoutReturnTest.php | 246 ++++++++++++++++++ 9 files changed, 480 insertions(+), 9 deletions(-) create mode 100644 src/Services/StripeCheckoutConfirmation.php create mode 100644 tests/Unit/Stripe/StripeCheckoutReturnTest.php diff --git a/lang/de/cart.php b/lang/de/cart.php index 1570751..2a38ac8 100644 --- a/lang/de/cart.php +++ b/lang/de/cart.php @@ -12,4 +12,5 @@ return [ 'checkout_failed' => 'Der Checkout konnte nicht gestartet werden.', 'checkout_unavailable' => 'Der Checkout ist derzeit nicht verfügbar.', 'checkout_created' => 'Du wirst zur Zahlung weitergeleitet.', + 'confirm_failed' => 'Wir konnten deine Zahlung gerade nicht prüfen. Bitte versuch es gleich noch einmal.', ]; diff --git a/lang/en/cart.php b/lang/en/cart.php index 5b69740..2520f5a 100644 --- a/lang/en/cart.php +++ b/lang/en/cart.php @@ -12,4 +12,5 @@ return [ 'checkout_failed' => 'Checkout could not be started.', 'checkout_unavailable' => 'Checkout is currently unavailable.', 'checkout_created' => 'Redirecting to payment.', + 'confirm_failed' => 'We could not check your payment. Please try again in a moment.', ]; diff --git a/lang/pl/cart.php b/lang/pl/cart.php index 94aa0a8..8228376 100644 --- a/lang/pl/cart.php +++ b/lang/pl/cart.php @@ -12,4 +12,5 @@ return [ 'checkout_failed' => 'Nie udało się rozpocząć płatności.', 'checkout_unavailable' => 'Płatność jest obecnie niedostępna.', 'checkout_created' => 'Przekierowujemy do płatności.', + 'confirm_failed' => 'Nie udało się sprawdzić płatności. Spróbuj ponownie za chwilę.', ]; diff --git a/src/Http/Controllers/StripeWebhookController.php b/src/Http/Controllers/StripeWebhookController.php index b5386cc..4f1c1f0 100644 --- a/src/Http/Controllers/StripeWebhookController.php +++ b/src/Http/Controllers/StripeWebhookController.php @@ -13,6 +13,7 @@ use Blax\Shop\Models\OrderNote; use Blax\Shop\Models\Product; use Blax\Shop\Models\ProductPurchase; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; use Stripe\Exception\SignatureVerificationException; use Stripe\Stripe; @@ -113,8 +114,25 @@ class StripeWebhookController return false; } + /** + * Fulfil a completed Checkout Session outside the webhook, e.g. when the + * buyer lands on the success page before Stripe's event does. Same code + * path (and lock) as `checkout.session.completed`, so whichever comes + * first creates the order and the other finds it done. + */ + public function completeCheckoutSession($session): bool + { + return $this->handleCheckoutSessionCompleted($session); + } + /** * Handle checkout.session.completed event + * + * Idempotent: Stripe delivers events at least once, and the success page + * may fulfil the same session (completeCheckoutSession). Runs under a lock + * per payment, records the payment once, and only when Stripe reports the + * session paid (async methods complete `unpaid` first and settle later via + * checkout.session.async_payment_succeeded). */ protected function handleCheckoutSessionCompleted($session): bool { @@ -126,13 +144,31 @@ class StripeWebhookController return false; } - $cart = Cart::find($cartId); - if (! $cart) { + if (! Cart::find($cartId)) { Log::warning('Stripe checkout session for non-existent cart', ['cart_id' => $cartId]); return false; } + return $this->withPaymentLock($session->payment_intent ?? "cart:{$cartId}", function () use ($cartId, $session) { + return $this->fulfilCheckoutSession(Cart::find($cartId), $session); + }); + } + + /** + * Run $callback while holding the lock for one payment, so concurrent + * deliveries (checkout.session.completed, charge.succeeded, the success + * page) never both create an order or record the same payment twice. + * A lock that can't be had in time throws, and Stripe retries the event. + */ + protected function withPaymentLock(string $key, callable $callback): mixed + { + return Cache::lock("shop:stripe-payment:{$key}", 60)->block(20, $callback); + } + + /** checkout.session.completed for a cart that exists; see handleCheckoutSessionCompleted(). */ + protected function fulfilCheckoutSession(Cart $cart, $session): bool + { // Only update if not already converted if ($cart->status !== CartStatus::CONVERTED) { $cart->update([ @@ -177,6 +213,22 @@ class StripeWebhookController $amountPaid = (int) ($session->amount_total ?? 0); $currency = strtoupper($session->currency ?? $order->currency ?? 'USD'); + $order->refresh(); + $settled = in_array($session->payment_status ?? 'paid', ['paid', 'no_payment_required'], true); + $recorded = $order->paid_at + || ($session->payment_intent && $order->payment_reference === $session->payment_intent && $order->amount_paid > 0); + + if (! $settled || $recorded) { + Log::info('Stripe checkout session: no payment to record', [ + 'order_id' => $order->id, + 'session_id' => $session->id, + 'payment_status' => $session->payment_status ?? null, + 'already_recorded' => (bool) $recorded, + ]); + + return true; + } + // recordPayment(int $amount, ?string $reference, ?string $method, ?string $provider) $order->recordPayment($amountPaid, $session->payment_intent, 'stripe', 'stripe'); @@ -406,9 +458,14 @@ class StripeWebhookController } } - // Try to find related order via payment_intent - $order = $this->findOrderByPaymentIntent($charge->payment_intent); - if ($order) { + // Try to find related order via payment_intent. Under the payment lock: a + // checkout.session.completed for the same payment may be recording it right now. + $this->withPaymentLock($charge->payment_intent ?? "charge:{$charge->id}", function () use ($charge) { + $order = $this->findOrderByPaymentIntent($charge->payment_intent); + if (! $order) { + return; + } + // Remember the charge so ledger rows (source_id = ch_…) can be matched to the order. if ($order->getMeta('stripe_charge_id') !== $charge->id) { $order->updateMetaKey('stripe_charge_id', $charge->id); @@ -418,7 +475,7 @@ class StripeWebhookController // Stripe amounts are cents, like every amount_* column. $order->recordPayment((int) $charge->amount, $charge->id, 'stripe', 'stripe'); } - } + }); $this->syncLedger($charge->id); diff --git a/src/Models/Cart.php b/src/Models/Cart.php index 606c504..75d996e 100644 --- a/src/Models/Cart.php +++ b/src/Models/Cart.php @@ -2430,11 +2430,38 @@ class Cart extends Model // Validate cart before proceeding (doesn't convert it) $this->validateForCheckout(); - // Create ProductPurchase records for each cart item + // Create ProductPurchase records for each cart item. A buyer who left an + // earlier session (cancel, back button) may have changed the cart since: + // pending purchases follow the lines as they are now, and the ones whose + // line is gone are dropped, so the order paid for is the cart on screen. DB::transaction(function () { + $this->unsetRelation('items'); + $linked = $this->items->pluck('purchase_id')->filter()->all(); + + ProductPurchase::query() + ->where('cart_id', $this->id) + ->where('status', PurchaseStatus::PENDING) + ->whereNotIn('id', $linked) + ->delete(); + foreach ($this->items as $item) { - // Skip if purchase already exists - if ($item->purchase_id) { + $existing = $item->purchase_id ? ProductPurchase::find($item->purchase_id) : null; + + if ($existing && $existing->status === PurchaseStatus::PENDING) { + $existing->update([ + 'price_id' => $item->price_id, + 'quantity' => $item->quantity, + 'amount' => $item->subtotal, + 'from' => $item->from, + 'until' => $item->until, + 'meta' => $item->meta, + ]); + + continue; + } + + // Skip if purchase already exists (and is past pending) + if ($existing) { continue; } diff --git a/src/Services/CartService.php b/src/Services/CartService.php index 3d22e7a..fee90d1 100644 --- a/src/Services/CartService.php +++ b/src/Services/CartService.php @@ -86,6 +86,18 @@ class CartService 'customer_type' => null, ]); + // A cart that became an order keeps its row but lets go of the session + // (session_id is unique), so the guest shops on with an empty cart. + if ($cart->isConverted()) { + $cart->update(['session_id' => 'converted_'.$cart->getKey()]); + + $cart = Cart::create([ + 'session_id' => $sessionId, + 'customer_id' => null, + 'customer_type' => null, + ]); + } + // Store cart ID in session Cart::setSession($cart); diff --git a/src/Services/StripeCheckoutConfirmation.php b/src/Services/StripeCheckoutConfirmation.php new file mode 100644 index 0000000..cd4bd6b --- /dev/null +++ b/src/Services/StripeCheckoutConfirmation.php @@ -0,0 +1,82 @@ +retrieve($sessionId); + + $cartId = data_get($session, 'metadata.cart_id') ?? data_get($session, 'client_reference_id'); + $cartModel = config('shop.models.cart', Cart::class); + $cart = $cartId ? $cartModel::find($cartId) : null; + + if (! $cart) { + throw new \InvalidArgumentException('This checkout session does not belong to a cart of this shop.'); + } + + $status = match (true) { + data_get($session, 'status') === 'expired' => self::EXPIRED, + data_get($session, 'status') !== 'complete' => self::OPEN, + in_array(data_get($session, 'payment_status'), ['paid', 'no_payment_required'], true) => self::PAID, + default => self::PROCESSING, + }; + + if ($status === self::PAID || $status === self::PROCESSING) { + app(StripeWebhookController::class)->completeCheckoutSession($session); + } + + $cart = $cart->fresh(); + + return [ + 'status' => $status, + 'order' => $cart?->order, + 'cart' => $cart, + ]; + } + + /** The Checkout Session from Stripe (overridable for tests). */ + protected function retrieve(string $sessionId): object + { + return (new StripeClient((string) config('services.stripe.secret'))) + ->checkout->sessions->retrieve($sessionId); + } +} diff --git a/src/Websocket/Controllers/CartController.php b/src/Websocket/Controllers/CartController.php index f734d91..92544db 100644 --- a/src/Websocket/Controllers/CartController.php +++ b/src/Websocket/Controllers/CartController.php @@ -8,6 +8,7 @@ use Blax\Shop\Enums\CartStatus; use Blax\Shop\Exceptions\NotEnoughStockException; use Blax\Shop\Facades\Cart as CartFacade; use Blax\Shop\Http\Resources\CartResource; +use Blax\Shop\Services\StripeCheckoutConfirmation; /** * Reusable storefront cart controller over the Blax WebSocket bridge. @@ -172,6 +173,49 @@ class CartController extends \BlaxSoftware\LaravelWebSockets\Websocket\Controlle ]); } + /** + * `cart.confirm {session_id}` → `{ status, order: {id, order_number, status}|null, cart }` + * + * The buyer is back from Stripe Checkout (the success URL carries + * `session_id`). Settles the session through the webhook's code path, see + * {@see StripeCheckoutConfirmation} for the statuses. `cart` is the cart + * this connection shops with now: a fresh one once the old cart became an + * order, so the client can swap its stored cart id right away. + */ + public function confirm() + { + $data = request()->validate([ + 'session_id' => 'required|string|max:255', + ]); + + try { + $result = app(StripeCheckoutConfirmation::class)->confirm($data['session_id']); + } catch (\Throwable $e) { + return $this->error([ + 'message' => __('shop::cart.confirm_failed'), + 'detail' => $e->getMessage(), + ]); + } + + $order = $result['order']; + + // booted() adopted the client's cart before the payment was settled; a + // converted cart hands this connection a fresh one. + if ($this->cart->isConverted()) { + $this->cart = CartFacade::adopt(null, $this->connection->socketId); + } + + return $this->success([ + 'status' => $result['status'], + 'order' => $order ? [ + 'id' => $order->getKey(), + 'order_number' => $order->order_number, + 'status' => $order->status, + ] : null, + 'cart' => CartResource::make($this->cart->fresh()), + ]); + } + /** * Resolve the overloaded `cart_item_id` into a purchasable (CartItem's * purchasable, else a ProductPrice, else a Product). diff --git a/tests/Unit/Stripe/StripeCheckoutReturnTest.php b/tests/Unit/Stripe/StripeCheckoutReturnTest.php new file mode 100644 index 0000000..65e511e --- /dev/null +++ b/tests/Unit/Stripe/StripeCheckoutReturnTest.php @@ -0,0 +1,246 @@ + true, 'services.stripe.secret' => 'sk_test_fake']); + + // Any real Stripe call fails at once instead of reaching the network. + $this->apiBase = \Stripe\Stripe::$apiBase; + \Stripe\Stripe::$apiBase = 'http://127.0.0.1:9'; + } + + protected function tearDown(): void + { + \Stripe\Stripe::$apiBase = $this->apiBase; + + parent::tearDown(); + } + + private function guestCartWith(Product $product, int $quantity): Cart + { + $cart = Cart::factory()->create(['session_id' => 'socket-A', 'customer_id' => null]); + $cart->addToCart($product, $quantity); + + return $cart->fresh(); + } + + private function product(int $price = 1000): Product + { + return Product::factory()->withPrices(unit_amount: $price)->create(['manage_stock' => false]); + } + + private function stripeSession(Cart $cart, array $overrides = []): object + { + return (object) array_merge([ + 'id' => 'cs_test_'.uniqid(), + 'status' => 'complete', + 'payment_intent' => 'pi_test_'.uniqid(), + 'metadata' => (object) ['cart_id' => $cart->id], + 'client_reference_id' => $cart->id, + 'amount_total' => (int) $cart->getTotal(), + 'currency' => 'eur', + 'payment_status' => 'paid', + ], $overrides); + } + + private function completed(object $session): bool + { + $controller = new StripeWebhookController; + $method = (new ReflectionClass($controller))->getMethod('handleCheckoutSessionCompleted'); + + return $method->invoke($controller, $session); + } + + /** Run checkoutSession() up to the Stripe call (which fails here by design). */ + private function startCheckout(Cart $cart): void + { + try { + $cart->fresh()->checkoutSession(); + } catch (\Throwable) { + // The session itself isn't under test, the purchases written before it are. + } + } + + private function confirmation(object $session): StripeCheckoutConfirmation + { + return new class($session) extends StripeCheckoutConfirmation + { + public function __construct(private object $session) {} + + protected function retrieve(string $sessionId): object + { + return $this->session; + } + }; + } + + #[Test] + public function a_second_checkout_attempt_updates_pending_purchases_to_the_edited_cart() + { + $kept = $this->product(1000); + $dropped = $this->product(500); + + $cart = $this->guestCartWith($kept, 1); + $cart->addToCart($dropped, 1); + + $this->startCheckout($cart); + $this->assertSame(2, ProductPurchase::where('cart_id', $cart->id)->count()); + + // Back from Stripe without paying: one more of the first, the second removed. + $cart = $cart->fresh(); + $cart->addToCart($kept, 2); + $cart->removeFromCart($cart->items()->where('purchasable_id', $dropped->id)->first(), 1); + + $this->startCheckout($cart); + + $purchases = ProductPurchase::where('cart_id', $cart->id)->get(); + $this->assertCount(1, $purchases); + $this->assertSame($kept->id, $purchases->first()->purchasable_id); + $this->assertSame(3, $purchases->first()->quantity); + $this->assertSame(3000, $purchases->first()->amount); + } + + #[Test] + public function a_redelivered_completed_event_records_the_payment_once() + { + $cart = $this->guestCartWith($this->product(1000), 2); + $session = $this->stripeSession($cart); + + $this->assertTrue($this->completed($session)); + $this->assertTrue($this->completed($session)); + + $order = $cart->fresh()->order; + $this->assertSame(2000, $order->amount_paid); + $this->assertNotNull($order->paid_at); + $this->assertSame(OrderStatus::PROCESSING, $order->status); + $this->assertSame(1, $order->notes()->where('content', 'like', 'Payment of%')->count()); + } + + #[Test] + public function an_unpaid_completed_session_creates_the_order_but_no_payment() + { + $cart = $this->guestCartWith($this->product(1000), 1); + $session = $this->stripeSession($cart, ['payment_status' => 'unpaid']); + + $this->completed($session); + + $order = $cart->fresh()->order; + $this->assertNotNull($order); + $this->assertSame(0, $order->amount_paid); + $this->assertNull($order->paid_at); + $this->assertSame(OrderStatus::PENDING, $order->status); + + // The async payment settles later: now it is recorded. + $this->completed($this->stripeSession($cart, ['id' => $session->id, 'payment_intent' => $session->payment_intent])); + + $this->assertSame(1000, $order->fresh()->amount_paid); + $this->assertNotNull($order->fresh()->paid_at); + } + + #[Test] + public function confirming_a_paid_session_creates_the_order_before_the_webhook() + { + $cart = $this->guestCartWith($this->product(1500), 1); + $session = $this->stripeSession($cart); + + $result = $this->confirmation($session)->confirm($session->id); + + $this->assertSame(StripeCheckoutConfirmation::PAID, $result['status']); + $this->assertNotNull($result['order']); + $this->assertSame(1500, $result['order']->amount_paid); + + // The webhook arrives afterwards and finds the work done. + $this->completed($session); + + $this->assertSame(1500, $result['order']->fresh()->amount_paid); + $this->assertSame(1, Cart::find($cart->id)->order()->count()); + } + + #[Test] + public function confirming_an_open_or_expired_session_leaves_the_cart_alone() + { + $cart = $this->guestCartWith($this->product(), 1); + + $open = $this->confirmation($this->stripeSession($cart, ['status' => 'open', 'payment_status' => 'unpaid']))->confirm('cs_x'); + $expired = $this->confirmation($this->stripeSession($cart, ['status' => 'expired', 'payment_status' => 'unpaid']))->confirm('cs_x'); + + $this->assertSame(StripeCheckoutConfirmation::OPEN, $open['status']); + $this->assertSame(StripeCheckoutConfirmation::EXPIRED, $expired['status']); + $this->assertNull($cart->fresh()->order); + $this->assertFalse($cart->fresh()->isConverted()); + } + + #[Test] + public function confirming_a_session_of_an_unknown_cart_is_refused() + { + $cart = $this->guestCartWith($this->product(), 1); + $session = $this->stripeSession($cart, ['metadata' => (object) ['cart_id' => 'nope'], 'client_reference_id' => null]); + + $this->expectException(\InvalidArgumentException::class); + + $this->confirmation($session)->confirm($session->id); + } + + #[Test] + public function the_socket_that_paid_gets_a_fresh_cart() + { + $cart = $this->guestCartWith($this->product(), 1); + $this->completed($this->stripeSession($cart)); + + $next = CartFacade::adopt($cart->id, 'socket-A'); + + $this->assertFalse($next->is($cart)); + $this->assertFalse($next->isConverted()); + $this->assertSame('socket-A', $next->session_id); + $this->assertSame(0, $next->items()->count()); + $this->assertTrue($cart->fresh()->isConverted()); + } + + #[Test] + public function a_charge_event_after_checkout_does_not_pay_twice() + { + $cart = $this->guestCartWith($this->product(1000), 1); + $session = $this->stripeSession($cart); + $this->completed($session); + + $controller = new StripeWebhookController; + $method = (new ReflectionClass($controller))->getMethod('handleChargeSucceeded'); + $method->invoke($controller, (object) [ + 'id' => 'ch_test_1', + 'payment_intent' => $session->payment_intent, + 'amount' => 1000, + ]); + + $order = $cart->fresh()->order; + $this->assertSame(1000, $order->amount_paid); + $this->assertSame('ch_test_1', $order->getMeta('stripe_charge_id')); + } +}