fix: Prevent double charges and duplicate events in the checkout flow

src/viur/shop/modules/order.py

@@ -2,8 +2,10 @@
 import time
 import typing as t  # noqa
 
+import datetime
+
 from viur import toolkit
-from viur.core import current, db, errors as core_errors, exposed, force_post
+from viur.core import current, db, errors as core_errors, exposed, force_post, utils
 from viur.core.prototypes import List
 from viur.shop.types import *
 from viur.shop.types.results import PaymentProviderResult
@@ -300,7 +302,6 @@ def checkout_start(
             return JsonResponse({
                 "errors": errors,
             }, status_code=400)
-            raise e.InvalidStateError(", ".join(errors))
 
         if order_skel["cart"]["dest"]["key"] == self.shop.cart.current_session_cart_key:
             # This is now an order basket and should no longer be modified
@@ -401,18 +402,79 @@ def checkout_order(
             return JsonResponse({
                 "errors": errors,
             }, status_code=400)
-            raise e.InvalidStateError(", ".join(error_))
 
-        order_skel = HOOK_SERVICE.dispatch(Hook.ORDER_ASSIGN_UID, self._default_assign_uid)(order_skel)
-        # TODO: charge order if it should directly be charged
-        pp_res = self.get_payment_provider_by_name(order_skel["payment_provider"]).checkout(order_skel)
+        # Claim the order before talking to the payment provider: a repeated
+        # checkout_order call (double click, crash between the provider call
+        # and set_ordered, replayed request) must not trigger a second charge.
+        try:
+            order_skel = self._claim_checkout_order(order_skel)
+        except e.InvalidStateError as exc:
+            logging.warning(f"Rejecting checkout_order for {order_key!r}: {exc}")
+            return JsonResponse({
+                "errors": [ClientError("checkout_order is already in progress")],
+            }, status_code=409)
+
+        try:
+            order_skel = HOOK_SERVICE.dispatch(Hook.ORDER_ASSIGN_UID, self._default_assign_uid)(order_skel)
+            # TODO: charge order if it should directly be charged
+            pp_res = self.get_payment_provider_by_name(order_skel["payment_provider"]).checkout(order_skel)
+        except Exception:
+            # Release the claim, the user may retry immediately
+            toolkit.set_status(
+                key=order_skel["key"],
+                skel=order_skel,
+                values={"checkout_order_started": None},
+            )
+            raise
         order_skel = self.set_ordered(order_skel, pp_res)
         EVENT_SERVICE.call(Event.ORDER_CHANGED, order_skel=order_skel, deleted=False)
         return JsonResponse({
             "skel": order_skel,
             "payment": pp_res,
         })
 
+    CHECKOUT_ORDER_CLAIM_TIMEOUT: t.Final[datetime.timedelta] = datetime.timedelta(minutes=15)
+    """How long a `checkout_order` claim blocks further attempts.
+
+    If a claimed checkout neither completed (``is_ordered``) nor released its
+    claim (crash between the payment provider call and :meth:`set_ordered`),
+    a new attempt is allowed after this period."""
+
+    def _claim_checkout_order(
+        self,
+        order_skel: SkeletonInstance_T[OrderSkel],
+    ) -> SkeletonInstance_T[OrderSkel]:
+        """
+        Mark the order as "checkout_order in progress".
+
+        Sets ``checkout_order_started`` to now, guarded by a precondition:
+        already ordered orders and orders with a non-expired claim are
+        rejected with :exc:`InvalidStateError`.  Together with
+        `toolkit.set_status` this makes the claim a check-and-set --
+        the payment provider gets called at most once per claim period,
+        preventing double charges.
+
+        :param order_skel: Skeleton of the order to claim.
+        :return: The updated order skeleton.
+        :raises InvalidStateError: If the order is already ordered or claimed.
+        """
+
+        def precondition(skel: "SkeletonInstance") -> None:
+            if skel["is_ordered"]:
+                raise e.InvalidStateError("Order already is_ordered")
+            if (started := skel["checkout_order_started"]) is not None:
+                if utils.utcNow() - started < self.CHECKOUT_ORDER_CLAIM_TIMEOUT:
+                    raise e.InvalidStateError(f"checkout_order already started at {started.isoformat()}")
+                logging.warning(f'Overtaking expired checkout_order claim from {started.isoformat()} '
+                                f'of order {skel["key"]!r}')
+
+        return toolkit.set_status(
+            key=order_skel["key"],
+            skel=order_skel,
+            precondition=precondition,
+            values={"checkout_order_started": utils.utcNow()},
+        )
+
     def can_order(
         self,
         order_skel: "SkeletonInstance",
@@ -457,37 +519,78 @@ def set_checkout_in_progress(self, order_skel: "SkeletonInstance") -> "SkeletonI
         EVENT_SERVICE.call(Event.ORDER_CHANGED, order_skel=order_skel, deleted=False)
         return order_skel
 
+    def _set_state_once(
+        self,
+        order_skel: "SkeletonInstance",
+        state_bone: str,
+    ) -> tuple["SkeletonInstance", bool]:
+        """
+        Set a boolean state bone of an order to ``True`` exactly once.
+
+        Uses a `set_status` precondition, so the check and the write happen
+        in the same read-modify-write cycle: if the state is already set
+        (e.g. by a concurrently processed payment webhook), nothing is
+        written and the caller gets ``False`` -- it then must not fire the
+        state's events again, keeping side effects like mails or discount
+        accounting from running twice.
+
+        :param order_skel: Skeleton of the order to modify.
+        :param state_bone: Name of the boolean state bone (e.g. ``is_paid``).
+        :return: Tuple of the (possibly updated) skeleton and whether this
+            call actually performed the transition.
+        """
+
+        def precondition(skel: "SkeletonInstance") -> None:
+            if skel[state_bone]:
+                raise e.InvalidStateError(f"Order already has {state_bone} set")
+
+        try:
+            order_skel = toolkit.set_status(
+                key=order_skel["key"],
+                skel=order_skel,
+                precondition=precondition,
+                values={state_bone: True},
+            )
+        except e.InvalidStateError:
+            logger.info(f'Order {order_skel["key"]!r} already has {state_bone} set; skipping transition')
+            return order_skel, False
+        return order_skel, True
+
     def set_ordered(self, order_skel: "SkeletonInstance", payment: t.Any) -> "SkeletonInstance":
-        """Set an order to the state *ordered*"""
-        order_skel = toolkit.set_status(
-            key=order_skel["key"],
-            skel=order_skel,
-            values={"is_ordered": True},
-        )
-        EVENT_SERVICE.call(Event.ORDER_ORDERED, order_skel=order_skel, payment=payment)
-        EVENT_SERVICE.call(Event.ORDER_CHANGED, order_skel=order_skel, deleted=False)
+        """Set an order to the state *ordered*.
+
+        Idempotent: the ``ORDER_ORDERED`` event fires only on the actual
+        transition, a repeated call changes and triggers nothing.
+        """
+        order_skel, changed = self._set_state_once(order_skel, "is_ordered")
+        if changed:
+            EVENT_SERVICE.call(Event.ORDER_ORDERED, order_skel=order_skel, payment=payment)
+            EVENT_SERVICE.call(Event.ORDER_CHANGED, order_skel=order_skel, deleted=False)
         return order_skel
 
     def set_paid(self, order_skel: "SkeletonInstance") -> "SkeletonInstance":
-        """Set an order to the state *paid*"""
-        order_skel = toolkit.set_status(
-            key=order_skel["key"],
-            skel=order_skel,
-            values={"is_paid": True},
-        )
-        EVENT_SERVICE.call(Event.ORDER_PAID, order_skel=order_skel)
-        EVENT_SERVICE.call(Event.ORDER_CHANGED, order_skel=order_skel, deleted=False)
+        """Set an order to the state *paid*.
+
+        Idempotent: the ``ORDER_PAID`` event fires only on the actual
+        transition, a repeated call (e.g. return handler and payment
+        webhook processing the same payment) changes and triggers nothing.
+        """
+        order_skel, changed = self._set_state_once(order_skel, "is_paid")
+        if changed:
+            EVENT_SERVICE.call(Event.ORDER_PAID, order_skel=order_skel)
+            EVENT_SERVICE.call(Event.ORDER_CHANGED, order_skel=order_skel, deleted=False)
         return order_skel
 
     def set_rts(self, order_skel: "SkeletonInstance") -> "SkeletonInstance":
-        """Set an order to the state *Ready to ship*"""
-        order_skel = toolkit.set_status(
-            key=order_skel["key"],
-            skel=order_skel,
-            values={"is_rts": True},
-        )
-        EVENT_SERVICE.call(Event.ORDER_RTS, order_skel=order_skel)
-        EVENT_SERVICE.call(Event.ORDER_CHANGED, order_skel=order_skel, deleted=False)
+        """Set an order to the state *Ready to ship*.
+
+        Idempotent: the ``ORDER_RTS`` event fires only on the actual
+        transition, a repeated call changes and triggers nothing.
+        """
+        order_skel, changed = self._set_state_once(order_skel, "is_rts")
+        if changed:
+            EVENT_SERVICE.call(Event.ORDER_RTS, order_skel=order_skel)
+            EVENT_SERVICE.call(Event.ORDER_CHANGED, order_skel=order_skel, deleted=False)
         return order_skel
 
     # --- Hooks ---------------------------------------------------------------

src/viur/shop/skeletons/order.py

@@ -95,6 +95,16 @@ class OrderSkel(Skeleton):
     is_checkout_in_progress = BooleanBone(
     )
 
+    checkout_order_started = DateBone(
+    )
+    """When the final ``checkout_order`` step claimed this order.
+
+    Set before the payment provider is called and acts as a guard against
+    concurrent or repeated ``checkout_order`` calls charging twice.
+    Cleared again if the payment provider call fails; a stale claim (crash
+    after the provider call) expires after
+    :attr:`Order.CHECKOUT_ORDER_CLAIM_TIMEOUT`."""
+
     state = SelectBone(
         values=OrderState,
         multiple=True,