Introduce locking to prevent duplicate invoices (Z#23150548) (#4067)

* Introduce locking to prevent duplicate invoices

This is not a perfect solution as it does not handle all code paths to
create invoices, but it handles all that seem likely to be triggered
concurrently

* Review note

src/pretix/api/views/order.py

@@ -97,6 +97,7 @@ from pretix.base.signals import (
 )
 from pretix.base.templatetags.money import money_filter
 from pretix.control.signals import order_search_filter_q
+from pretix.helpers import OF_SELF
 
 logger = logging.getLogger(__name__)
 
@@ -575,8 +576,10 @@ class EventOrderViewSet(OrderViewSetMixin, viewsets.ModelViewSet):
         return self.retrieve(request, [], **kwargs)
 
     @action(detail=True, methods=['POST'])
+    @transaction.atomic()
     def create_invoice(self, request, **kwargs):
         order = self.get_object()
+        order = Order.objects.select_for_update(of=OF_SELF).get(pk=order.pk)
         has_inv = order.invoices.exists() and not (
             order.status in (Order.STATUS_PAID, Order.STATUS_PENDING)
             and order.invoices.filter(is_cancellation=True).count() >= order.invoices.filter(is_cancellation=False).count()
@@ -1905,6 +1908,7 @@ class InvoiceViewSet(viewsets.ReadOnlyModelViewSet):
             return Response(status=204)
 
     @action(detail=True, methods=['POST'])
+    @transaction.atomic()
     def reissue(self, request, **kwargs):
         inv = self.get_object()
         if inv.canceled:
@@ -1912,9 +1916,10 @@ class InvoiceViewSet(viewsets.ReadOnlyModelViewSet):
         elif inv.shredded:
             raise PermissionDenied('The invoice file is no longer stored on the server.')
         else:
+            order = Order.objects.select_for_update(of=OF_SELF).get(pk=inv.order_id)
             c = generate_cancellation(inv)
             if inv.order.status != Order.STATUS_CANCELED:
-                inv = generate_invoice(inv.order)
+                inv = generate_invoice(order)
             else:
                 inv = c
             inv.order.log_action(

src/pretix/control/views/orders.py

@@ -129,6 +129,7 @@ from pretix.control.forms.rrule import RRuleForm
 from pretix.control.permissions import EventPermissionRequiredMixin
 from pretix.control.signals import order_search_forms
 from pretix.control.views import PaginationMixin
+from pretix.helpers import OF_SELF
 from pretix.helpers.compat import CompatDeleteView
 from pretix.helpers.format import format_map
 from pretix.helpers.safedownload import check_token
@@ -1562,20 +1563,22 @@ class OrderInvoiceCreate(OrderView):
     permission = 'can_change_orders'
 
     def post(self, *args, **kwargs):
-        has_inv = self.order.invoices.exists() and not (
-            self.order.status in (Order.STATUS_PAID, Order.STATUS_PENDING)
-            and self.order.invoices.filter(is_cancellation=True).count() >= self.order.invoices.filter(is_cancellation=False).count()
-        )
-        if self.request.event.settings.get('invoice_generate') not in ('admin', 'user', 'paid', 'True') or not invoice_qualified(self.order):
-            messages.error(self.request, _('You cannot generate an invoice for this order.'))
-        elif has_inv:
-            messages.error(self.request, _('An invoice for this order already exists.'))
-        else:
-            inv = generate_invoice(self.order)
-            self.order.log_action('pretix.event.order.invoice.generated', user=self.request.user, data={
-                'invoice': inv.pk
-            })
-            messages.success(self.request, _('The invoice has been generated.'))
+        with transaction.atomic():
+            order = Order.objects.select_for_update(of=OF_SELF).get(pk=self.order.pk)
+            has_inv = order.invoices.exists() and not (
+                order.status in (Order.STATUS_PAID, Order.STATUS_PENDING)
+                and order.invoices.filter(is_cancellation=True).count() >= order.invoices.filter(is_cancellation=False).count()
+            )
+            if self.request.event.settings.get('invoice_generate') not in ('admin', 'user', 'paid', 'True') or not invoice_qualified(order):
+                messages.error(self.request, _('You cannot generate an invoice for this order.'))
+            elif has_inv:
+                messages.error(self.request, _('An invoice for this order already exists.'))
+            else:
+                inv = generate_invoice(order)
+                order.log_action('pretix.event.order.invoice.generated', user=self.request.user, data={
+                    'invoice': inv.pk
+                })
+                messages.success(self.request, _('The invoice has been generated.'))
         return redirect(self.get_order_url())
 
     def get(self, *args, **kwargs):
@@ -1657,25 +1660,27 @@ class OrderInvoiceReissue(OrderView):
     permission = 'can_change_orders'
 
     def post(self, *args, **kwargs):
-        try:
-            inv = self.order.invoices.get(pk=kwargs.get('id'))
-        except Invoice.DoesNotExist:
-            messages.error(self.request, _('Unknown invoice.'))
-        else:
-            if inv.canceled:
-                messages.error(self.request, _('The invoice has already been canceled.'))
-            elif inv.shredded:
-                messages.error(self.request, _('The invoice has been cleaned of personal data.'))
+        with transaction.atomic():
+            order = Order.objects.select_for_update(of=OF_SELF).get(pk=self.order.pk)
+            try:
+                inv = order.invoices.get(pk=kwargs.get('id'))
+            except Invoice.DoesNotExist:
+                messages.error(self.request, _('Unknown invoice.'))
             else:
-                c = generate_cancellation(inv)
-                if self.order.status not in (Order.STATUS_CANCELED, Order.STATUS_EXPIRED):
-                    inv = generate_invoice(self.order)
+                if inv.canceled:
+                    messages.error(self.request, _('The invoice has already been canceled.'))
+                elif inv.shredded:
+                    messages.error(self.request, _('The invoice has been cleaned of personal data.'))
                 else:
-                    inv = c
-                self.order.log_action('pretix.event.order.invoice.reissued', user=self.request.user, data={
-                    'invoice': inv.pk
-                })
-                messages.success(self.request, _('The invoice has been reissued.'))
+                    c = generate_cancellation(inv)
+                    if order.status not in (Order.STATUS_CANCELED, Order.STATUS_EXPIRED):
+                        inv = generate_invoice(order)
+                    else:
+                        inv = c
+                    order.log_action('pretix.event.order.invoice.reissued', user=self.request.user, data={
+                        'invoice': inv.pk
+                    })
+                    messages.success(self.request, _('The invoice has been reissued.'))
         return redirect(self.get_order_url())
 
     def get(self, *args, **kwargs):  # NOQA

src/pretix/presale/views/order.py

@@ -84,6 +84,7 @@ from pretix.base.signals import order_modified, register_ticket_outputs
 from pretix.base.templatetags.money import money_filter
 from pretix.base.views.mixins import OrderQuestionsViewMixin
 from pretix.base.views.tasks import AsyncAction
+from pretix.helpers import OF_SELF
 from pretix.helpers.http import redirect_to_url
 from pretix.helpers.safedownload import check_token
 from pretix.multidomain.urlreverse import build_absolute_uri, eventreverse
@@ -450,20 +451,22 @@ class OrderPaymentConfirm(EventViewMixin, OrderDetailMixin, TemplateView):
 
     def post(self, request, *args, **kwargs):
         try:
-            i = self.order.invoices.filter(is_cancellation=False).last()
-            has_active_invoice = i and not i.canceled
-            if (not has_active_invoice or self.order.invoice_dirty) and invoice_qualified(self.order):
-                if self.request.event.settings.get('invoice_generate') == 'True' or (
-                        self.request.event.settings.get('invoice_generate') == 'paid' and self.payment.payment_provider.requires_invoice_immediately):
-                    if has_active_invoice:
-                        generate_cancellation(i)
-                    i = generate_invoice(self.order)
-                    self.order.log_action('pretix.event.order.invoice.generated', data={
-                        'invoice': i.pk
-                    })
-                    messages.success(self.request, _('An invoice has been generated.'))
-            self.payment.process_initiated = True
-            self.payment.save(update_fields=['process_initiated'])
+            with transaction.atomic():
+                order = Order.objects.select_for_update(of=OF_SELF).get(pk=self.order.pk)
+                i = order.invoices.filter(is_cancellation=False).last()
+                has_active_invoice = i and not i.canceled
+                if (not has_active_invoice or order.invoice_dirty) and invoice_qualified(order):
+                    if self.request.event.settings.get('invoice_generate') == 'True' or (
+                            self.request.event.settings.get('invoice_generate') == 'paid' and self.payment.payment_provider.requires_invoice_immediately):
+                        if has_active_invoice:
+                            generate_cancellation(i)
+                        i = generate_invoice(order)
+                        order.log_action('pretix.event.order.invoice.generated', data={
+                            'invoice': i.pk
+                        })
+                        messages.success(self.request, _('An invoice has been generated.'))
+                self.payment.process_initiated = True
+                self.payment.save(update_fields=['process_initiated'])
             resp = self.payment.payment_provider.execute_payment(request, self.payment)
         except PaymentException as e:
             messages.error(request, str(e))