From f654ad0ba58f6722e00f92576e2217148a9543c9 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Mon, 19 Oct 2026 12:43:38 +0100 Subject: [PATCH 1/5] fix(checkout): close the webhook-first, contact and poll-ownership gaps - A customer who confirms a payment intent with the gateway directly, without calling pay(), leaves the session Open. The success listener only completed PaymentProcessing sessions, so the gateway placed the order with no OrderPlacing listeners, no contact email stamp, no payment method and no confirmation handover. The listener now pins an Open session against the live cart the gateway charged for (flagged meta.completed_without_pay_step, logged) and runs complete() like any other completion. The money is already taken, so the order is adopted rather than refused. - storeContact wrote a guest email onto PaymentProcessing and terminal sessions. The guest branch now applies the same gate as the driver's operableCart(): 409 frozen, 410 gone. - If the webhook completed the order before the guest's next processing poll, CartSession::current() had already minted a fresh cart and the poll 403'd. pay() now records the pinned session in the browser session, and ensureOwnership() accepts that for PaymentProcessing and Completed sessions. Co-Authored-By: Claude Opus 5.5 --- src/Http/Controllers/CheckoutController.php | 46 +++++++++++++ .../CompleteSessionOnPaymentSuccess.php | 51 ++++++++++++++- tests/Feature/ContactInformationTest.php | 27 ++++++++ tests/Feature/PayBoundaryTest.php | 29 +++++++++ tests/Feature/WebhookFirstPlacementTest.php | 64 +++++++++++++++++++ 5 files changed, 216 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/WebhookFirstPlacementTest.php diff --git a/src/Http/Controllers/CheckoutController.php b/src/Http/Controllers/CheckoutController.php index 69d88c2..b2bee26 100644 --- a/src/Http/Controllers/CheckoutController.php +++ b/src/Http/Controllers/CheckoutController.php @@ -29,6 +29,8 @@ use Lunar\Checkout\DataObjects\CheckoutTheme; use Lunar\Checkout\Events\CheckoutElementStored; use Lunar\Checkout\Exceptions\AddressLookupException; +use Lunar\Checkout\Exceptions\CheckoutSessionConflictException; +use Lunar\Checkout\Exceptions\CheckoutSessionNotOperableException; use Lunar\Checkout\Exceptions\PaymentConfirmationException; use Lunar\Checkout\Exceptions\RollbackQuote; use Lunar\Checkout\Express; @@ -56,6 +58,9 @@ class CheckoutController extends Controller { + /** Browser-session key listing the checkout sessions this browser pinned for payment. */ + private const PINNED_SESSIONS_KEY = 'lunar.checkout.pinned'; + /** * Collaborators are constructor-injected per the service-layer DI rule * (spec 0016): the element registry and the address lookup are bound to @@ -353,6 +358,18 @@ private function ensureOperable(CheckoutSessionModel $session): void */ private function ensureOwnership(CheckoutSessionModel $session): void { + /* + * The browser that pinned this session for payment owns it through + * the gateway round-trip. Checked before CartSession::current(): once + * a webhook has placed the order, current() mints the browser a fresh + * cart, so the cart id no longer proves anything and the guest's + * processing poll would 403 instead of reaching the confirmation. + */ + if (($session->status instanceof PaymentProcessing || $session->status instanceof Completed) + && in_array($session->uuid, (array) session()->get(self::PINNED_SESSIONS_KEY, []), true)) { + return; + } + $cart = CartSession::current(); if ($cart !== null && (string) $cart->id === $session->cart_reference) { @@ -370,6 +387,19 @@ private function ensureOwnership(CheckoutSessionModel $session): void ); } + /** + * Record in the browser session that this browser pinned the checkout + * session for payment, so ensureOwnership() still recognises it after a + * webhook completes the order. Only the most recent few are kept. + */ + private function rememberPinnedSession(CheckoutSessionModel $session): void + { + $pinned = array_values(array_diff((array) session()->get(self::PINNED_SESSIONS_KEY, []), [$session->uuid])); + $pinned[] = $session->uuid; + + session()->put(self::PINNED_SESSIONS_KEY, array_slice($pinned, -5)); + } + /** * Project the live checkout session into the prop shape the Vue app * consumes. Cart figures are read live (read verbs never persist); money @@ -656,6 +686,21 @@ public function storeContact(Request $request, CheckoutSessionModel $session, Ch if ($customerId !== null) { $checkoutDriver->associateCustomer($session, (string) $customerId, $data['email']); } else { + /* + * Same gate as the driver's operableCart() behind + * associateCustomer(): once the session is pinned for payment + * the contact email is part of what completion stamps on the + * order, so it is frozen (409); a terminal or expired session + * is gone (410). + */ + if ($session->status instanceof PaymentProcessing) { + throw new CheckoutSessionConflictException('frozen'); + } + + if (! $session->status instanceof Open || $session->isExpired()) { + throw new CheckoutSessionNotOperableException('The checkout session is expired or terminal.'); + } + $session->customer_email = $data['email']; $session->save(); } @@ -1163,6 +1208,7 @@ public function pay(Request $request, CheckoutSessionModel $session, CheckoutDri } $checkoutDriver->assertReadyForPayment($session, $data['fingerprint']); + $this->rememberPinnedSession($session); } catch (PaymentConfirmationException $e) { // A stale fingerprint or a Pay pressed before the contact step is // routine customer behaviour; every other reason (unorderable diff --git a/src/Listeners/CompleteSessionOnPaymentSuccess.php b/src/Listeners/CompleteSessionOnPaymentSuccess.php index 1f7ac51..35061db 100644 --- a/src/Listeners/CompleteSessionOnPaymentSuccess.php +++ b/src/Listeners/CompleteSessionOnPaymentSuccess.php @@ -2,8 +2,10 @@ namespace Lunar\Checkout\Listeners; +use Illuminate\Support\Facades\Log; use Lunar\Checkout\Contracts\CheckoutDriver; use Lunar\Checkout\Models\CheckoutSession; +use Lunar\Checkout\States\CheckoutSession\Open; use Lunar\Checkout\States\CheckoutSession\PaymentProcessing; use Lunar\Core\Events\PaymentAttemptEvent; use Lunar\Core\Models\Order; @@ -40,13 +42,60 @@ public function handle(PaymentAttemptEvent $event): void $session = CheckoutSession::query() ->where('cart_reference', (string) $cartId) - ->where('status', PaymentProcessing::$name) + ->whereIn('status', [PaymentProcessing::$name, Open::$name]) + ->orderByRaw('case when status = ? then 0 else 1 end', [PaymentProcessing::$name]) ->first(); if ($session === null) { return; } + if ($session->status instanceof Open && ! $this->pinForGatewayPayment($session)) { + return; + } + $this->driver->complete($session); } + + /** + * The customer confirmed the payment with the gateway directly (e.g. + * Stripe.js) without going through the pay boundary, so the session is + * still Open. The money is already taken, so refusing the order would + * strand it; instead pin the session the way the pay boundary does, + * against the live cart the gateway just charged for, and let complete() + * run its listeners (OrderPlacing, contact email, payment method) like + * any other completion. Expiry is not checked: the payment happened. + * + * Returns false when another process moved the session first. + */ + private function pinForGatewayPayment(CheckoutSession $session): bool + { + $live = $this->driver->snapshot($session); + + $pinned = CheckoutSession::query() + ->whereKey($session->getKey()) + ->where('status', Open::$name) + ->update([ + 'status' => PaymentProcessing::$name, + 'amount_subtotal' => $live->amountSubtotal, + 'amount_total' => $live->amountTotal, + 'cart_fingerprint' => $live->fingerprint, + 'payment_processing_at' => now(), + 'meta' => json_encode(array_merge((array) $session->meta, ['completed_without_pay_step' => true])), + 'updated_at' => now(), + ]); + + $session->refresh(); + + if ($pinned !== 1) { + return $session->status instanceof PaymentProcessing; + } + + Log::warning('Checkout session completed from a gateway payment without the pay step.', [ + 'checkout_session' => $session->uuid, + 'cart' => $session->cart_reference, + ]); + + return true; + } } diff --git a/tests/Feature/ContactInformationTest.php b/tests/Feature/ContactInformationTest.php index 2536261..e481500 100644 --- a/tests/Feature/ContactInformationTest.php +++ b/tests/Feature/ContactInformationTest.php @@ -6,6 +6,8 @@ use Lunar\Checkout\Contracts\ElementRegistry; use Lunar\Checkout\Elements\ContactInformation; use Lunar\Checkout\Session\SessionElementStore; +use Lunar\Checkout\States\CheckoutSession\Completed; +use Lunar\Checkout\States\CheckoutSession\PaymentProcessing; use Lunar\Core\Facades\CartSession; use Lunar\Core\Models\Customer; use Lunar\Tests\Checkout\Stubs\User; @@ -114,6 +116,31 @@ function makeUserWithCustomer(): array ->and($session->fresh()->customer_reference)->toBeNull(); }); +it('refuses a guest email change once the session is pinned for payment', function () { + $cart = routeTestCart(); + $session = app(CheckoutDriver::class)->createSession($cart); + $session->forceFill(['status' => PaymentProcessing::$name, 'customer_email' => 'paying@example.test'])->save(); + CartSession::use($cart); + + $this->postJson(route('lunar.checkout.contact.store', $session->uuid), ['email' => 'swapped@example.test']) + ->assertStatus(409) + ->assertJsonPath('reason', 'frozen'); + + expect($session->fresh()->customer_email)->toBe('paying@example.test'); +}); + +it('refuses a guest email change on a completed session', function () { + $cart = routeTestCart(); + $session = app(CheckoutDriver::class)->createSession($cart); + $session->forceFill(['status' => Completed::$name, 'customer_email' => 'paid@example.test'])->save(); + CartSession::use($cart); + + $this->postJson(route('lunar.checkout.contact.store', $session->uuid), ['email' => 'swapped@example.test']) + ->assertStatus(410); + + expect($session->fresh()->customer_email)->toBe('paid@example.test'); +}); + it('refuses an email with no domain', function (string $route) { $cart = routeTestCart(); $session = app(CheckoutDriver::class)->createSession($cart); diff --git a/tests/Feature/PayBoundaryTest.php b/tests/Feature/PayBoundaryTest.php index ff9246a..380d0e7 100644 --- a/tests/Feature/PayBoundaryTest.php +++ b/tests/Feature/PayBoundaryTest.php @@ -358,3 +358,32 @@ public function paymentBlocker(CheckoutSession $session, Cart $cart): ?string expect(Order::query()->count())->toBe(1); }); + +it('lets the browser that pinned a session poll it after a webhook completed it', function () { + app(PaymentMethodRegistry::class)->add(IntentTestMethod::class); + + $session = CheckoutCart::session(CheckoutCart::orderable()); + + $this->postJson(route('lunar.checkout.pay', $session->uuid), [ + 'fingerprint' => CheckoutCart::fingerprint($session), + 'payment_method' => 'card', + ])->assertSuccessful(); + + // The gateway webhook wins the race: completion places the order, so the + // next CartSession::current() mints the browser a fresh cart. + app(CheckoutDriver::class)->complete($session->refresh()); + + $this->get(route('lunar.checkout.processing', $session->uuid)) + ->assertRedirect(); +}); + +it('still forbids polling a completed session from a browser that never pinned it', function () { + app(PaymentMethodRegistry::class)->add(IntentTestMethod::class); + + $session = CheckoutCart::session(CheckoutCart::orderable()); + $session->forceFill(['status' => PaymentProcessing::$name])->save(); + app(CheckoutDriver::class)->complete($session->refresh()); + + $this->get(route('lunar.checkout.processing', $session->uuid)) + ->assertForbidden(); +}); diff --git a/tests/Feature/WebhookFirstPlacementTest.php b/tests/Feature/WebhookFirstPlacementTest.php new file mode 100644 index 0000000..cf90840 --- /dev/null +++ b/tests/Feature/WebhookFirstPlacementTest.php @@ -0,0 +1,64 @@ +update(['placed_at' => now()]); + + PaymentAttemptEvent::dispatch(new PaymentAuthorize( + success: $success, + orderId: $order->id, + paymentType: 'stripe', + )); +} + +it('completes a session the customer never pinned when the gateway takes payment for its cart', function () { + Event::fake([OrderPlacing::class]); + + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = app(CheckoutDriver::class)->resolveOrCreateSession($cart); + + expect($session->status)->toBeInstanceOf(Open::class); + + $order = $cart->createOrder(); + gatewayPlacesOrder($order); + + $session->refresh(); + + expect($session->status)->toBeInstanceOf(Completed::class) + ->and((string) $session->order_reference)->toBe((string) $order->id) + ->and($session->meta['completed_without_pay_step'] ?? null)->toBeTrue(); + + Event::assertDispatchedTimes(OrderPlacing::class, 1); + Event::assertDispatched(OrderPlacing::class, fn (OrderPlacing $event): bool => $event->order->id === $order->id); +}); + +it('leaves an open session alone when the gateway reports a failed payment', function () { + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = app(CheckoutDriver::class)->resolveOrCreateSession($cart); + + gatewayPlacesOrder($cart->createOrder(), success: false); + + expect($session->refresh()->status)->toBeInstanceOf(Open::class); +}); From 8a444785ae052e3a01a8c6ff9c99db4078d0b417 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Mon, 19 Oct 2026 12:50:48 +0100 Subject: [PATCH 2/5] fix(checkout): review follow-ups on webhook-first adoption - Write meta.completed_without_pay_step with a JSON-path update so a concurrent meta write (intent renewal, payment handle) is not lost. - Never adopt an Open hold-mode session from the success listener: it stays Open on purpose until the confirm page captures at reconcile. - storePaymentIntent refuses terminal and expired sessions. - Document that an OrderPlacing listener must not authorise a payment. - Tests: replayed success completes once, expired Open session is still adopted, chosen payment method survives the pin, hold-mode sessions are left Open, expired-session contact change is 410, payment intent on a completed session is 409. Co-Authored-By: Claude Opus 5.5 --- src/Http/Controllers/CheckoutController.php | 1 + .../CompleteSessionOnPaymentSuccess.php | 19 ++++++- tests/Feature/ContactInformationTest.php | 12 ++++ tests/Feature/PayBoundaryTest.php | 11 ++++ tests/Feature/WebhookFirstPlacementTest.php | 56 +++++++++++++++++++ 5 files changed, 96 insertions(+), 3 deletions(-) diff --git a/src/Http/Controllers/CheckoutController.php b/src/Http/Controllers/CheckoutController.php index b2bee26..92f064f 100644 --- a/src/Http/Controllers/CheckoutController.php +++ b/src/Http/Controllers/CheckoutController.php @@ -962,6 +962,7 @@ public function storeBillingAddress(Request $request, CheckoutSessionModel $sess public function storePaymentIntent(Request $request, CheckoutSessionModel $session, PaymentMethodRegistry $methods): JsonResponse { $this->ensureOwnership($session); + $this->ensureOperable($session); $data = $request->validate([ 'payment_method' => ['required', 'string'], diff --git a/src/Listeners/CompleteSessionOnPaymentSuccess.php b/src/Listeners/CompleteSessionOnPaymentSuccess.php index 35061db..bb6e6a7 100644 --- a/src/Listeners/CompleteSessionOnPaymentSuccess.php +++ b/src/Listeners/CompleteSessionOnPaymentSuccess.php @@ -14,11 +14,17 @@ * The gateway-agnostic success bridge (spec 0002 §D): any gateway driver that * authorises a payment dispatches PaymentAttemptEvent; when the paid order's * cart has a session pinned at the pay boundary, the checkout driver completes - * it — session → Completed, order adopted as its order_reference. + * it — session → Completed, order adopted as its order_reference. A session + * still Open (the customer confirmed with the gateway without the pay step) + * is pinned here first; see pinForGatewayPayment(). * * complete() is idempotent (terminal short-circuit + guarded transition), so * webhook replays, the reconciliation sweep, and this listener can all race * safely — exactly one completion wins. + * + * An OrderPlacing listener must not authorise a payment itself: during a + * synchronous complete() the session is still Open, so this listener would + * pin and complete it re-entrantly. */ class CompleteSessionOnPaymentSuccess { @@ -50,7 +56,12 @@ public function handle(PaymentAttemptEvent $event): void return; } - if ($session->status instanceof Open && ! $this->pinForGatewayPayment($session)) { + /* + * A hold-mode session stays Open on purpose until the customer + * finishes on the confirm page, which captures at reconcile; it is + * never adopted from here. + */ + if ($session->status instanceof Open && ($session->isHoldMode() || ! $this->pinForGatewayPayment($session))) { return; } @@ -81,7 +92,9 @@ private function pinForGatewayPayment(CheckoutSession $session): bool 'amount_total' => $live->amountTotal, 'cart_fingerprint' => $live->fingerprint, 'payment_processing_at' => now(), - 'meta' => json_encode(array_merge((array) $session->meta, ['completed_without_pay_step' => true])), + // A JSON-path write, so a concurrent meta write (intent + // renewal, payment handle) is not overwritten. + 'meta->completed_without_pay_step' => true, 'updated_at' => now(), ]); diff --git a/tests/Feature/ContactInformationTest.php b/tests/Feature/ContactInformationTest.php index e481500..ef8a1a1 100644 --- a/tests/Feature/ContactInformationTest.php +++ b/tests/Feature/ContactInformationTest.php @@ -141,6 +141,18 @@ function makeUserWithCustomer(): array expect($session->fresh()->customer_email)->toBe('paid@example.test'); }); +it('refuses a guest email change on an expired session', function () { + $cart = routeTestCart(); + $session = app(CheckoutDriver::class)->createSession($cart); + $session->forceFill(['expires_at' => now()->subMinute()])->save(); + CartSession::use($cart); + + $this->postJson(route('lunar.checkout.contact.store', $session->uuid), ['email' => 'late@example.test']) + ->assertStatus(410); + + expect($session->fresh()->customer_email)->toBeNull(); +}); + it('refuses an email with no domain', function (string $route) { $cart = routeTestCart(); $session = app(CheckoutDriver::class)->createSession($cart); diff --git a/tests/Feature/PayBoundaryTest.php b/tests/Feature/PayBoundaryTest.php index 380d0e7..a2d42a7 100644 --- a/tests/Feature/PayBoundaryTest.php +++ b/tests/Feature/PayBoundaryTest.php @@ -387,3 +387,14 @@ public function paymentBlocker(CheckoutSession $session, Cart $cart): ?string $this->get(route('lunar.checkout.processing', $session->uuid)) ->assertForbidden(); }); + +it('refuses to create a payment intent for a completed session', function () { + app(PaymentMethodRegistry::class)->add(IntentTestMethod::class); + + $session = CheckoutCart::session(CheckoutCart::orderable()); + $session->forceFill(['status' => Completed::$name])->save(); + + $this->postJson(route('lunar.checkout.payment-intent.store', $session->uuid), [ + 'payment_method' => 'card', + ])->assertStatus(409); +}); diff --git a/tests/Feature/WebhookFirstPlacementTest.php b/tests/Feature/WebhookFirstPlacementTest.php index cf90840..2d8d1f7 100644 --- a/tests/Feature/WebhookFirstPlacementTest.php +++ b/tests/Feature/WebhookFirstPlacementTest.php @@ -62,3 +62,59 @@ function gatewayPlacesOrder(Order $order, bool $success = true): void expect($session->refresh()->status)->toBeInstanceOf(Open::class); }); + +it('completes only once when the gateway replays the success', function () { + Event::fake([OrderPlacing::class]); + + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = app(CheckoutDriver::class)->resolveOrCreateSession($cart); + + $order = $cart->createOrder(); + gatewayPlacesOrder($order); + gatewayPlacesOrder($order->fresh()); + + expect($session->refresh()->status)->toBeInstanceOf(Completed::class); + + Event::assertDispatchedTimes(OrderPlacing::class, 1); +}); + +it('adopts the payment even when the open session has expired', function () { + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = app(CheckoutDriver::class)->resolveOrCreateSession($cart); + $session->forceFill(['expires_at' => now()->subMinute()])->save(); + + $order = $cart->createOrder(); + gatewayPlacesOrder($order); + + expect($session->refresh()->status)->toBeInstanceOf(Completed::class) + ->and((string) $session->order_reference)->toBe((string) $order->id); +}); + +it('keeps the payment method chosen before the gateway took payment', function () { + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = app(CheckoutDriver::class)->resolveOrCreateSession($cart); + $session->forceFill(['meta' => ['payment_method' => 'stripe']])->save(); + + gatewayPlacesOrder($cart->createOrder()); + + $session->refresh(); + + expect($session->meta['payment_method'] ?? null)->toBe('stripe') + ->and($session->meta['completed_without_pay_step'] ?? null)->toBeTrue() + ->and($session->cart_fingerprint)->not->toBeNull() + ->and($session->payment_processing_at)->not->toBeNull(); +}); + +it('never adopts an open hold-mode session', function () { + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = app(CheckoutDriver::class)->resolveOrCreateSession($cart); + $session->forceFill(['meta' => ['payment_intent_mode' => 'hold']])->save(); + + gatewayPlacesOrder($cart->createOrder()); + + expect($session->refresh()->status)->toBeInstanceOf(Open::class); +}); From c64682595cee727dc6198b4faf556aeacd749fcc Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Mon, 19 Oct 2026 12:54:56 +0100 Subject: [PATCH 3/5] fix(checkout): keep the webhook-first flag on MySQL; freeze pinned intents - MySQL's JSON_SET on a NULL meta column returns NULL, so the JSON-path write dropped meta.completed_without_pay_step for sessions that never stored an intent. Re-read the row under lockForUpdate() and write the merged meta instead; still safe against concurrent meta writes. - storePaymentIntent refuses a PaymentProcessing session (409 frozen): replacing the charged intent would leave reconciliation polling an unconfirmed one. Co-Authored-By: Claude Opus 5.5 --- src/Http/Controllers/CheckoutController.php | 6 +++ .../CompleteSessionOnPaymentSuccess.php | 44 +++++++++++++------ tests/Feature/PayBoundaryTest.php | 13 ++++++ 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/Http/Controllers/CheckoutController.php b/src/Http/Controllers/CheckoutController.php index 92f064f..24eed60 100644 --- a/src/Http/Controllers/CheckoutController.php +++ b/src/Http/Controllers/CheckoutController.php @@ -964,6 +964,12 @@ public function storePaymentIntent(Request $request, CheckoutSessionModel $sessi $this->ensureOwnership($session); $this->ensureOperable($session); + // A pinned session's intent is the one being charged; replacing it + // would leave reconciliation polling an unconfirmed intent. + if ($session->status instanceof PaymentProcessing) { + throw new CheckoutSessionConflictException('frozen'); + } + $data = $request->validate([ 'payment_method' => ['required', 'string'], 'mode' => ['sometimes', 'string', 'in:hold'], diff --git a/src/Listeners/CompleteSessionOnPaymentSuccess.php b/src/Listeners/CompleteSessionOnPaymentSuccess.php index bb6e6a7..7aee3e9 100644 --- a/src/Listeners/CompleteSessionOnPaymentSuccess.php +++ b/src/Listeners/CompleteSessionOnPaymentSuccess.php @@ -2,6 +2,7 @@ namespace Lunar\Checkout\Listeners; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Log; use Lunar\Checkout\Contracts\CheckoutDriver; use Lunar\Checkout\Models\CheckoutSession; @@ -83,20 +84,35 @@ private function pinForGatewayPayment(CheckoutSession $session): bool { $live = $this->driver->snapshot($session); - $pinned = CheckoutSession::query() - ->whereKey($session->getKey()) - ->where('status', Open::$name) - ->update([ - 'status' => PaymentProcessing::$name, - 'amount_subtotal' => $live->amountSubtotal, - 'amount_total' => $live->amountTotal, - 'cart_fingerprint' => $live->fingerprint, - 'payment_processing_at' => now(), - // A JSON-path write, so a concurrent meta write (intent - // renewal, payment handle) is not overwritten. - 'meta->completed_without_pay_step' => true, - 'updated_at' => now(), - ]); + /* + * Re-read under a row lock and write the merged meta, so a + * concurrent meta write (intent renewal, payment handle) is not + * lost. Not a JSON-path update: MySQL's JSON_SET on a NULL column + * returns NULL and would drop the flag. + */ + $pinned = DB::transaction(function () use ($session, $live): int { + $locked = CheckoutSession::query() + ->whereKey($session->getKey()) + ->lockForUpdate() + ->first(); + + if ($locked === null || ! $locked->status instanceof Open) { + return 0; + } + + return CheckoutSession::query() + ->whereKey($session->getKey()) + ->where('status', Open::$name) + ->update([ + 'status' => PaymentProcessing::$name, + 'amount_subtotal' => $live->amountSubtotal, + 'amount_total' => $live->amountTotal, + 'cart_fingerprint' => $live->fingerprint, + 'payment_processing_at' => now(), + 'meta' => json_encode(array_merge((array) $locked->meta, ['completed_without_pay_step' => true])), + 'updated_at' => now(), + ]); + }); $session->refresh(); diff --git a/tests/Feature/PayBoundaryTest.php b/tests/Feature/PayBoundaryTest.php index a2d42a7..9b2beed 100644 --- a/tests/Feature/PayBoundaryTest.php +++ b/tests/Feature/PayBoundaryTest.php @@ -398,3 +398,16 @@ public function paymentBlocker(CheckoutSession $session, Cart $cart): ?string 'payment_method' => 'card', ])->assertStatus(409); }); + +it('refuses to replace the payment intent of a session pinned for payment', function () { + app(PaymentMethodRegistry::class)->add(IntentTestMethod::class); + + $session = CheckoutCart::session(CheckoutCart::orderable()); + $session->forceFill(['status' => PaymentProcessing::$name, 'payment_intent_ref' => 'pi_charged'])->save(); + + $this->postJson(route('lunar.checkout.payment-intent.store', $session->uuid), [ + 'payment_method' => 'card', + ])->assertStatus(409)->assertJsonPath('reason', 'frozen'); + + expect($session->fresh()->payment_intent_ref)->toBe('pi_charged'); +}); From 99b8225f778e8f5de920bd080ef745ae4b3be066 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Mon, 19 Oct 2026 13:32:39 +0100 Subject: [PATCH 4/5] fix(checkout): address review on #3 - Pinned-session ownership grants carry a timestamp and lapse after an hour, so a shared browser does not keep an earlier guest's completed order reachable. - storePaymentIntent records the grant too: a customer who confirms with the gateway directly never calls pay(), and their processing poll must still reach the confirmation if the webhook wins. - One ensureWritable() gate for storeContact and storePaymentIntent, matching operableCart(): 409 frozen, 410 gone. - The listener takes its snapshot under the row lock, and documents that the raw guarded update mirrors assertReadyForPayment(). - Drop the session-preference ordering: active_cart_reference is unique, so a cart never has more than one live session. - Tests: poll asserts the success redirect and handover; the grant lapses after an hour; a webhook-first browser reaches the confirmation; the listener completes without flagging when pay() wins the race, and leaves a session another process ended. Co-Authored-By: Claude Opus 5.5 --- src/Http/Controllers/CheckoutController.php | 66 +++++---- .../CompleteSessionOnPaymentSuccess.php | 15 ++- tests/Feature/PayBoundaryTest.php | 24 +++- tests/Feature/WebhookFirstPlacementTest.php | 125 ++++++++++++++++++ 4 files changed, 200 insertions(+), 30 deletions(-) diff --git a/src/Http/Controllers/CheckoutController.php b/src/Http/Controllers/CheckoutController.php index 24eed60..2d1a48a 100644 --- a/src/Http/Controllers/CheckoutController.php +++ b/src/Http/Controllers/CheckoutController.php @@ -61,6 +61,9 @@ class CheckoutController extends Controller /** Browser-session key listing the checkout sessions this browser pinned for payment. */ private const PINNED_SESSIONS_KEY = 'lunar.checkout.pinned'; + /** Seconds a pinned-session ownership grant stays valid. */ + private const PINNED_SESSION_TTL = 3600; + /** * Collaborators are constructor-injected per the service-layer DI rule * (spec 0016): the element registry and the address lookup are bound to @@ -364,9 +367,14 @@ private function ensureOwnership(CheckoutSessionModel $session): void * a webhook has placed the order, current() mints the browser a fresh * cart, so the cart id no longer proves anything and the guest's * processing poll would 403 instead of reaching the confirmation. + * The grant lapses after PINNED_SESSION_TTL so a shared browser + * does not keep an earlier guest's completed order reachable. */ + $pinnedAt = ((array) session()->get(self::PINNED_SESSIONS_KEY, []))[$session->uuid] ?? null; + if (($session->status instanceof PaymentProcessing || $session->status instanceof Completed) - && in_array($session->uuid, (array) session()->get(self::PINNED_SESSIONS_KEY, []), true)) { + && is_int($pinnedAt) + && now()->timestamp - $pinnedAt <= self::PINNED_SESSION_TTL) { return; } @@ -389,15 +397,34 @@ private function ensureOwnership(CheckoutSessionModel $session): void /** * Record in the browser session that this browser pinned the checkout - * session for payment, so ensureOwnership() still recognises it after a - * webhook completes the order. Only the most recent few are kept. + * session for payment (or holds an intent it can confirm with the + * gateway), so ensureOwnership() still recognises it after a webhook + * completes the order. Keyed by UUID with the time it was recorded; + * only the most recent few are kept. */ private function rememberPinnedSession(CheckoutSessionModel $session): void { - $pinned = array_values(array_diff((array) session()->get(self::PINNED_SESSIONS_KEY, []), [$session->uuid])); - $pinned[] = $session->uuid; + $pinned = (array) session()->get(self::PINNED_SESSIONS_KEY, []); + unset($pinned[$session->uuid]); + $pinned[$session->uuid] = now()->timestamp; + + session()->put(self::PINNED_SESSIONS_KEY, array_slice($pinned, -5, preserve_keys: true)); + } + + /** + * The write gate shared by the routes that write session state directly, + * matching the driver's operableCart(): a session pinned for payment is + * frozen (409); a terminal or expired one is gone (410). + */ + private function ensureWritable(CheckoutSessionModel $session): void + { + if ($session->status instanceof PaymentProcessing) { + throw new CheckoutSessionConflictException('frozen'); + } - session()->put(self::PINNED_SESSIONS_KEY, array_slice($pinned, -5)); + if (! $session->status instanceof Open || $session->isExpired()) { + throw new CheckoutSessionNotOperableException('The checkout session is expired or terminal.'); + } } /** @@ -686,20 +713,9 @@ public function storeContact(Request $request, CheckoutSessionModel $session, Ch if ($customerId !== null) { $checkoutDriver->associateCustomer($session, (string) $customerId, $data['email']); } else { - /* - * Same gate as the driver's operableCart() behind - * associateCustomer(): once the session is pinned for payment - * the contact email is part of what completion stamps on the - * order, so it is frozen (409); a terminal or expired session - * is gone (410). - */ - if ($session->status instanceof PaymentProcessing) { - throw new CheckoutSessionConflictException('frozen'); - } - - if (! $session->status instanceof Open || $session->isExpired()) { - throw new CheckoutSessionNotOperableException('The checkout session is expired or terminal.'); - } + // Once the session is pinned for payment the contact email is + // part of what completion stamps on the order. + $this->ensureWritable($session); $session->customer_email = $data['email']; $session->save(); @@ -962,13 +978,10 @@ public function storeBillingAddress(Request $request, CheckoutSessionModel $sess public function storePaymentIntent(Request $request, CheckoutSessionModel $session, PaymentMethodRegistry $methods): JsonResponse { $this->ensureOwnership($session); - $this->ensureOperable($session); // A pinned session's intent is the one being charged; replacing it // would leave reconciliation polling an unconfirmed intent. - if ($session->status instanceof PaymentProcessing) { - throw new CheckoutSessionConflictException('frozen'); - } + $this->ensureWritable($session); $data = $request->validate([ 'payment_method' => ['required', 'string'], @@ -1092,6 +1105,11 @@ public function storePaymentIntent(Request $request, CheckoutSessionModel $sessi ]); } + // This browser now holds an intent it can confirm with the gateway + // directly; if the webhook completes the order first, its processing + // poll must still reach the confirmation. + $this->rememberPinnedSession($session); + return response()->json([ 'intent' => $descriptor->reference, 'clientSecret' => $descriptor->clientSecret, diff --git a/src/Listeners/CompleteSessionOnPaymentSuccess.php b/src/Listeners/CompleteSessionOnPaymentSuccess.php index 7aee3e9..2d0b4ef 100644 --- a/src/Listeners/CompleteSessionOnPaymentSuccess.php +++ b/src/Listeners/CompleteSessionOnPaymentSuccess.php @@ -49,8 +49,9 @@ public function handle(PaymentAttemptEvent $event): void $session = CheckoutSession::query() ->where('cart_reference', (string) $cartId) + // At most one: active_cart_reference is unique across live + // (Open / PaymentProcessing) sessions. ->whereIn('status', [PaymentProcessing::$name, Open::$name]) - ->orderByRaw('case when status = ? then 0 else 1 end', [PaymentProcessing::$name]) ->first(); if ($session === null) { @@ -82,15 +83,19 @@ public function handle(PaymentAttemptEvent $event): void */ private function pinForGatewayPayment(CheckoutSession $session): bool { - $live = $this->driver->snapshot($session); - /* * Re-read under a row lock and write the merged meta, so a * concurrent meta write (intent renewal, payment handle) is not * lost. Not a JSON-path update: MySQL's JSON_SET on a NULL column * returns NULL and would drop the flag. + * + * The status moves with the same guarded raw update the pay + * boundary uses (LunarCheckoutDriver::assertReadyForPayment()): + * session states change through these one-statement guards, not + * model transitions. The snapshot is taken under the lock; complete() + * re-verifies it against the cart under the cart-row lock anyway. */ - $pinned = DB::transaction(function () use ($session, $live): int { + $pinned = DB::transaction(function () use ($session): int { $locked = CheckoutSession::query() ->whereKey($session->getKey()) ->lockForUpdate() @@ -100,6 +105,8 @@ private function pinForGatewayPayment(CheckoutSession $session): bool return 0; } + $live = $this->driver->snapshot($locked); + return CheckoutSession::query() ->whereKey($session->getKey()) ->where('status', Open::$name) diff --git a/tests/Feature/PayBoundaryTest.php b/tests/Feature/PayBoundaryTest.php index 9b2beed..67ad35f 100644 --- a/tests/Feature/PayBoundaryTest.php +++ b/tests/Feature/PayBoundaryTest.php @@ -372,9 +372,29 @@ public function paymentBlocker(CheckoutSession $session, Cart $cart): ?string // The gateway webhook wins the race: completion places the order, so the // next CartSession::current() mints the browser a fresh cart. app(CheckoutDriver::class)->complete($session->refresh()); + $session->refresh(); $this->get(route('lunar.checkout.processing', $session->uuid)) - ->assertRedirect(); + ->assertRedirect($session->success_url ?: (config('lunar.checkout.urls.success') ?: '/')) + ->assertSessionHas('lunar.checkout.completed.order_reference', $session->order_reference); +}); + +it('lets a pinned-session ownership grant lapse after an hour', function () { + app(PaymentMethodRegistry::class)->add(IntentTestMethod::class); + + $session = CheckoutCart::session(CheckoutCart::orderable()); + + $this->postJson(route('lunar.checkout.pay', $session->uuid), [ + 'fingerprint' => CheckoutCart::fingerprint($session), + 'payment_method' => 'card', + ])->assertSuccessful(); + + app(CheckoutDriver::class)->complete($session->refresh()); + + $this->travel(61)->minutes(); + + $this->get(route('lunar.checkout.processing', $session->uuid)) + ->assertForbidden(); }); it('still forbids polling a completed session from a browser that never pinned it', function () { @@ -396,7 +416,7 @@ public function paymentBlocker(CheckoutSession $session, Cart $cart): ?string $this->postJson(route('lunar.checkout.payment-intent.store', $session->uuid), [ 'payment_method' => 'card', - ])->assertStatus(409); + ])->assertStatus(410); }); it('refuses to replace the payment intent of a session pinned for payment', function () { diff --git a/tests/Feature/WebhookFirstPlacementTest.php b/tests/Feature/WebhookFirstPlacementTest.php index 2d8d1f7..d1607e4 100644 --- a/tests/Feature/WebhookFirstPlacementTest.php +++ b/tests/Feature/WebhookFirstPlacementTest.php @@ -2,19 +2,85 @@ use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Log; use Lunar\Checkout\Contracts\CheckoutDriver; +use Lunar\Checkout\Contracts\PaymentMethodRegistry; use Lunar\Checkout\Events\OrderPlacing; +use Lunar\Checkout\Models\CheckoutSession; +use Lunar\Checkout\PaymentMethods\AbstractPaymentMethod; +use Lunar\Checkout\States\CheckoutSession\Cancelled; use Lunar\Checkout\States\CheckoutSession\Completed; use Lunar\Checkout\States\CheckoutSession\Open; +use Lunar\Checkout\States\CheckoutSession\PaymentProcessing; +use Lunar\Core\Contracts\CreatesPaymentIntents; use Lunar\Core\DataObjects\PaymentAuthorize; +use Lunar\Core\DataObjects\PaymentIntentDescriptor; use Lunar\Core\Events\PaymentAttemptEvent; use Lunar\Core\Facades\CartSession; +use Lunar\Core\Facades\Payments; +use Lunar\Core\Models\Cart; use Lunar\Core\Models\Order; +use Lunar\Core\PaymentTypes\OfflinePayment; use Lunar\Tests\Checkout\TestCase; use Lunar\Tests\Checkout\Utils\CheckoutCart; uses(TestCase::class, RefreshDatabase::class); +/** A Stripe-like gateway: the browser gets an intent it can confirm directly. */ +class WebhookFirstIntentGateway extends OfflinePayment implements CreatesPaymentIntents +{ + public function createIntent(Cart $cart): PaymentIntentDescriptor + { + return new PaymentIntentDescriptor('pi_webhook_first_'.$cart->id, 'secret_'.$cart->id); + } +} + +class WebhookFirstIntentMethod extends AbstractPaymentMethod +{ + public function handle(): string + { + return 'webhook-first-card'; + } + + public function label(): string + { + return 'Card'; + } + + public function driver(): string + { + return 'webhook-first-intents'; + } + + public function requiresIntent(): bool + { + return true; + } + + public function component(): string + { + return 'stripe-card'; + } +} + +/** + * Swap the checkout driver for a proxy whose snapshot() first runs $race, + * standing in for another process moving the session while the listener + * holds its lock-free view of it. + */ +function raceTheListener(Closure $race): void +{ + $real = app(CheckoutDriver::class); + $proxy = Mockery::mock($real)->makePartial(); + $proxy->shouldReceive('snapshot')->andReturnUsing(function (CheckoutSession $session) use ($real, $race) { + $race($session); + + return $real->snapshot($session); + }); + + app()->instance(CheckoutDriver::class, $proxy); +} + /** * The gateway placed an order for the cart (its webhook authorised a payment * the customer confirmed client-side) and announced it the way every gateway @@ -118,3 +184,62 @@ function gatewayPlacesOrder(Order $order, bool $success = true): void expect($session->refresh()->status)->toBeInstanceOf(Open::class); }); + +it('lets the browser that created the intent reach the confirmation after a webhook-first completion', function () { + Payments::extend('webhook-first-intents', fn () => app(WebhookFirstIntentGateway::class)); + app(PaymentMethodRegistry::class)->add(WebhookFirstIntentMethod::class); + + $session = CheckoutCart::session(CheckoutCart::orderable()); + + $this->postJson(route('lunar.checkout.payment-intent.store', $session->uuid), [ + 'payment_method' => 'webhook-first-card', + ])->assertSuccessful(); + + // The customer confirms with the gateway directly; the webhook places + // the order before the browser's first processing poll. + $cart = Cart::find($session->cart_reference); + gatewayPlacesOrder($cart->createOrder()); + $session->refresh(); + + expect($session->status)->toBeInstanceOf(Completed::class); + + $this->get(route('lunar.checkout.processing', $session->uuid)) + ->assertRedirect($session->success_url ?: (config('lunar.checkout.urls.success') ?: '/')) + ->assertSessionHas('lunar.checkout.completed.order_reference', $session->order_reference); +}); + +it('still completes when the pay step pins the session first, without flagging it', function () { + Log::spy(); + + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = app(CheckoutDriver::class)->resolveOrCreateSession($cart); + + raceTheListener(fn (CheckoutSession $s) => CheckoutSession::query()->whereKey($s->getKey()) + ->update(['status' => PaymentProcessing::$name, 'cart_fingerprint' => $s->cart_fingerprint])); + + gatewayPlacesOrder($cart->createOrder()); + $session->refresh(); + + expect($session->status)->toBeInstanceOf(Completed::class) + ->and($session->meta['completed_without_pay_step'] ?? null)->toBeNull(); + + Log::shouldNotHaveReceived('warning'); +}); + +it('leaves a session alone when another process ends it first', function () { + Log::spy(); + + $cart = CheckoutCart::orderable(); + CartSession::use($cart); + $session = app(CheckoutDriver::class)->resolveOrCreateSession($cart); + + raceTheListener(fn (CheckoutSession $s) => CheckoutSession::query()->whereKey($s->getKey()) + ->update(['status' => Cancelled::$name])); + + gatewayPlacesOrder($cart->createOrder()); + + expect($session->refresh()->status)->toBeInstanceOf(Cancelled::class); + + Log::shouldNotHaveReceived('warning'); +}); From 3b1ad7b7b28b821627d904f2ebc336d717d77252 Mon Sep 17 00:00:00 2001 From: Alec Ritson Date: Mon, 19 Oct 2026 13:35:45 +0100 Subject: [PATCH 5/5] docs(checkout): clarify the race helper and when the intent grant starts Co-Authored-By: Claude Opus 5.5 --- src/Http/Controllers/CheckoutController.php | 4 +++- tests/Feature/WebhookFirstPlacementTest.php | 7 ++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Http/Controllers/CheckoutController.php b/src/Http/Controllers/CheckoutController.php index 2d1a48a..92ea8dc 100644 --- a/src/Http/Controllers/CheckoutController.php +++ b/src/Http/Controllers/CheckoutController.php @@ -1107,7 +1107,9 @@ public function storePaymentIntent(Request $request, CheckoutSessionModel $sessi // This browser now holds an intent it can confirm with the gateway // directly; if the webhook completes the order first, its processing - // poll must still reach the confirmation. + // poll must still reach the confirmation. The grant's clock starts + // here, so a customer who sits on the card form past the TTL and + // then confirms directly falls back to the cart ownership check. $this->rememberPinnedSession($session); return response()->json([ diff --git a/tests/Feature/WebhookFirstPlacementTest.php b/tests/Feature/WebhookFirstPlacementTest.php index d1607e4..52c5988 100644 --- a/tests/Feature/WebhookFirstPlacementTest.php +++ b/tests/Feature/WebhookFirstPlacementTest.php @@ -64,9 +64,10 @@ public function component(): string } /** - * Swap the checkout driver for a proxy whose snapshot() first runs $race, - * standing in for another process moving the session while the listener - * holds its lock-free view of it. + * Swap the checkout driver for a proxy whose snapshot() first runs $race. + * The listener calls snapshot() between its locked read and its guarded + * update, so this exercises the guarded update's lost-race fallback (in + * production the row lock serialises the other writer instead). */ function raceTheListener(Closure $race): void {